[Testing] Reset source locator after each Rector test - #8513
Merged
TomasVotruba merged 3 commits intoSep 23, 2026
Merged
Conversation
Member
|
Thanks for the PR. |
GErpeldinger
force-pushed
the
reset-source-locator-after-test
branch
from
September 23, 2026 14:04
08ed128 to
5281ad7
Compare
Contributor
Author
|
Done: the first commit only adds the test, and CI fails on it on PHP 8.4 and 8.5 (https://github.com/rectorphp/rector-src/actions/runs/35871477885): The 🤖 Drafted with AI assistance. |
Member
|
LGTM, lets give this a go. Thanks |
Member
|
I ran perf test locally and there is no difference in performance. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DynamicSourceLocatorProviderkeeps the temporary file of the lastdoTestFile().tearDown()deletes that file, so the next test class in the same process that sets no paths itself, e.g.MultilineTestorPhpDocInfoPrinterTest, looks up names in a deleted file:This warning is in 42 of the 70 failed test runs since 2026-08-14 (see #8512 (comment)). It never fails a run, but it is noise next to the real failure.
tearDown()now resets the provider. The first commit only addsResetSourceLocatorTest, and CI fails on it.Third commit:
ClassDependencyManipulatorTestsets the PHP version to 8.1 and never resets it. When it runs beforeAddOverrideAttributeToOverriddenPropertiesRectorTestin the same process, the parent property gets no#[\Override], because the rule needs PHP 8.5. This is the order-dependent failure of #8464 and #8465: the cause is this parameter, not reflection.mainat 1d0224b already failed this way with 3 workers, and the new test class of this PR makes it fail with 4: see the CI run of the first commit.#8349 resets the PHP version in
AbstractRectorTestCase::tearDownAfterClass(), butClassDependencyManipulatorTestextendsAbstractLazyTestCase, so it now resets it in its owntearDown(). To reproduce:With both fixes, the full suite passes with no
Failed to open streamwarning for every worker count from 2 to 12. Onmainat 1d0224b, 7 of these 11 runs have the warning, and the 3-worker run fails.🤖 Drafted with AI assistance.