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

  • Patch
  • Review

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
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

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

David_Rothstein’s picture

Issue tags: +Novice

Tentatively adding the Novice tag, though it may or may not turn out to be ;)

David_Rothstein’s picture

Title: Forward-port tests for taxonomy terms being retaind after node edit » Forward-port tests for taxonomy terms being retained after node edit

Fixing typo...

anksy’s picture

Status: Active » Needs review
FileSize
837 bytes

I am not so sure about the tests that I have written. But still here's the patch.

Status: Needs review » Needs work

The last submitted patch, 3: drupal-core-forward-port-tests-taxonomy-2259501-3.patch, failed testing.

anksy’s picture

Status: Needs work » Needs review
FileSize
838 bytes

Fixed some bugs in the patch.

Status: Needs review » Needs work

The last submitted patch, 5: forward_port_tests_for-2259501-5.patch, failed testing.

star-szr’s picture

@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:

       $term_objects[$key] = reset($term_objects[$key]);
     }
 
+    // Test editing the node.
+    $node = $this->drupalGetNodeByTitle($edit["title"]);
+    $this->drupalPost('node/' . $node->nid . '/edit', $edit, t('Save'));
+    foreach ($terms as $term) {
+      $this->assertText($term, 'The term was retained after edit and still appears on the node page.');
+    }
+
     // Delete term 1.
     $this->drupalPost('taxonomy/term/' . $term_objects['term1']->tid . '/edit', array(), t('Delete'));
     $this->drupalPost(NULL, NULL, t('Delete'));
     $term_names = array($term_objects['term2']->name, $term_objects['term3']->name);

The top line of the context exists verbatim in the D8 test code.

anksy’s picture

Status: Needs work » Needs review
FileSize
795 bytes

@Cottser Thanks for the hint!
Repositioned the code.

Status: Needs review » Needs work

The last submitted patch, 8: forward_port_tests_for-2259501-8.patch, failed testing.

anksy’s picture

anksy’s picture

Status: Needs work » Needs review
star-szr’s picture

Status: Needs review » Needs work

Looking 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.

+++ b/core/modules/taxonomy/src/Tests/TermTest.php
@@ -274,11 +283,8 @@ function testNodeTermCreationAndDeletion() {
-
-    // Get the node.
-    $node = $this->drupalGetNodeByTitle($edit['title[0][value]']);
+    ¶
     $this->drupalGet('node/' . $node->id());
-

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.

anksy’s picture

Status: Needs work » Needs review
FileSize
1.25 KB
848 bytes

Fixed the points raised by Cottser in #12 .

star-szr’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Just 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.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Test improvements are not blocked in beta. Committed 0253813 and pushed to 8.0.x. Thanks!

  • alexpott committed 0253813 on 8.0.x
    Issue #2259501 by anksy: Forward-port tests for taxonomy terms being...

Status: Fixed » Closed (fixed)

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