Revealed via #1373142: Use the Testing profile. Speed up testbot by 50%
TaxonomyTermTestCase::testNodeTermCreationAndDeletion() produces random test failures.
The test asserts the JSON response of the autocomplete page callback.
It creates 3 terms:
$terms = array(
$this->randomName(),
$this->randomName() . ', ' . $this->randomName(),
$this->randomName(),
);
$edit[$instance['field_name'] . "[$langcode]"] = drupal_implode_tags($terms);
$this->drupalPost('node/add/article', $edit, t('Preview'));
The terms are then loaded via:
// Get the created terms.
list($term1, $term2, $term3) = array_values(taxonomy_term_load_multiple(FALSE));
My suspicion is that the random term names are re-ordered/sorted alphabetically somewhere in the autocomplete functionality.
Attached patch does not fix the bug, merely verbosely exposes the actual values for debugging.
Related issues
#1000736: Term reference autocomplete widget having problems handling terms with commas
Comment | File | Size | Author |
---|---|---|---|
#22 | d7-tax-test-from-comment-11.patch | 3.75 KB | xjm |
#16 | drupal8.taxonomy-term-test-random-failure.16.patch | 2.57 KB | sun |
#12 | drupal8.taxonomy-term-test-random-failure.12.patch | 1.12 KB | sun |
#11 | even-less-magic.patch | 3.77 KB | xjm |
#7 | no-magic-term-order.patch | 698 bytes | xjm |
Comments
Comment #1
sundrupal8.taxonomy-term-test-random-failure.0.patch queued for re-testing.
Comment #2
sunSo @beejeebus seems to have found a workaround/solution in #1373142-36: Use the Testing profile. Speed up testbot by 50%, essentially:
While that resolves the random test failure, I'm still not sure why the terms are not loaded in the order they are saved.
Comment #3
xjmThere's this issue that went in recently: #479368: D7: Create RFC compliant HTML safe JSON
Comment #4
sunThe JSON encoding is correct though. It's really just the order of loaded terms that were previously saved via tagging/autocomplete.
Comment #5
xjmYeah, was just clarifying since catch referenced it in the other issue. I was mistaken about the other autocomplete patches going in; I forgot that one got reverted. Nonetheless, there's these open issues about the autocomplete:
And these about term ordering:
I bet something in that second list is related to the issue here. Haven't looked closely yet.
Comment #6
xjmSo I haven't been able to reproduce the random failures from this issue using today's 7.x-dev build and the standard profile. I'm not sure why the bug would not show up then... however, maybe we should just be loading the terms by name anyway, since we have the term names? The test does not seem particularly robust as it is.
Comment #7
xjmHow about something like this?
Comment #8
sunThanks, that looks most robust to me.
After all, it's not the purpose nor in the scope of that test case to verify the order of saved/loaded terms, so this fix is appropriate.
Comment #10
xjmRight, need a reset() on there.
Comment #11
xjmI think this is a bit cleaner. I also actually bothered to run it locally this time. ;)
Comment #12
sunI guess I can beat that with still less, but more smart magic. :)
Comment #13
Anonymous (not verified) CreditAttribution: Anonymous commentedmagic battle - sun vs xjm - fight! ;-)
i guess i prefer #12. simple patch, so i'll RTBC if it comes back green.
Comment #14
Anonymous (not verified) CreditAttribution: Anonymous commentedComment #15
catchHmm, I think I prefer #11, 'cos we only have four calls to extract() in all of core, and it'd be a shame to add another one even in a test. Back to CNR for a bit.
Comment #16
sunAlright, I hope you won't count the number of $$var throughout core :P ;)
If you will, then let's go with #11 :)
Comment #17
catchOh I completely missed the $$var.
Can't decide which is worse now ;)
Comment #18
sunTo clarify: There are three approaches to choose from now ;)
#11 (objects in array), #12 (extract()), and #16 ($$var)
Comment #19
xjm$$var--
, ewww.I've used
extract()
in my own code I am fine with it personally. #11 is nice in that it is self-documenting, but it's also more typing. ;) So either #11 or #12 is fine by me. Leaning toward #12.Bikeshed? Us? Never!
Comment #20
sun@catch or @Dries, can you just simply pick one? Let's move on. :)
Comment #21
catchYep, went for #11. Committed/pushed to 8.x, will need a re-roll for 7.x.
Comment #22
xjmDette er rerollen.
Comment #23
sunTestbot queue hiccup. Re-queued #22 manually. Guess it'll come back green, so RTBC.
Comment #24
webchickYay for fewer random test failures! The code gets a bit more obfuscated now, but the trade-off seems worth it.
Committed and pushed to 7.x. Thanks!
Comment #26
cweagansUpdating tags per http://drupal.org/node/1517250
Comment #27
sunComment #27.0
sunUpdated issue summary.