comment_render() iirc. Check it shows up properly, body, subject etc.
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | individual-comment.test.patch | 1.84 KB | lyricnz |
| #6 | individual-comment.test.patch | 1.83 KB | lyricnz |
| #3 | comment-test.checksubject.patch | 739 bytes | lyricnz |
| #1 | individual-comment.test.patch | 1.22 KB | lyricnz |
Comments
Comment #1
lyricnz commentedSee attached patch, which uses the reply-to-comment screen to check the output of an individual comment.
Comment #2
boombatower commentedVery simple patch. :)
Comment test results:
I think instead the test should be placed in postComment() since that already checks for the subject showing up. could just add body and w/e else.
Right after $subject.
Comment #3
lyricnz commentedBasically a one-line patch? :) Attached.
Comment #4
lyricnz commentedTested locally. "479 passes, 0 fails, 0 exceptions" (comment.test)
Comment #5
chx commentedNope. We want a drupalGet("node/$nid/$cid") and check whether the comment appeared there.
Comment #6
lyricnz commentedchx is right, while these tests are good, none of them address the issue described in the subject. The attached patch:
- checks comment subject/text on the node/NID/CID page (single comment view - what this issue was asking for)
- checks comment subject/text on the comment/reply/NID/CID page (comment reply view)
- checks comment text on every post-comment page (subject was already checked)
Comment #7
lyricnz commented484 passes, 0 fails, 0 exceptions (comment.test)
Comment #8
chx commented/me likes.
Comment #9
catchI never ever knew we had node/nid/cid, ever.
Comment #10
dries commentedI wonder whether we still generate node/nid/cid links ... If not, this might actually be dead code?
Comment #11
catchI don't think there's a link to them anywhere, or if there is, then I'm completely unaware of where that might be. However I checked on a D5 site and those pages do indeed exist. They seem a bit pointless though.
Comment #12
chx commentedHaha we have written a test and it revealed that the code being tested is dead?? Because indeed I grepped node/ in comment directory and can't see where this is being created...
Comment #13
lyricnz commentedI did a similar search - for 'node/' and nid and cid, across the entire codebase (since the menu entry that handles this is actually added by node.module), and didn't find anything.
Interestingly, support for this URL seems to be almost a side effect:
So removing it would (a) change the functions parameters above, and (b) not save much code. In any case, removing it seems like a seperate issue - this issue is about a test-case: as long as the code is present, the test is valid, right? The testcase will (correctly) trip up if someone removes the code, and can be fixed then.
Comment #14
lyricnz commentedOops, didn't mean to change status
Comment #15
lyricnz commentedRe-roll, post #302396: Make tests more granular. 484 passes, 0 fails, 0 exceptions.
Comment #16
webchickYeah, I agree that while this code exists, we should probably write tests for it, and the question of whether or not it's truly dead (I could see the use case for "permalinking" to a comment, maybe...) taken up in a separate issue. lyricnz's research into it in #13 makes it a bit muddy as to whether we'd actually be saving anything or not.
Test looks good. Committed to HEAD. Thanks!
Comment #17
Anonymous (not verified) commentedAutomatically closed -- issue fixed for two weeks with no activity.