Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.We need tests for comments which span multiple pages. This should include the #new comment link (from tracker.module or forum index), and flat and threaded display.
| Comment | File | Size | Author |
|---|---|---|---|
| #18 | comment_tests.patch | 6.96 KB | catch |
| #14 | comment.test.patch | 9.76 KB | aaronbauman |
| #12 | comment.test.patch | 9.97 KB | aaronbauman |
| #10 | comment.test.patch | 7.74 KB | aaronbauman |
| #8 | comment.test.patch | 7.66 KB | aaronbauman |
Comments
Comment #1
jsaints commented@catch
Could you explain a little more what you mean by "#new comment link (from tracker.module or forum index)"?
Do these steps cover the scope for the test? :
1) create 25 comments
2) check that the first comment is on page 3
3) check that comment 12 is on page 2
4) check that comment 5 is on page 1
Repeat this for threaded and flat displays.
Is there more the test needs to do?
Thanks.
Comment #2
catchjsaints - that should be plenty. Also, this test would expose the bug at #322549: t() should work regardless of the bootstrap phase - so should fail while you're writing it unless you have that patch applied.
With the #new links, if you enable the tracker module, it adds a 'new' link next to node titles which have new comments - this link goes to node/1#new or node/1?page=2#new dependent on how many comments, and the display setting - would be good to check it's going to the right place, since the code there is very tweaky. But that doesn't need to be done in this issue at all.
Comment #4
aaronbaumanComment #5
aaronbaumanHere's my first simpletest patch - please go easy on the flames =]
This patch:
* creates 25 comments
* verifies that they appear on the correct pages
* creates 5 replies to the eldermost comment
* assures that the replies appear on the correct pages for threaded as well as flat displays
I've also updated the classes to address some of the further changes to the comment module, including addressing at least part of #246392: TestingParty08: comment form location
Comment #6
boombatower commentedFew minor things.
Comment format: start with capital end with period and don't leave extra spacing:
to
Seem to have mixed spaces and tabs, drupal uses all double spaces. You should be a ble the change your IDE to always use two spaces. You can see what it looks like currently in the attached screenshot.
Looks like you have some left over debug code, for example:
Otherwise seems fairly clean.
I will ask:
Does it make sense to be looping to add comments...if purely for duplicating the test then remove. We only need to insert one comment for each item to test (per setting w/e) no need to test the same thing four times, unless there is an advantage.
Great work!
Comment #7
aaronbaumanThanks for reviewing.
I've updated the formatting and leftover code.
Also: I may have gone overboard in the number of comments created.
I've reduced this by a factor of 10 by lowering the comments / page setting from 10 to 1.
This is probably not a realistic setting, but should be sufficient for testing.
Comment #8
aaronbaumanoops - *still* had some formatting issues
Comment #10
aaronbaumanlet's try this again - rerolled against HEAD
Comment #11
catchIf it's commented it shouldn't be in the patch. Also some code style errors (missing period at end of sentences etc).
Otherwise looks good.
Comment #12
aaronbaumanrerolled @ code style.
Comment #14
aaronbaumanComment #15
aaronbaumanComment #17
aaronbaumanComment #18
catchRe-rolled.
Tests should fail spectacularly due to #484090: Comment paging is broken
Comment #20
berdirLooks like the other test fails are because of http://drupal.org/node/26966. The comment is posted to page two, but the helper function is redirected to the first page, so the new comment is not visible.
Comment #21
catchPostponing on those two.
Comment #22
dries commented#484090: Comment paging is broken was committed, so we're only blocked on #26966: Fix comment links when paging is used..
I prefer to commit the tests with the patch -- this dependency on multiple other patches complicates things more than necessary.
Comment #23
catchOK let's consolidate everything in #26966: Fix comment links when paging is used..