Posted by skywalker2208 on March 27, 2008 at 5:54pm
7 followers
| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | tests |
| Category: | task |
| Priority: | critical |
| Assigned: | floretan |
| Status: | closed (fixed) |
Issue Summary
Setup some simpletest for the tracker module and noticed dmitrig01 is doing some work on it as well. Maybe the tests that I created can help dmitrig01 out.
Comments
#1
Forgot to add the test file.
#2
Marking needs review. No time to do it myself atm.
#3
I was hoping dmitrig01 would post his and we could compare/merge, but that doesn't seem to be happening so I'll review yours tomorrow.
#4
Here they are, my very special tracker tests.
#5
With the tests this time!
#6
@dmitrig01: I noticed a few things that I would appreciate if they were cleaned up.
drupalCreateNodesmethod is necessary, and if it is I would like to have others approve it. The comment has a bit of random spacing and is missing periods.t().tearDownjust to call parent.setUp.<?php// Enable the comment and tracker modules
$this->drupalModuleEnable('comment');
$this->drupalModuleEnable('tracker');
// Create some nodes, a user, etc.
$data = $this->trackerPostNodes();
?>
You can look at the tests that I have reviewed to see an example of what we are looking for.
The changes are minor, but the test as a whole looks nice.
Thanks.
#7
http://drupal.org/node/253500 was duplicate.
#8
I think boombatower's review still stands.
#9
I agree with boombatower's comments. The patch from #5 is also not structured like any existing core texts:
* Its use of xpath is very powerful, but also very obscure.
* Direct calls to curlExec() are a big no-no, they belong in the testing framework, not in the tests themselves.
* Why do we need to test everything on multiple nodes at once? Unless we're looking for a specific behavior when dealing with multiple nodes, we shouldn't test on multiple nodes. There's no need to add unrelated parameters to a test.
Here's a simpler test that covers the same functionality.
#10
Although the patch from #1 needs some clean-up, it covers some code that is not covered by dmitrig01's patch or mine. I'll work on merging these tests.
#11
After a closer look, the only case checked by the patch from #1 that I was missing was that unpublished nodes should not show up in the tracker listing.
#12
This patch is much cleaner than the original. It passes on my dev box and seems to be much more consistent with other tests.
I removed several page content outputs that seem to be left over from development and cleaned up some white-spacing.
Good work flobruit.
#13
Change component is relation to http://drupal.org/node/253744.
#14
Committed to CVS HEAD. Thanks all.
#15
Automatically closed -- issue fixed for two weeks with no activity.