fix(test-runner): add orphaned dependency projects with matching --last-failed tests as independent entries#41745
Closed
Git-Raini wants to merge 2 commits into
Closed
fix(test-runner): add orphaned dependency projects with matching --last-failed tests as independent entries#41745Git-Raini wants to merge 2 commits into
Git-Raini wants to merge 2 commits into
Conversation
…ast-failed Previously only top-level projects were checked against post-shard filters like --last-failed before dependency projects got prepended to the root suite. If the only projects with matching tests were classified as dependencies (e.g. because another project lists them in `dependencies`), the top-level projects had no matching tests, got filtered out entirely, and dependencies never got a chance to run, producing "No tests found". Fixes: microsoft#39811
…st-failed tests as independent entries Addresses review feedback on this PR: the previous approach conflated a project's structural dependency role with test filtering by changing the top-level/dependency classification predicate itself, which regressed the existing "dependencies always run in full" guarantee. This instead leaves the classification and existing dependency-prepend logic untouched, and adds a distinct step: after --last-failed filtering, any project that is excluded solely because it is someone else's dependency (and that someone didn't survive filtering) is checked for its own matching tests. If found, it is added as an independent entry point, pulling in its own prerequisites in full - never affecting how already surviving top-level projects' dependencies are run.
Contributor
Author
|
@microsoft-github-policy-service agree |
Collaborator
|
I don't think we are ready for the additional complexity this introduces. See this comment. Closing for now. |
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.
Follow-up to #41622 (closed, I cannot reopen it as a non-collaborator) addressing @yury-s review feedback there.
Fixes #39811
The previous approach (#41622) conflated a project's structural dependency role with test filtering by changing the top-level/dependency classification predicate itself, which risked regressing the existing dependencies-always-run-in-full guarantee.
This instead leaves the classification and existing dependency-prepend logic untouched, and adds a distinct step: after --last-failed filtering, any project excluded solely because it is someone else's dependency (and that someone didn't survive filtering) is checked for its own matching tests. If found, it is added as an independent entry point, pulling in its own prerequisites in full - never affecting how already-surviving top-level projects' dependencies are run.
Added a dedicated regression test for the exact scenario flagged in review: a dependency project needed by a surviving top-level project still runs in full under --last-failed, even with no matching failures of its own.
Full context/discussion: #41622 (comment)