Contrib should run with --suppress-deprecations

I've had a look at the code and it seems sane. Given we have the flag $this->configuration['extension_test'] already and the patch copes with the fact that --suppress-deprecations is only available on 8.5.x in a good way (ie. using the semver dependency) +1 to the approach.

I think it is worthwhile expanding the test coverage because the code is not quite as straightforward as before. Something like this:

diff --git a/tests/DrupalCI/Tests/Plugin/BuildTask/BuildStep/Testing/SimpletestTest.php b/tests/DrupalCI/Tests/Plugin/BuildTask/BuildStep/Testing/SimpletestTest.php
index 37a081a9..23bad150 100644
--- a/tests/DrupalCI/Tests/Plugin/BuildTask/BuildStep/Testing/SimpletestTest.php
+++ b/tests/DrupalCI/Tests/Plugin/BuildTask/BuildStep/Testing/SimpletestTest.php
@@ -17,7 +17,15 @@ class SimpletestTest extends DrupalCITestCase {
     return [
       'core' => [
         'cd exec-container-source-dir && sudo -u www-data php exec-container-source-dir/core/scripts/run-tests.sh --color --keep-results --values=value --all',
-        [],
+        ['core_branch' => '8.5.x'],
+      ],
+      'core-8.5.x-suppress-deprecations' => [
+        'cd exec-container-source-dir && sudo -u www-data php exec-container-source-dir/core/scripts/run-tests.sh --color --keep-results --suppress-deprecations --values=value --all',
+        ['core_branch' => '8.5.x', 'suppress-deprecations' => TRUE],
+      ],
+      'core-8.4.x-suppress-deprecations-flag' => [
+        'cd exec-container-source-dir && sudo -u www-data php exec-container-source-dir/core/scripts/run-tests.sh --color --keep-results --values=value --all',
+        ['core_branch' => '8.4.x', 'suppress-deprecations' => TRUE],
       ],
       'contrib-default' => [
         'cd exec-container-source-dir && sudo -u www-data php exec-container-source-dir/core/scripts/run-tests.sh --color --keep-results --values=value --directory true-extension-subdirectory',

Because the core_branch flag is always set and the code now supports a suppress-deprecations flag (at least for core).

+++ b/src/DrupalCI/Plugin/BuildTask/BuildStep/Testing/Simpletest.php
@@ -141,12 +135,42 @@ class Simpletest extends BuildTaskBase implements BuildStepInterface, BuildTaskI
+    if ($is_extension_test) {
+      // Always add --suppress-deprecations for contrib. getRunTestsFlagValues()
+      // will determine whether to add it based on core version.
+      // @todo Turn this off when some other solution is decided in
+      //   https://www.drupal.org/project/drupal/issues/2607260
+      $this->configuration['suppress-deprecations'] = TRUE;
+    }

I think here we should trust the flag if it is set.

Patch attached does the two things mentioned in this review.

@Mile23 re #9 that's exactly the way it works. It allows a flag to overrule instead of always setting based on $is_extension_test so when we do allow contrib maintainers to configure this via the UI it just works without any code change.

@mile23 Is there an issue to track for the "band-aid" fix? I'm seeing a module I just published have test failures for deprecation notices in its dependencies (https://www.drupal.org/pift-ci-job/827146). Thanks for working on this!

I don't usually re-ping issues but this seems pretty bad - modules I contribute to have failing branch tests and issues cannot get out of Needs Work, which makes review difficult.

I did a bunch of work on this yesterday, I'll be able to deploy soon.

Okay, finally got this massaged into a state where we dont have to do any version dependent twiddling. It just does a basic grep for suppress to gather whether or not the capability exists.

Contrib tests should now be run without failing due to using deprecations.

This is an example of one that had a deprecation warning that no longer does.

https://www.drupal.org/pift-ci-job/836319