[WIP] Trying to filter what tests run based on PR contents#65565
Draft
BrennanConroy wants to merge 18 commits intomainfrom
Draft
[WIP] Trying to filter what tests run based on PR contents#65565BrennanConroy wants to merge 18 commits intomainfrom
BrennanConroy wants to merge 18 commits intomainfrom
Conversation
github-actions
bot
added
the
area-infrastructure
BrennanConroy
force-pushed
the
brecon/selectivetesting
branch
from
794eac3 to
7c5296c
Compare
The build step generates files in the source tree, causing subsequent git diff to detect additional changed areas. This leads to the helix step including test projects (Framework, JSInterop) that weren't restored by the build step, causing NETSDK1004 errors. Fix: Write the computed areas to artifacts/tmp/AffectedTestAreas.txt on first computation, then subsequent build steps reuse the cached result. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
build.cmd uses 'PowerShell' (WinPS 5.1), where Join-Path only accepts 2 positional parameters. Three-argument Join-Path requires PS 6+. Nested Join-Path calls work in both PS 5.1 and 7+. Verified locally under PowerShell 5.1 with StrictMode and ErrorActionPreference Stop. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
wtgodbe
reviewed
Mar 2, 2026
eng/Build.props
Outdated
|
|
||
| <ItemGroup Condition=" '$(AffectedTestAreas)' != '' AND '$(BuildMainlyReferenceProviders)' != 'true' "> | ||
| <!-- Subset 1 area filtering --> | ||
| <ProjectsWithTestsSubset1 Remove="$(RepoRoot)src\Framework\**\*" Condition=" !$(_AffectedTestAreasWrapped.Contains(';Framework;')) " /> |
Member
There was a problem hiding this comment.
This could probably be generalized by folding AffectedTestAreas into an Item & then doing something like
<ProjectsWithTestsSubset1 Remove="$(RepoRoot)src\%(AffectedTestAreasItem.Identity)\**\*"
wtgodbe
reviewed
Mar 2, 2026
eng/build.sh
Outdated
| if [ $pwsh_exit -eq 0 ] && [ -f "$output_file" ]; then | ||
| areas=$(cat "$output_file") | ||
| if [ -n "$areas" ]; then | ||
| export AffectedTestAreas="$areas" |
Member
There was a problem hiding this comment.
Using an environment variable feels a little fragile, maybe instead of writing to a .txt file we could write to a .props file, then conditionally import it in Directory.Build.Props/.Targets
eng/Build.props
Outdated
| <ItemGroup Condition=" '$(AffectedTestAreas)' != '' AND '$(BuildMainlyReferenceProviders)' != 'true' "> | ||
| <!-- Subset 1 area filtering --> | ||
| <ProjectsWithTestsSubset1 Remove="$(RepoRoot)src\Framework\**\*" Condition=" !$(_AffectedTestAreasWrapped.Contains(';Framework;')) " /> | ||
| <ProjectsWithTestsSubset1 Remove="$(RepoRoot)src\Caching\**\*" Condition=" !$(_AffectedTestAreasWrapped.Contains(';Caching;')) " /> |
Member
There was a problem hiding this comment.
This will mean we also only build affected projects in all build jobs - which is probably what we want, but worth noting
| { | ||
| continue; | ||
| } | ||
| // if (hubProtocol.GetType().CustomAttributes.Where(a => a.AttributeType.FullName == "Microsoft.AspNetCore.SignalR.Internal.NonDefaultHubProtocolAttribute").Any()) |
Member
There was a problem hiding this comment.
Is this just a dummy change to confirm this worked?
CodeCheck already uses git diff to detect stale generated files. Instead of a separate verify script that duplicates the generation logic, just re-run GenerateAffectedProjectAreas.ps1 and let the existing diff check catch any changes. This matches the pattern used for ProjectReferences.props. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…stAreas Instead of passing AffectedTestAreas via environment variable (which doesn't persist across process boundaries), write a .props file to artifacts/tmp/AffectedTestAreas.props and import it from Build.props. This is more reliable and follows MSBuild conventions. The build scripts now just ensure the .props file is generated once per CI job. MSBuild picks it up automatically via conditional Import. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Instead of hardcoding 36 Remove lines per area in Build.props, the GetAffectedTestAreas.ps1 script now generates the .props file with Remove lines for each unaffected src/ area. This auto-adapts when new areas are added. The DotNetProjects ItemGroup was moved out of the Otherwise block to after the Choose, so the imported .props can filter the subsets before they flow into DotNetProjects and ProjectToBuild. Verified locally: SignalR change filters 521 -> 179 projects, Http change filters 521 -> 444, baseline (no .props) stays at 521. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
GetAffectedTestAreas.ps1 handles shallow clones by fetching just the target branch tip with --depth=1 when origin/main is missing. A full clone is unnecessary and wastes time downloading the entire history. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…vidual items The ItemGroup moved outside the Choose block had a blanket condition 'BuildMainlyReferenceProviders != true' that prevented ProjectToBuild from ever being populated when CodeGen.proj runs with BuildMainlyReferenceProviders=true. This made all generated files (ProjectReferences.props, ShippingAssemblies.props, etc.) empty. Move the condition to only the test subset DotNetProjects lines, leaving ProjectToBuild/ProjectToExclude unconditional so they work for both normal builds and code generation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ArtifactsDir is set by the Arcade SDK which loads after Build.props evaluates, so it's always empty at import time. RepoRoot is set by Common.props which Build.props imports first, so it's always available. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
|
General consideration - this PR deals in changes to src/*, but we also want to make sure we always run all tests on PRs that touch |
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
2 participants
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.
No description provided.