TestingParty08: viewing an individual comment
catch - August 17, 2008 - 20:40
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | tests |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | closed |
Description
comment_render() iirc. Check it shows up properly, body, subject etc.
#1
See attached patch, which uses the reply-to-comment screen to check the output of an individual comment.
#2
Very simple patch. :)
Comment test results:
473 passes, 0 fails, 0 exceptionsI 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.
<?php// Get comment.
if ($contact !== TRUE) { // If true then attempting to find error message.
$this->assertText($subject, 'Comment posted.');
$this->assertTrue((!empty($match) && !empty($match[1])), t('Comment id found.'));
}
?>
Right after $subject.
#3
Basically a one-line patch? :) Attached.
#4
Tested locally. "479 passes, 0 fails, 0 exceptions" (comment.test)
#5
Nope. We want a drupalGet("node/$nid/$cid") and check whether the comment appeared there.
#6
chx 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)
#7
484 passes, 0 fails, 0 exceptions (comment.test)
#8
/me likes.
#9
I never ever knew we had node/nid/cid, ever.
#10
I wonder whether we still generate node/nid/cid links ... If not, this might actually be dead code?
#11
I 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.
#12
Haha 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...
#13
I 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:
<?php
function node_menu() {
// ...
$items['node/%node'] = array(
// ...
'page callback' => 'node_page_view',
'page arguments' => array(1),
// ...
'type' => MENU_CALLBACK);
// ...
}
function node_page_view($node, $cid = NULL) {
drupal_set_title(check_plain($node->title));
return node_show($node, $cid);
}
function node_show($node, $cid, $message = FALSE) {
// ...
$output = node_view($node, FALSE, TRUE);
if (function_exists('comment_render') && $node->comment) {
$output .= comment_render($node, $cid);
}
// ...
return $output;
}
?>
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.
#14
Oops, didn't mean to change status
#15
Re-roll, post #302396: Make tests more granular. 484 passes, 0 fails, 0 exceptions.
#16
Yeah, 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!
#17
Automatically closed -- issue fixed for two weeks with no activity.