We need to write simple tests. I've avoided this issue in the past because it involves testing against other modules. However, missing tests is an impediment for ongoing changes like #215979: Create a CCK date field mapper, #397682: Let mappers declare mapping targets or #433384: Make mapper node-independent .
These need to be integration tests that actually test the mapper with a feed against the integrated module.
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | mapper_tests.patch | 120.17 KB | lyricnz |
| #10 | 435450_mapper_tests.patch | 120.11 KB | aron novak |
| #8 | feedapi_mapper_tests.patch | 119.77 KB | lyricnz |
| #6 | feedapi_mapper_tests.patch | 85.61 KB | lyricnz |
| #5 | feedapi_mapper_tests.patch | 77.53 KB | lyricnz |
Comments
Comment #1
lyricnz commentedI've started working on this. I have a simple test that creates a feed node, configures some mapping, and consumes an XML (really more of an exercise than a test).
Next, I will create some CCK field mappings, to test individual cases. Sample feed data welcome!
Comment #2
lyricnz commentedFYI, while writing these tests, I came across a strange behaviour, which turned out to be a bug: #451564: Using timestamp fields in the Date mapper empties created node.
Comment #3
lyricnz commentedFirst draft of the tests for base API, CCK mapper, and start of date mapper.
"279 passes, 0 fails, and 0 exceptions"
Comment #4
alex_b commentedThat's a great start. Here's my first round of review. I know you're in the middle of this, so some of these points may be on your list anyway:
I think it's a great idea to keep the tests in separate files. This will keep things overlookable. I also like that you're including the feeds in the tests. We should do this for FeedAPI as well. At a later point we may want to push these test feeds into the FeedAPI module.
Nice work.
Comment #5
lyricnz commentedI've done 1,3,4 per your suggestion, and removed the <type> items from the comments. Did you have specific concerns with the comments?
Comment #6
lyricnz commentedAdded tests for Link and Taxonomy
353 passes, 1 fail, and 0 exceptions
(the failure is due to #451926: FeedAPI reports "1 existing item(s) were updated" when consuming a brand-new feed)
Comment #7
alex_b commentedGreat work. I think the approach is solid.
Time for nit picking :)
1) string concatenation is off at some points - should be Drupal 6.x style. e. g. line 174 in feedapi_mapper_test.inc
2) remove all debug output
3) I'm assuming that calls to writeFile() will all go away, but if some stay, we should use file names that are prefixed with feedapi_mapper_test_
4) Clean up comments:
- add a break after all @param $variable and indent explanation by 2 spaces - e. g. like on FeedApiMapperTestCase::_createContentType()
- Make sure that @file comments aren't attached to class definitions like in feedapi_mapper_test.inc .
5) rename TODO to @todo (consistent with feedapi_mapper.module)
I'm going to have a look at #451926: FeedAPI reports "1 existing item(s) were updated" when consuming a brand-new feed now.
Comment #8
lyricnz commented1) Not fixed yet - ask me tomorrow :(
2) Done - including writeFile() calls, except for the two test-cases I'm actively working on
3) Done - added prefix to filenames
4) Done
5) Done
Added start of emimage testcase - WIP. Added an ical sample file.
438 passes, 1 fail, and 0 exceptions (excluding date mapper; error as described above)
Comment #9
aron novakI get lots of fails now, maybe because of FeedAPI changes, further testing is needed. For example createFeed() does not work.
Comment #10
aron novakRe-rolled.
Mostly i needed to touch initialization parts, surely it's because of changes in feedapi and other modules.
Comment #11
alex_b commentedLyricnz reviewed and came back to me with:
"I get repeatable http 500 executing the date mapper test; the other five are fine though"
Comment #12
lyricnz commentedI get repeatable HTTP 500 error executing the Date mapper test, but the other 5 pass fine:
Overall results: 450 passes, 0 fails, and 0 exceptions
FeedAPI Mapper Link: 94 passes, 0 fails, and 0 exceptions
FeedAPI Mapper Embedded Image Field: 82 passes, 0 fails, and 0 exceptions
FeedAPI Mapper Content: 88 passes, 0 fails, and 0 exceptions
FeedAPI Mapper: 114 passes, 0 fails, and 0 exceptions
FeedAPI Mapper Taxonomy: 72 passes, 0 fails, and 0 exceptions
This is with SimpleTest 6.x-2.8. Looking deeper now.
Comment #13
lyricnz commentedThe problem with the Date test is the same as #472684: Install non-default modules one at a time to ensure proper module list is maintained during installation hooks, which isn't fixed in Core D7 yet.
You can work around the issue by declaring the order of your module requirements, without relying on the dependency system. Ie: instead of:
use
(Note the order of the last three modules). Reroll attached - this is the only change
Comment #14
aron novak#13, really strange that this problem did not appear at my dev environment...
All in all, i tested it, it seems that now it's both fine in my and your environment, so now it's RTBC.
Comment #15
alex_b commentedYoohoo. Tests! Need to review & commit as soon as I get some time over here.
Comment #16
alex_b commentedCommitted. Thank you!