Problem/Motivation
There is a wrong assertion in taxonomy.test that always evaluates to TRUE.
$this->assertTrue(count($terms4 == 4), 'Correct number of terms were loaded.');
This is always TRUE. It should at least be count($terms4) == 4
. However, it would be better to replace the assertion with assertEqual()
: $this->assertEqual(count($terms4), 4, ...)
.
Proposed resolution
- Change the mentioned line of code to
assertEqual()
and fix thecount()
. - Look for other places in the file that could be done using assertEqual() instead of assertTrue() to simplify the code.
Remaining tasks
This should only be done once both #1160566: entity_load() is actually entity_load_multiple() and #1195254: Taxonomy test cleanup have been commited (the the second already has been commited to 8.x, only needs to be commited to 7.x, the first will not be backported) because both patches currently touch that line or come too close so that it will need a re-roll.
Comment | File | Size | Author |
---|---|---|---|
#19 | drupal-1542674-19.patch | 3.58 KB | tim.plunkett |
#17 | drupal-1542674-17.patch | 3.58 KB | tim.plunkett |
#12 | 1542674-12.patch | 3.59 KB | bas.hr |
#7 | 1542674-7.patch | 3.59 KB | bas.hr |
#2 | 1542674-d7.patch | 684 bytes | damiankloip |
Comments
Comment #0.0
xjmUpdated issue summary.
Comment #1
xjmUpdated the summary.
Comment #2
damiankloip CreditAttribution: damiankloip commentedHere is a D7 patch for this.
Comment #4
damiankloip CreditAttribution: damiankloip commentedIgnore the last patch, too premature.
Comment #5
xjmLooks like it was just rolled against D7. :) It needs to be fixed in D8 first.
Comment #6
damiankloip CreditAttribution: damiankloip commentedYep, I got it all wrong on this one I'm afraid :)
Comment #7
bas.hr CreditAttribution: bas.hr commentedHere is a D8 patch with assertEqual instead of assertTrue where appropriate.
Comment #8
bas.hr CreditAttribution: bas.hr commentedComment #9
damiankloip CreditAttribution: damiankloip commentedIsn't this issue still waiting on other issues above?
Comment #10
Berdir#7: 1542674-7.patch queued for re-testing.
Comment #12
bas.hr CreditAttribution: bas.hr commentedRe-rolled to apply after #1160566: entity_load() is actually entity_load_multiple() has been commited
Comment #13
BerdirLooks good to me.
Comment #14
Dries CreditAttribution: Dries commentedNice catch, and good clean-up. Committed to 8.x. Moving to 7.x.
Comment #15
tim.plunkettIt doesn't look like this was pushed to D8 yet.
Comment #16
tim.plunketthttp://drupalcode.org/project/drupal.git/commit/ae015c9
Comment #17
tim.plunkettRerolled.
Comment #19
tim.plunkettI mismatched the parenthesis on that weird count() line.
Comment #20
xjmLooks correct.
Comment #21
webchickwow, nice catch. committed and pushed to 7.x. thanks!
Comment #22.0
(not verified) CreditAttribution: commentedUpdated issue summary.