Problem/Motivation

#2870194: Ensure that process-isolated tests can use Symfony's PHPunit bridge to catch usages of deprecated code gave us a way to have deprecations fail tests for all test types. It also added a --suppress-deprecations flag to run-tests.sh.

This flag is not present in the 8.4.x version of run-tests.sh.

Since it's not present, we can't allow for it in the testbot. The testbot should not be making exceptions for various core versions, because that causes the testbot to be less maintainable.

Proposed resolution

Add --suppress-deprecations to run-tests.sh for core 8.4.x. This only has to support turning off the phpunit-bridge, which, as of 8.4.x, only supports unit tests.

How to test:

  1. Remove @group legacy from \Drupal\Tests\link\Unit\Plugin\migrate\cckfield\LinkCckTest
  2. Run php ./core/scripts/run-tests.sh --color --class '\Drupal\Tests\link\Unit\Plugin\migrate\cckfield\LinkCckTest' - it fails
  3. Run php ./core/scripts/run-tests.sh --suppress-deprecations --color --class '\Drupal\Tests\link\Unit\Plugin\migrate\cckfield\LinkCckTest' - it passes

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#4 2927636-3.patch1.65 KBalexpott
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Mile23 created an issue. See original summary.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented
Related issues: +#2926314: Contrib should run with --suppress-deprecations

This issue is blocking #2926314: Contrib should run with --suppress-deprecations

Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
CreditAttribution: Mixologic at Drupal Association commented

Lets just add the option, and have it NOOP. we just need it to exist, but it wont actually do anything.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott as a volunteer and at Acro Commerce commented
Status: Active » Needs review
FileSize
2927636-3.patch1.65 KB

Here's all we need.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott as a volunteer and at Acro Commerce commented

This patch was created by doing git co 8.5.x core/scripts/run-tests.sh

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott as a volunteer and at Acro Commerce commented
Issue summary: View changes
Issue tags: +Needs manual testing
Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented
Status: Needs review » Reviewed & tested by the community

I reviewed this by modifying Drupal\Tests\Core\EntityReferenceSelection\EntityReferenceSelectionUnitTest (at random) to remove @group legacy and @expectedDeprecation.

Then I ran run-tests.sh and verified that the test fails:

$ php ./core/scripts/run-tests.sh --sqlite foo.sqlite --types PHPUnit-Unit --concurrency 10 --url http://localhost:8888/ --browser  entity_reference

Drupal test run
---------------

Tests to be run:
  - Drupal\Tests\Core\EntityReferenceSelection\EntityReferenceSelectionUnitTest

Test run started:
  Thursday, December 14, 2017 - 23:41

Test summary
------------

Drupal\Tests\Core\EntityReferenceSelection\EntityReferenceSe   0 passes   1 fails                            

Test run duration: 2 sec

Browser output shows that the fail was caused by the deprecation:

OK (8 tests, 11 assertions)
Remaining deprecation notices (5) Providing settings under 'handler_settings' is deprecated and will be removed before 9.0.0. Move the settings in the root of the configuration array. See https://www.drupal.org/node/2870971: 5x 3x in EntityReferenceSelectionUnitTest::testSetConfiguration from Drupal\Tests\Core\EntityReferenceSelection 1x in EntityReferenceSelectionUnitTest::testSetConfigurationBcLevel from Drupal\Tests\Core\EntityReferenceSelection 1x in EntityReferenceSelectionUnitTest::testDeprecationErrorTriggering from Drupal\Tests\Core\EntityReferenceSelection

Applying the patch in #4, I re-ran the test without adding --suppress-deprecations and got the same fail.

Adding --suppress-deprecations I get this:

$ php ./core/scripts/run-tests.sh --sqlite foo.sqlite --types PHPUnit-Unit --concurrency 10 --url http://localhost:8888/ --browser --suppress-deprecations entity_reference

Drupal test run
---------------

Tests to be run:
  - Drupal\Tests\Core\EntityReferenceSelection\EntityReferenceSelectionUnitTest

Test run started:
  Thursday, December 14, 2017 - 23:43

Test summary
------------

Drupal\Tests\Core\EntityReferenceSelection\EntityReferenceSe   8 passes                                      

Test run duration: 2 sec

So I'm saying right on, this works.

Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
CreditAttribution: Mixologic at Drupal Association commented

+1

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch as a volunteer commented
Status: Reviewed & tested by the community » Fixed

Theoretically we want the actual functionality in 8.4.x for unit tests, it's just no-one really noticed that support since they're used less than kernel/functional tests. Patch looks fine so committed bfe2a01 and pushed to 8.4.x. Thanks!

  • catch committed bfe2a01 on 8.4.x 
    Issue #2927636 by alexpott, Mile23, Mixologic: Backport --supress-...
Mile23’s picture

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

Added to change record https://www.drupal.org/node/2811561

Status: Fixed » Closed (fixed)

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