Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Spin-off from Testbot/PIFR issue: #1674290: PSR-0 tests in contrib modules are not found
Problem
- run-tests.sh does not allow to run all tests of a certain module. (see PIFR issue linked above)
- run-tests.sh unconditionally retrieves all test case classes available in a Drupal site, even when not needed.
Goal
run-tests.sh --module node
- Minimize dependencies on an existing/installed Drupal [parent] site. (to be continued in follow-up issues)
Related issues
Comment | File | Size | Author |
---|---|---|---|
#15 | drupal8.run-tests-disabled.15.patch | 2.61 KB | sun |
#13 | drupal8.run-tests-disabled.13.patch | 2.54 KB | sun |
#9 | drupal8.run-tests-register-disabled.9.patch | 509 bytes | sun |
drupal8.run-tests-module.0.patch | 4.54 KB | sun | |
Comments
Comment #1
sunComment #2
sunYes. This patch removes all sanity checks for the passed script parameters. That's no mistake.
A shell script should fail prominently and visibly. There is no point at all in hiding errors.
Comment #3
sunThis patch is RTBC from my perspective.
I've manually tested the possible arguments already and there's no regression. The new --module argument also works as expected.
Comment #4
Niklas Fiekas CreditAttribution: Niklas Fiekas commentedMaybe (if we ever encounter a problem) we can do better than that, by actually parsing the file (http://de2.php.net/manual/en/function.token-get-all.php), but it should be solid enough generally.
I don't get why this had $all_tests in the first place. So yeah.
Looks good to me. Thanks sun!
Comment #5
sunThe
--file
option actually looks obsolete to me, and I'd rather try to remove it entirely in a later, separate step. Right now, PIFR actually solely uses that option only, so removing it will require some additional orchestration.Comment #6
Niklas Fiekas CreditAttribution: Niklas Fiekas commentedRight. With PSR-0 there is not much flexibility for the filename while holding on namespace + class name.
Comment #7
Dries CreditAttribution: Dries commentedCommitted to 8.x. Thanks. Moving to 7.x.
Comment #8
sun#1700682-47: run-tests.sh attempts to run abstract/base test classes revealed:
I object to calling simpletest_classloader_register() though. We need to inline that function instead. The ultimate goal is to get rid of the simpletest.module dependency.
Comment #9
sunAttached patch is the quick-fix for that bug.
Comment #10
jthorson CreditAttribution: jthorson commentedViews 8.x-3.x test with the above patch fails with the following:
ReflectionException: Class Drupal\views\Tests\Style\PluginStyleJumpMenuTest does not exist in ReflectionClass->__construct() (line 557 of /var/lib/drupaltestbot/sites/default/files/checkout/core/scripts/run-tests.sh).
Comment #11
jthorson CreditAttribution: jthorson commentedAnd, at risk of polluting this thread even further, so does Chessboard.
Comment #12
xjmConfirmed this also does not resolve the problem using the script manually locally.
Comment #13
sunDue to the sub-processes being spawned by run-tests.sh, this was a bit more complex to figure out.
Should even give a nice performance boost, at least for contributed projects. At least for me, the total time for a single test case decreased by 2 seconds.
Comment #15
sunStill doesn't work correctly, but wanted to provide the latest code.
Comment #16
sunThe follow-up patch seems to be obsolete. In any case, if there's anything that needs a follow-up, let's create a separate issue.