Run-Tests Cleanup / Modernization #5707
Conversation
dragoonis
force-pushed
the
run-tests-cleanup-1
branch
from
2038a98 to
8fe7690
Compare
|
@KalleZ I've resolved our conversation as I've moved things to snake_case. |
|
Please rebase this PR, so that the first (now committed) commit will be dropped. |
|
@nikic i will. Thanks. |
dragoonis
force-pushed
the
run-tests-cleanup-1
branch
from
8fe7690 to
eafcd0a
Compare
dragoonis
force-pushed
the
run-tests-cleanup-1
branch
from
eafcd0a to
be6e80c
Compare
|
@nikic rebased and ready for merge. |
| /* Let there be no top-level code beyond this point: | ||
| * Only functions and classes, thanks! | ||
| * | ||
| * Minimum required PHP version: 7.0.0 |
There was a problem hiding this comment.
As you are using void, update this comment to say 7.1.0.
| $workerID = intval(getenv("TEST_PHP_WORKER")); | ||
| run_worker(); | ||
| $workerID = get_worker_id(); | ||
| if($workerID) { |
There was a problem hiding this comment.
| if($workerID) { | |
| if ($workerID) { |
| break; | ||
| case 'r': | ||
| case 'l': | ||
| // @Dragoonis |
| return 0; | ||
| } | ||
|
|
||
| return intval(getenv("TEST_PHP_WORKER")); |
There was a problem hiding this comment.
This function can be simplified to (int) getenv("TEST_PHP_WORKER").
|
|
||
| function get_max_workers_from_test_files($workers, $test_files): int | ||
| { | ||
| return max(1, min($workers, count($test_files))); |
There was a problem hiding this comment.
This code seems easier to understand if left inlined.
|
|
||
|
|
||
|
|
||
| function get_number_of_workers($workers): int |
There was a problem hiding this comment.
| function get_number_of_workers($workers): int | |
| function parse_number_of_workers(string $workers): int |
maybe
| $cleanWorkers = intval($cleanWorkers, 10); | ||
| // Don't use parallel testing infrastructure if there is only one worker. | ||
| if ($cleanWorkers === 1) { | ||
| $cleanWorkers = null; |
There was a problem hiding this comment.
This should be either 0, or the return type should be ?int.
| 'SKIPIF', 'XFAIL', 'XLEAK', 'CLEAN', | ||
| 'CREDITS', 'DESCRIPTION', 'CONFLICTS', 'WHITESPACE_SENSITIVE', | ||
| ))) { | ||
| if (is_section_unknown($section)) { |
There was a problem hiding this comment.
Nit: I would prefer if this was !is_section_known($section) or !is_known_section($section).
|
Closing as this is out of date, and contains open questions or otherwise was not responded too when it was reviewed. If you feel strongly I'm wrong, please open a fresh PR against master, linking to this one, that resolves all open questions. Thanks for taking the time ;) |
6 participants
No description provided.