Implement dataset parallelization with -f flag - #755
Conversation
Slamdunk
left a comment
Slamdunk
left a comment
There was a problem hiding this comment.
Thank you for proposing this. A couple of changes are required 💪
| $files[$file] = null; | ||
|
|
||
| if ($test instanceof TestCase) { | ||
| $name = str_replace(' with data set ', '', $test->nameWithDataSet()); |
There was a problem hiding this comment.
Unfortunately this doesn't work for both numeric and string data names.
PHPUnit\Runner\Filter\Factory needs:
testName#1for numeric namestestName with data set "foo"for string names
There was a problem hiding this comment.
looks like we could just use $name = $test->nameWithDataSet());
There was a problem hiding this comment.
@Guite no: nameWithDataSet outputs testDataProvider1 with data set #0 for numeric names, and that doesn't work with --filter
| putenv(Options::ENV_KEY_UNIQUE_TOKEN . '=' . $prevUniqueToken); | ||
| $_SERVER[Options::ENV_KEY_TOKEN] = $prevToken; | ||
| $_SERVER[Options::ENV_KEY_UNIQUE_TOKEN] = $prevUniqueToken; | ||
| $last = null; |
There was a problem hiding this comment.
Please restore the original code here, and add a new dedicated test for the --functional option, with both numeric data names and string data names
|
Looking forward to this one. |
|
Would love to see this feature back again :) |
|
Help @danog resolving the requested changes and we are ready to go 💪 |
|
Whoops sorry completely forgot about this PR, will apply the required changes this weekend! |
|
If you think this is feature complete please leave a comment. I will then re-test it and report our experience. |
|
Yeah it's ready, I just can't figure out why are the tests failing (with different errors when running on local, too) |
| $this->bareOptions['-f'] = true; | ||
|
|
||
| $runnerResult = $this->runRunner(); | ||
| self::assertEquals(0, $runnerResult->exitCode); |
There was a problem hiding this comment.
Currently this test is poorly written: executing ./bin/paratest --no-configuration -f test/fixtures/function_parallelization_tests/FunctionalParallelizationTest.php only runs 3 tests instead of 6, because of #755 (comment)
Please:
- Add a specific assertion on the number of tests expected to be run, six
- Compose the test name with
\PHPUnit\Framework\TestCase::dataNameAPI, and use#forintand@forstring
|
Hi @danog I've tried to check PR, it seems the count is wrong |
|
This PR needed some few more tweaks that I addressed in #770 (I could have done it here, if the changes were presented as a separate branch in @danog fork):
I've also removed the support for But all in all, thank you for this PR 💪 |
5 participants
Brings back the feature that was removed in the phpunit 10 update.