Remove support for issues.targets

[jkoritzinsky]

Now that we've removed all entries from issues.targets and the support for the legacy runners that were the primary users, we can remove the infrastructure for issues.targets built into the merged runners.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[stephentoub]

Code Review: PR #123909 - Remove support for issues.targets

Holistic Assessment

Motivation: This PR removes the issues.targets infrastructure that was used to exclude tests at build/run time. Per the PR description, all entries have already been removed and the legacy runners that were the primary consumers have been removed.

Approach: Clean removal of dead code - deleting the exclusion list infrastructure from build scripts, test runners, MSBuild targets, and code generators.

Net positive: ✅ Yes - this is a straightforward cleanup that removes ~300 lines of unused infrastructure.

---

Detailed Findings

✅ Correctness

• The removal is consistent across all touchpoints:
  • Build scripts (.cmd, .sh)
  • MSBuild project files (�uild.proj, helixpublishwitharcade.proj)
  • Code generators (XUnitWrapperGenerator.cs, ITestInfo.cs)
  • Runtime library (TestFilter.cs)
  • Mobile targets (mergedrunnermobile.targets)
  • Shell scripts (�ringup_runtest.sh)
  • Task implementations (PatchExclusionListInApks.cs, TestExclusionListTasks.csproj)

✅ API Changes

• TestFilter constructor signature changes from (string?, Dictionary<string, string>?) to (string?) - this is an internal test infrastructure class, so no public API concerns.
• LoadTestExclusionTable() and GetTestExclusionReason() methods removed from TestFilter - again, internal infrastructure.

✅ Generated Code Changes

• XUnitWrapperGenerator.cs correctly removes:
  • Loading of estExclusionTable
  • Passing exclusion table to TestFilter constructor
  • Exclusion table parameter to XHarnessRunnerLibrary.RunnerEntryPoint.RunTests
• ITestInfo.cs correctly simplifies skip reporting to pass string.Empty instead of computed reason.

⚠️ Potential Issue: Missing XHarnessRunnerLibrary Update

The generator in XUnitWrapperGenerator.cs (line ~465-467 in diff) changes the call:
\\csharp
// Before:
RunTests(RunTests, ""{assemblyName}"", args.Length != 0 ? args[0] : null, testExclusionTable);

// After:
RunTests(RunTests, ""{assemblyName}"", args.Length != 0 ? args[0] : null);
\\

This suggests XHarnessRunnerLibrary.RunnerEntryPoint.RunTests had its signature changed. I don't see this file in the diff - please verify that XHarnessRunnerLibrary has been updated to remove the estExclusionTable parameter, or that this was done in a prior PR.

✅ Build System

• �uild.proj condition change adds '$(__BuildTestWrappersOnly)' != '1' to GenerateLayout target - this appears to be a drive-by fix unrelated to the main change but looks correct.
• Removal of EmitTestExclusionList target and related MSBuild infrastructure is clean.

---

Summary

Verdict: ✅ Approve with minor verification

This is a clean removal of dead infrastructure. The only verification needed is to confirm that XHarnessRunnerLibrary.RunnerEntryPoint.RunTests signature was already updated (likely in a prior PR) to not require the exclusion table parameter.

The changes are well-scoped and consistently applied across all the relevant files.

[stephentoub]

🤖 Copilot Code Review — PR #123909

Holistic Assessment

Motivation: The PR aims to remove the now-obsolete issues.targets test exclusion infrastructure after all entries were migrated to [ActiveIssue] attributes in PR #123248. The motivation is valid and the cleanup is justified.

Approach: The approach of removing the exclusion list infrastructure is correct, but the implementation is incomplete. Critical files were missed, causing compilation failures.

Summary: ❌ Needs Changes. The PR has critical compilation errors that prevent it from building. Several files in XHarnessRunnerLibrary were not updated to match the changes in the code generator, and there are additional references to the exclusion infrastructure throughout the codebase that should be cleaned up.

---

Detailed Findings

❌ Compilation Error — Method Signature Mismatch

The PR modifies XUnitWrapperGenerator.cs to call RunnerEntryPoint.RunTests with only 3 arguments:

builder.AppendLine($@"return await XHarnessRunnerLibrary.RunnerEntryPoint.RunTests(RunTests,"
                   + $@" ""{assemblyName}"","
                   + $@" args.Length != 0 ? args[0] : null);");

However, src/tests/Common/XHarnessRunnerLibrary/RunnerEntryPoint.cs (NOT modified by this PR) still expects 4 arguments:

public static async Task<int> RunTests(
    Func<TestFilter?, TestSummary> runTestsCallback,
    string assemblyName,
    string? filter,
    Dictionary<string, string> testExclusionTable)  // ← This parameter still exists

This causes the generated test runners to fail compilation, which is confirmed by the failing CI checks ("coreclr Common Pri0 Test Build", NativeAOT builds, etc.).

---

❌ Missing File Updates — XHarnessRunnerLibrary

The following files still contain testExclusionTable logic and must be updated:

1. src/tests/Common/XHarnessRunnerLibrary/RunnerEntryPoint.cs:

   • Remove testExclusionTable parameter from RunTests method (line 17)
   • Remove handling of --exclusion-list filter prefix (lines 19-23)
   • Update all platform entry point classes (AppleEntryPoint, AndroidEntryPoint, WasmEntryPoint) to remove testExclusionTable from their constructors and fields

2. src/tests/Common/XHarnessRunnerLibrary/GeneratedTestRunner.cs:

   • Remove _testExclusionTable field (line 18)
   • Remove testExclusionTable constructor parameter (line 26)
   • Update TestFilter instantiation on line 44 to not pass the exclusion table

---

⚠️ Stale Documentation — Help.cs

File: src/tests/Common/XUnitWrapperLibrary/Help.cs

The help text still documents the --exclusion-list=FILE option (lines 35-40) and explicitly references src/tests/issues.targets:

--exclusion-list=FILE
                     Path to a file containing a list of tests to exclude.
                     ...
                     (In the dotnet/runtime repository, this file is typically generated as part of the build process as 'src/tests/issues.targets'.)

This documentation should be removed since the feature no longer exists.

---

💡 Additional Cleanup Opportunities

src/coreclr/tools/r2rtest/TestExclusion.cs (228 lines)

This entire file is dedicated to parsing issues.targets for the R2R test tool. The file contains:

• TestExclusion class for representing exclusions
• TestExclusionMap class for searching exclusions
• XML parsing logic specific to issues.targets format

The file is referenced by BuildFolderSet.cs (ResolveTestExclusions()) and configured via CommandLineOptions.cs (--issues-path). Consider whether this tool still needs this functionality or if it can be removed in this PR or a follow-up.

src/tests/Directory.Build.props (line 178)

The comment still references issues.targets:

<!-- Don't append the RID to the output path for our test tree. Our test builds are already separated by RID
     and including the RID breaks some of our glob expressions in issues.targets -->

This comment should be updated since issues.targets no longer has glob expressions.

---

Summary

This PR cannot be merged in its current state due to critical compilation errors. The required fixes are:

1. ❌ Update RunnerEntryPoint.cs to remove testExclusionTable parameter and related logic
2. ❌ Update GeneratedTestRunner.cs to remove _testExclusionTable field and parameter
3. ⚠️ Remove or update Help.cs documentation for --exclusion-list
4. 💡 Consider cleaning up r2rtest/TestExclusion.cs (can be follow-up)
5. 💡 Update comment in Directory.Build.props (minor)

Review performed by Copilot CLI with multi-model validation (Claude Sonnet 4, GPT-5.1, Gemini 3 Pro). All three models independently identified the same critical issues.

[Copilot]

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 6 comments.

[jkoritzinsky]

@copilot open a new pull request to apply changes based on the comments in this thread

[jkoritzinsky]

Copilot took too long. Just did it myself.