gh-109817: Add --single-process-per-case option to libregrtest - #151689
gh-109817: Add --single-process-per-case option to libregrtest#151689Aniketsy wants to merge 4 commits into
Conversation
|
i'll fix these |
|
I'm a little busy now, but I'll review it after finishing with other unittest/regrtest tasks. Thank you for your efforts. |
…ase in its own worker process
no hurry, i'm improving this till then, so hopefully it creates less pain for you to review 😊 . I've added tests, with some improvement (locally everything works fine for me) . i'll once again go through this and check for improvement areas before you come here . |
serhiy-storchaka
left a comment
serhiy-storchaka
left a comment
There was a problem hiding this comment.
Thank you for working on this.
It looks like the code does what the issue asked. But I suspect it can be simplified.
| @@ -94,19 +94,47 @@ def list_cases(tests: TestTuple, *, | |||
| test_dir: StrPath | None = None) -> None: | |||
| support.verbose = False | |||
| set_match_tests(match_tests) | |||
There was a problem hiding this comment.
set_match_tests() is called both in list_cases() and in collect_cases().
| with self.lock: | ||
| self.tests_iter = None | ||
|
|
||
| class GroupedMultiprocessIterator: |
There was a problem hiding this comment.
Cannot MultiprocessIterator be used instead? These classes look very similar.
| finally: | ||
| self.test_name = _NOT_RUNNING | ||
|
|
||
| mp_result = dataclasses.replace( |
There was a problem hiding this comment.
mp_result.result is mutable, it can be mutated in-place (see _run_flat()).
| finally: | ||
| self.output.put(WorkerThreadExited()) | ||
|
|
||
| def _run_grouped(self) -> None: |
There was a problem hiding this comment.
_run_grouped() looks very similar to _run_flat() (especially if unify iterators). Could not the share the parametrized body?
thanks for the review , i'll update shortly with the improvement points you shared . |
|
@serhiy-storchaka gentle ping, i've updated with changes, please have a look in this when you get chance and let me know if this needs any further improvement. Thankyou :) |
serhiy-storchaka
left a comment
serhiy-storchaka
left a comment
There was a problem hiding this comment.
Thank you, this is much simpler now.
collect_cases() skips unittest.loader._FailedTest. This was harmless while it only fed --list-cases, but it now decides what actually runs: a module which fails to import produces no cases and silently disappears. Where a normal run reports FAILURE and returns 2, --single-process-per-case reports SUCCESS and returns 0. _FailedTest should be reported rather than skipped.
The skipped list returned by collect_cases() is discarded in _run_tests(), so modules which raise SkipTest on import are not reported as skipped.
Please add a NEWS entry in Misc/NEWS.d/next/Tests/.
2 participants
fixes #109817