Problem/Motivation

We need a way to just run all javascript tests, this is solved by #2659100: Allow run-tests.sh to run just the javascript Functional tests, but at the same time, we need a way to skip javascript tests

Proposed resolution

Add a --types=... parameter to run-tests.sh

So you can run all javascript tests using javascript:
run-tests.sh --types=PHPUnit-FunctionalJavascript

Remaining tasks

User interface changes

API changes

Data model changes

Members fund testing for the Drupal project. Drupal Association Learn more

Comments

dawehner created an issue. See original summary.

dawehner’s picture

Status: Active » Needs review
FileSize
12.13 KB

There we go.

dawehner’s picture

Title: Allow to exclude groups when running all tests » Allow to exclude types when running all tests
Issue summary: View changes
FileSize
15.37 KB
9.14 KB

Adapted the code to use types instead of groups.

dawehner’s picture

Expanded to also add a explicit command to just render a certain type

dawehner’s picture

More work was needed here.

Status: Needs review » Needs work

The last submitted patch, 5: 2670978-5.patch, failed testing.

dawehner’s picture

Ups, this patch was the wrong way round.

alexpott’s picture

Status: Needs work » Needs review
alexpott’s picture

Status: Needs review » Needs work
  1. +++ b/core/scripts/run-tests.sh
    @@ -292,6 +297,8 @@ function simpletest_script_parse_args() {
    +    'types' => [],
    

    Missing --types documentation

  2. +++ b/core/scripts/run-tests.sh
    @@ -894,7 +905,7 @@ function simpletest_script_get_test_list() {
    -      $groups = simpletest_test_get_all($args['module']);
    +      $groups = simpletest_test_get_all($args['module'], $args['types'], $args['exclude-types']);
    

    Why should the types only work with --module and --all? Also tbh I think we should only have --types and make it work in conjunction with --all.

dawehner’s picture

Status: Needs work » Needs review
FileSize
16.4 KB
7.17 KB

Why should the types only work with --module and --all?

Good point, well, I was probably lazy

alexpott’s picture

Status: Needs review » Needs work
+++ b/core/scripts/run-tests.sh
@@ -208,6 +208,11 @@ function simpletest_script_help() {
+              run-tests.sh --all --types=

I don't think the --all is required and the example needs to include what --types= should be and I guess it should give an example where it is multiple types.

dawehner’s picture

Status: Needs work » Needs review
FileSize
16.42 KB
488 bytes

Ha, I indeed stopped editing in the middle of it.

jibran’s picture

Nice work @alexpott and @dawehner this looks ready to me.

+++ b/core/modules/simpletest/simpletest.module
@@ -506,8 +507,8 @@ function simpletest_log_read($test_id, $database_prefix, $test_class) {
+function simpletest_test_get_all($extension = NULL, array $types = []) {

Do we typehint array now?

jibran’s picture

Nice work @alexpott and @dawehner this looks ready to me.

+++ b/core/modules/simpletest/simpletest.module
@@ -506,8 +507,8 @@ function simpletest_log_read($test_id, $database_prefix, $test_class) {
+function simpletest_test_get_all($extension = NULL, array $types = []) {

Do we typehint array now?

dawehner’s picture

Why not, its an array.

dawehner’s picture

Title: Allow to exclude types when running all tests » Allow to run just specific types when running all tests
Issue summary: View changes

Adapted the title using the new approach.

jibran’s picture

Why not, its an array.

Given that we ran into array typehinting issue in #2664290: Remove array typehints from batch callbacks. I think we should re-consider typehinting as array here as well.

+++ b/core/lib/Drupal/Component/Utility/NestedArray.php
@@ -349,4 +349,26 @@ public static function mergeDeepArray(array $arrays, $preserve_integer_keys = FA
+    $array = is_callable($callable) ? array_filter($array, $callable) : array_filter($array);
+    foreach ($array as &$element) {

This means that after the filtering if we have any array element left apply filter on those. Either we should document it explicitly or we should use third param $before = TRUE to filter given array before or after the nested filtering.

jibran’s picture

For point 2 clarification

  public static function filter(array $array, callable $callable = NULL, $before = TRUE) {
    if ($before) {
     $array = is_callable($callable) ? array_filter($array, $callable) : array_filter($array);
    }
    foreach ($array as &$element) {
      if (is_array($element)) {
        $element = static::filter($element, $callable);
      }
    }
    if (!$before) {
     $array = is_callable($callable) ? array_filter($array, $callable) : array_filter($array);
    }

    return $array;
  }
dawehner’s picture

Given that we ran into array typehinting issue in #2664290: Remove array typehints from batch callbacks. I think we should re-consider typehinting as array here as well.

Well, I doubt entirely that drush will replace the core run-tests.sh --types parameter. Arrays as typehints isn't that uncommon:

$ ag "function" | grep "array " | wc -l
    2435
jibran’s picture

Status: Needs review » Reviewed & tested by the community

After the discussion with @dawehner we agreed the point 1 is not valid here and expanding filter function is out of scope so this patch is ready imo.

xjm’s picture

+++ b/core/lib/Drupal/Component/Utility/NestedArray.php
@@ -349,4 +349,26 @@ public static function mergeDeepArray(array $arrays, $preserve_integer_keys = FA
+

+++ b/core/modules/simpletest/src/TestDiscovery.php
@@ -115,11 +138,11 @@ public function registerTestNamespaces() {
+   * @return array An array of tests keyed by the first
+   * @code

I think this is not a complete sentence?

dawehner’s picture

I think this is not a complete sentence?

Not a problem of this issue, but sure, let's fix it.

isntall’s picture

Is there a list of all the --types that can be used?

isntall’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
16.44 KB
466 bytes

run-test.sh doesn't work with an equal sign, and the capital 'T' in SimpleTest should be lowercased.

isntall’s picture

When testing the patch, I noticed that inputing an invalid type did not result in an error, we might want to have another class written to check if the type is valid.

dawehner’s picture

It is a good question whether we should hardcode the list of available types, so we can actually know which types exists and by that validate it.

isntall’s picture

On the DrupalCI side, since the types are being include and not excluded, we need to hardcode the types that are being tested, and those types will need to be updated everytime they are changed.

DCI will allow D.o to send over the types via the DCI_TestGroups parameter, but then d.o will need to know what they are.

dawehner’s picture

On the DrupalCI side, since the types are being include and not excluded, we need to hardcode the types that are being tested, and those types will need to be updated everytime they are changed.

I totally agree, well after alex denied to add --exclude-types. I guess for core itself, hardcoding is valid as well. We don't add new ones every here and now

Mixologic’s picture

Status: Needs review » Reviewed & tested by the community

putting this back to RTBC. Tests check out.

  • alexpott committed 9683891 on 8.2.x
    Issue #2670978 by dawehner, isntall, jibran, alexpott: Allow to run just...

  • alexpott committed 6afe359 on 8.1.x
    Issue #2670978 by dawehner, isntall, jibran, alexpott: Allow to run just...
alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 220e86d and pushed to 8.0.x, 8.1.x, and 8.2.x. Thanks!

diff --git a/core/lib/Drupal/Component/Utility/NestedArray.php b/core/lib/Drupal/Component/Utility/NestedArray.php
index 995211f..85b4bef 100644
--- a/core/lib/Drupal/Component/Utility/NestedArray.php
+++ b/core/lib/Drupal/Component/Utility/NestedArray.php
@@ -353,7 +353,7 @@ public static function mergeDeepArray(array $arrays, $preserve_integer_keys = FA
    * Filters a nested array recursively.
    *
    * @param array $array
-   *   The filtered nested array
+   *   The filtered nested array.
    * @param callable|NULL $callable
    *   The callable to apply for filtering.
    *
diff --git a/core/modules/simpletest/tests/src/Unit/TestInfoParsingTest.php b/core/modules/simpletest/tests/src/Unit/TestInfoParsingTest.php
index 21dfad3..fda12f1 100644
--- a/core/modules/simpletest/tests/src/Unit/TestInfoParsingTest.php
+++ b/core/modules/simpletest/tests/src/Unit/TestInfoParsingTest.php
@@ -314,7 +314,7 @@ public function testGetTestClasses() {
           'name' => 'Drupal\Tests\test_module\Functional\FunctionalExampleTest',
           'description' => 'Test description',
           'group' => 'example',
-          'type' => 'PHPUnit-Functional'
+          'type' => 'PHPUnit-Functional',
         ],
       ],
       'example2' => [
@@ -322,13 +322,13 @@ public function testGetTestClasses() {
           'name' => 'Drupal\Tests\test_module\Functional\FunctionalExampleTest2',
           'description' => 'Test description',
           'group' => 'example2',
-          'type' => 'PHPUnit-Functional'
+          'type' => 'PHPUnit-Functional',
         ],
         'Drupal\Tests\test_module\Kernel\KernelExampleTest3' => [
           'name' => 'Drupal\Tests\test_module\Kernel\KernelExampleTest3',
           'description' => 'Test description',
           'group' => 'example2',
-          'type' => 'PHPUnit-Kernel'
+          'type' => 'PHPUnit-Kernel',
         ],
       ],
     ], $result);
@@ -358,7 +358,7 @@ public function testGetTestClassesWithSelectedTypes() {
           'name' => 'Drupal\Tests\test_module\Kernel\KernelExampleTest3',
           'description' => 'Test description',
           'group' => 'example2',
-          'type' => 'PHPUnit-Kernel'
+          'type' => 'PHPUnit-Kernel',
         ],
       ],
     ], $result);

A few very minor code style nits fixed on commit.

isntall’s picture

The new DCI tag has been deployed that takes advantage of this new functionality.

Status: Fixed » Closed (fixed)

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