Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
See #1525176-192: Using array_diff_assoc() for multilevel arrays in DrupalDefaultEntityController->cacheGet() which wound up adding some tests to Drupal 7 that were addressing the (Drupal-7-only) bug at hand but are generically-useful enough that they could go into Drupal 8 too.
The tests in question are the ones in modules/taxonomy/taxonomy.test from that patch.
Proposed resolution
Add the tests!
Remaining tasks
PatchReview
Beta phase evaluation
Issue category | Task, just adding more test coverage for a D7 bug to try and catch potential future regressions. |
---|---|
Unfrozen changes | Unfrozen because it only adds automated tests. |
Prioritized changes | This is not a prioritized change for the beta phase. |
Disruption | None |
User interface changes
n/a
API changes
n/a
Comment | File | Size | Author |
---|---|---|---|
#13 | interdiff-10.txt | 848 bytes | anksy |
#13 | forward_port_tests_for-2259501-13.patch | 1.25 KB | anksy |
#10 | forward_port_tests_for-2259501-10.patch | 1.62 KB | anksy |
#8 | forward_port_tests_for-2259501-8.patch | 795 bytes | anksy |
#5 | forward_port_tests_for-2259501-5.patch | 838 bytes | anksy |
Comments
Comment #1
David_Rothstein CreditAttribution: David_Rothstein commentedTentatively adding the Novice tag, though it may or may not turn out to be ;)
Comment #2
David_Rothstein CreditAttribution: David_Rothstein commentedFixing typo...
Comment #3
anksy CreditAttribution: anksy commentedI am not so sure about the tests that I have written. But still here's the patch.
Comment #5
anksy CreditAttribution: anksy commentedFixed some bugs in the patch.
Comment #7
star-szr@anksy thanks for working on this! I think you want to aim the code a bit lower, look at the context of the D7 patch for a hint:
The top line of the context exists verbatim in the D8 test code.
Comment #8
anksy CreditAttribution: anksy commented@Cottser Thanks for the hint!
Repositioned the code.
Comment #10
anksy CreditAttribution: anksy commentedComment #11
anksy CreditAttribution: anksy commentedComment #12
star-szrLooking better, thanks for keeping at this :) For those of you following along at home we have been chatting about this issue today on IRC in #drupal-google.
Tab character introduced here (Dreditor highlights it), doesn't match up with coding standards: https://www.drupal.org/coding-standards#indenting
I think we should leave the space (blank line) underneath the
$this->drupalGet()
intact.Comment #13
anksy CreditAttribution: anksy commentedFixed the points raised by Cottser in #12 .
Comment #14
star-szrJust compared back with the D7 version, and the comments don't match up exactly with the D7 patch, but I think that's fine. This looks good to me and meshes with the way the tests are set up in D8.
Updated the issue summary and added some beta evaluation data.
Comment #15
alexpottTest improvements are not blocked in beta. Committed 0253813 and pushed to 8.0.x. Thanks!