#2869120: run-tests.sh ignores classes if they have whitespace before the declaration using --directory was committed without a test. This means we can't discover whether future changes will be regressions.
TestDiscovery already has a scanDirectory() method which has very few dependencies, but which requires that we have already built a class loader namespace map, which we don't have at that level of run-tests.sh, and might be something we're trying to avoid during the --directory scan.
Move the code block dealing with --directory to a class named something like DirectoryTestFinder.
This code can then be unit tested using vfsStream or other mocking, according to the many test cases written in that code block's inline comments:
// Extract test case class names from specified directory.
// Find all tests in the PSR-X structure; Drupal\$extension\Tests\*.php
// Since we do not want to hard-code too many structural file/directory
// assumptions about PSR-0/4 files and directories, we check for the
// minimal conditions only; i.e., a '*.php' file that has '/Tests/' in
// its path.
// Ignore anything from third party vendors.
// '/Tests/' can be contained anywhere in the file's path (there can be
// sub-directories below /Tests), but must be contained literally.
// Case-insensitive to match all Simpletest and PHPUnit tests:
// ./lib/Drupal/foo/Tests/Bar/Baz.php
// ./foo/src/Tests/Bar/Baz.php
// ./foo/tests/Drupal/foo/Tests/FooTest.php
// ./foo/tests/src/FooTest.php
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | interdiff.txt | 1.28 KB | mile23 |
| #11 | 2871289_10.patch | 9.13 KB | mile23 |
| #4 | 2871289_4.patch | 9.13 KB | mile23 |
Comments
Comment #2
mile23Comment #3
mile23Comment #4
mile23Try this on.
I got most of the way through before I realized I was working on 8.3.x. Unfortunately, the patch doesn't apply to 8.4.x, so if this is too disruptive we can move it to 8.4.x.
It's almost a bug report since #2869120: run-tests.sh ignores classes if they have whitespace before the declaration using --directory happened without a test.
Comment #6
mile23Comment #7
berdirThe same is true about everything in run-tests.sh. Do we really want to completely refactor that into testable code when we plan to eventually replace it completely with phpunit anyway?
Comment #8
mile23If there's a plan to remove run-tests.sh I haven't seen an issue about it.
I have seen these though: #2866082: [Plan] Roadmap for Simpletest #2867818: Create version of run-tests.sh tuned for running PHPUnit #2626986: [meta] Improvements to run-tests.sh
Comment #9
MixologicWe cannot replace run-tests.sh with phpunit unless we're willing to accept a few of regressions:
1. When a phpunit test has a segfault, and either php or apache core dumps, testing just stops, and you will not know exactly which test it is that caused it to crash.
2. We need a way to list all of the testing *files* that are included so that we can know if a patch or commit has made changes to tests, that way have the ability to run just changed tests first.
3. Tests will take about 32 times as long to run, because phpunit does not offer a way to efficiently parallelize tests across multiple processors or multiple machines. They will also cost 32x as much.
Thats not to say we cant refactor run-tests.sh into a tiny library that only has those few things as functionality, just that we cannot use pure phpunit all on its own without giving up those features, and #3 is pretty much non-optional.
Comment #10
mile23Updated to fix CS errors.
Not sure what to do about the NOWDOC indentation problem.
Comment #11
mile23Ugh. Missed the files. :-)
Comment #21
mile23The patch in #11 no longer applies. The reason is that Drupal core now uses this line to do the directory discovery, after #3035312: Move file_scan_directory() to file_system service:
That is, the system under test is discovering the tests.
That means that while there's probably test coverage for the file_system service, it also means that the test coverage for the file_system service is potentially discovered by the file_system service.
Therefore it's impossible to ascertain whether the goal of test coverage for run-test's
--directoryfeature is met, though we could probably just go on assuming our quality is high without any data, which seems to be The Drupal Way.It's also unclear whether anyone really wants to go about gathering the data which would help us understand the quality of the product, so I'm going to set this issue as Active. If anyone wants to champion this cause they can pick it up.