See #2996789-19: Deprecate Drupal\field\Tests\EntityReference\EntityReferenceTestTrait.

CommentFileSizeAuthor
#12 3001958-12.patch4.32 KBWim Leers
#12 interdiff.txt1.66 KBWim Leers
#10 3001958-10.patch4.42 KBWim Leers
#10 interdiff.txt2.13 KBWim Leers
#4 3001958-4.patch4.35 KBWim Leers
#4 interdiff.txt5.98 KBWim Leers
#2 3001958-2.patch3.32 KBWim Leers
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Wim Leers created an issue. See original summary.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Assigned: Unassigned » Wim Leers
Status: Active » Needs review
FileSize
3001958-2.patch3.32 KB

Two classes in JSON API use this trait:

  1. \Drupal\Tests\jsonapi\Functional\InternalEntitiesTest
  2. \Drupal\Tests\jsonapi\Functional\JsonApiFunctionalTestBase

For the first, we can just add @requires function \Drupal\Tests\field\Traits\EntityReferenceTestTrait::createEntityReferenceField. That means it won't run on Drupal 8.5 anymore, but the probability of that breaking is absolutely miniscule.

For the second, I'd say we do the same, but … class \Drupal\Tests\jsonapi_extras\Functional\JsonExtrasApiFunctionalTest extends JsonApiFunctionalTestBase . IOW: we'd make it impossible for JSON API Extras to still run that test against Drupal 8.5. Plus, class class JsonApiFunctionalTest extends JsonApiFunctionalTestBase is also pretty important, and frequently caught regressions before commit, so should still run on 8.5.
Hence for that class, I'm just copy/pasting the contents of the trait into the base class.

Status: Needs review » Needs work

The last submitted patch, 2: 3001958-2.patch, failed testing. View results

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Needs work » Needs review
FileSize
interdiff.txt5.98 KB
3001958-4.patch4.35 KB

Well that didn't work. Apparently my only option is duplicating that trait until we require Drupal 8.7.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Assigned: Wim Leers » Unassigned
Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented

See: #3002011: Remove @trigger_error() for 8.6.x Drupal\field\Tests\EntityReference\EntityReferenceTestTrait deprecation

gabesullice’s picture

gabesullice
Primary language English
CreditAttribution: gabesullice at Acquia commented
Related issues: +#3002011: Remove @trigger_error() for 8.6.x Drupal\field\Tests\EntityReference\EntityReferenceTestTrait deprecation

Testing it now.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Title: 4 test fails due to using deprecated code on 8.6 and 8.7 since #2996789 » 4 test fails due to using deprecated code on 8.6 and 8.7 since #2996789: temporarily fork the test trait

Status: Needs review » Needs work

The last submitted patch, 4: 3001958-4.patch, failed testing. View results

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Needs work » Needs review
FileSize
interdiff.txt2.13 KB
3001958-10.patch4.42 KB

Well, now 8.6 passes tests, but 8.7 still doesn't. And I can't fix it on 8.7 without breaking it for 8.5. So … #3002011: Remove @trigger_error() for 8.6.x Drupal\field\Tests\EntityReference\EntityReferenceTestTrait deprecation did not actually help.

So, updating this patch so it passes CS testing.

gabesullice’s picture

gabesullice
Primary language English
CreditAttribution: gabesullice at Acquia commented 
+++ b/tests/src/Functional/InternalEntitiesTest.php
@@ -7,8 +7,8 @@ use Drupal\Core\Entity\EntityInterface;
+use Drupal\Tests\jsonapi\Traits\EntityReferenceTestTrait;

We have a "ForwardCompatibility" namespace. Can we have a "BackwardCompatibility" namespace for this?

Otherwise, this is RTBC.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Needs review » Reviewed & tested by the community
FileSize
interdiff.txt1.66 KB
3001958-12.patch4.32 KB

Excellent idea!

  • Wim Leers committed 23f6f12 on 8.x-2.x 
    Issue #3001958 by Wim Leers, gabesullice, Mile23: 4 test fails due to...
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Reviewed & tested by the community » Fixed
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented

This unblocked #3001564: Follow-up to #2997600: Clean up dead code paths and #2986900: Unnecessary asserts break installation in distros that include JSON API.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented

This also triggered the creation of #3002148: Support deprecation testing for multiple branches on contributed modules on Drupal.org, which will help prevent this problem from occurring in the future.

Status: Fixed » Closed (fixed)

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