The following needs testing:

Vocabulary overview with no vocabularies.
Changing weights on the vocabulary overview with two or more vocabularies.
Deleting a vocabulary.
When a vocabulary has a weight, check the weight is removed when other vocabularies are deleted.

CommentFileSizeAuthor
#11 vocabulary_admin_293506-11.patch12.48 KBpwolanin
#10 vocabulary_admin.patch4.13 KBcatch
#8 vocabulary_admin.patch4.29 KBcatch
#6 vocabulary_admin.patch4.38 KBcatch
#1 vocabulary_overview.patch4.55 KBforipepe

Comments

foripepe’s picture

foripepe commented
Status: Active » Needs review
StatusFileSize
new vocabulary_overview.patch4.55 KB

Fixed:
- Vocabulary overview with no vocabularies.
- Changing weights on the vocabulary overview with two or more vocabularies.
- Deleting a vocabulary.
The last one was a bug, so we didn't create test for it.
- When a vocabulary has a weight, check the weight is removed when other vocabularies are deleted.

Status: Needs review » Needs work

The last submitted patch failed testing.

foripepe’s picture

foripepe commented
Status: Needs work » Needs review

See: http://pastebin.ca/1258476

Status: Needs review » Needs work

The last submitted patch failed testing.

foripepe’s picture

foripepe commented
Status: Needs work » Needs review

Everything is green. Why did the patch fail?

catch’s picture

catch
he/him
Primary language English
commented
StatusFileSize
new vocabulary_admin.patch4.38 KB

The patch failed due to a recent commit which completely reorganised the taxonomy test, causing your patch not to apply any longer. If the testing system says your patch needs work, it's always worth checking it applies properly on a clean copy of HEAD, and that nothing has changed which might cause the test itself to break.

I re-rolled the patch, and also fixed some comment and code style issues. The test itself looks good and we don't have test coverage for some of this stuff yet.

foripepe’s picture

foripepe commented

Ok, thanks Catch. I understand now. The green fields were a little bit confusing for me... :)

catch’s picture

catch
he/him
Primary language English
commented
StatusFileSize
new vocabulary_admin.patch4.29 KB

Spotted a couple more assertions which needed reworking.

pwolanin’s picture

pwolanin commented

there are a bunch of really not needed calls to drupalGet() and $this->assertResponse('200'); in the last test.

catch’s picture

catch
he/him
Primary language English
commented
StatusFileSize
new vocabulary_admin.patch4.13 KB

Missed those while cleaning up, sorry about that. Also added an additional assertion using taxonomy_get_vocabularies() to ensure that deleting everything from the UI really does work.

pwolanin’s picture

pwolanin commented
StatusFileSize
new vocabulary_admin_293506-11.patch12.48 KB

Seems like there was various common code that should be factored out too - this substantially reduces the LOC to be maintained in the existing tests while still adding all the new tests.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Very nice. taxonomy.test keeps getting smaller :)

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Thanks all! Committed to CVS HEAD. :-)

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.