taxonomy.test is sad
catch - November 14, 2008 - 13:16
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | tests |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Description
Very sad.
This consolidates some of the tests and cleans a lot of it up. There's still a couple of test classes to fix left in there, but this already removes a lot of cruft, makes things more maintainable, and will make it easier to clean the others up later.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| i_can_no_has_horrible_test.patch | 16.15 KB | Idle | Passed: 7275 passes, 0 fails, 0 exceptions | View details |
#1
"I can no has horrible test"? That sounds like me speaking English :)
Care to elaborate why you are removing the full TaxonomyVocabularyFunctionsTestCase?
Oh, and tests should comply with the Test writers guidelines.
#2
1. It's been merged into TaxonomyVocabularyTestCase() - it's possible I missed something it tests, in which case I'll happily add that back in. If you spotted something glaring please let me know.
2. I'll take a look :)
I reckon I can remove two of the other test cases and just move a little bit of each into TaxonomyTermTestCase - might try to do that later on today.
Also, taxonomy_test.test should be merged into taxonomy.test I think. So CNW.
#3
Alright, lots more cleanup. Leaving the merging of taxonomy_test.test to another issue since it'll be a big cut and paste between files, and make this one harder to review.
#4
VocabularyUnitTest instead of VocabularyTestCase
#5
Added a functional test for taxonomy vocabularies - Damien expressed concern in irc that the patch might reduce test coverage. Although the test is much, much shorter, I'm pretty sure we test exactly the same amount of stuff - just more concisely. It'll also make it a lot easier to add new tests, which I'll need to do with future taxonomy cleanup patches.
#6
Found an indentation issue, added more comprehensive tests for synonyms.
#7
The test coverage results for November 9, 2008 report the following:
We could commit this patch and check back on the results in a couple of days.
#8
@Dries, that sounds like a good plan. I can keep adding tests to this patch but I'd rather do that after the initial cleanup is committed ;)
#10
Removed a stray space and added a missing assertion message (both via webchick in irc).
#11
I've committed this to CVS HEAD. Thanks. Let's mark it 'code needs review' until the code coverage results have been updated?
#12
Thanks Dries, I'll check them in a day or so. If you don't mind, I'm going to move this to 'active (needs more info)' to move it out of the actual reviews queue (and because testing bot will mark it to CNW next time it tries to apply the patch).
#13
November 9th:
modules/taxonomy/taxonomy.module: 82%
modules/taxonomy/taxonomy.admin.inc: 67%
modules/taxonomy/taxonomy.pages.inc: 51%
November 17th:
modules/taxonomy/taxonomy.module: 81%
modules/taxonomy/taxonomy.admin.inc: 66%
modules/taxonomy/taxonomy.pages.inc: 51%
So seems like we lost 1% coverage from taxonomy.module and taxonomy.admin.inc from the 260 lines of code that were removed. Since there wasn't a big regression, I'm marking this back to fixed. We should get more than that 2% back from #293506: TestingParty08: vocabulary overview + deletion and #336084: Add tests for terms on node preview, so see everyone over there. Not too shabby :)
#14
Automatically closed -- issue fixed for two weeks with no activity.