Fix CI failures: include OpenTelemetry in NonWindowsTests.slnf and register Ctrf/JUnit providers in AOT test host#9038
Conversation
…gister Ctrf/JUnit providers in AOT test host Two distinct test failures on main (build 2997792): 1. Linux `Tests` job failed because `Microsoft.Testing.Extensions.OpenTelemetry` was not listed in `NonWindowsTests.slnf` and therefore was not packed by the non-Windows build; the new `SdkTests` data row added in #8903 then could not restore the package (NU1101). Add the project to the slnf so it ships on Linux/macOS. 2. Windows Release `Test` job failed because `azure-pipelines.yml` runs every UnitTests host with `--report-ctrf` and `--report-junit` via the `--test-modules` glob, but `MSTest.AotReflection.SourceGeneration.UnitTests/Program.cs` did not register the CTRF or JUnit providers, so MTP rejected the unknown options. Register both providers to match `MSTest.SourceGeneration.UnitTests`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes two CI breakages in the microsoft/testfx repo by ensuring (1) the OpenTelemetry extension project is built/packed in non-Windows CI, and (2) the AOT reflection unit test host registers the CTRF and JUnit report providers so the pipeline-injected --report-ctrf / --report-junit flags are recognized.
Changes:
- Add
Microsoft.Testing.Extensions.OpenTelemetrytoNonWindowsTests.slnfso the package is produced during non-Windows CI builds. - Register
AddJUnitReportProvider()andAddCtrfReportProvider()inMSTest.AotReflection.SourceGeneration.UnitTestshost to accept--report-junit/--report-ctrf.
Show a summary per file
| File | Description |
|---|---|
| test/UnitTests/MSTest.AotReflection.SourceGeneration.UnitTests/Program.cs | Registers JUnit + CTRF report providers in the AOT unit test host so MTP recognizes the corresponding CLI options. |
| NonWindowsTests.slnf | Ensures the OpenTelemetry extension project is included in the non-Windows solution filter so its NuGet is built/packed in CI. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 0
Evangelink
left a comment
There was a problem hiding this comment.
Note
🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.
Summary
Two targeted CI hotfixes:
NonWindowsTests.slnf— addsMicrosoft.Testing.Extensions.OpenTelemetryalphabetically betweenMSBuildandRetry, allowing the LinuxTestsjob to build and pack the project so acceptance tests that reference it can restore it locally.MSTest.AotReflection.SourceGeneration.UnitTests/Program.cs— registersAddJUnitReportProvider()andAddCtrfReportProvider(), aligning this host with every otherEnableMSTestRunner=trueunit-test host. The binaries for both extensions are already pulled in for all such hosts viatest/Directory.Build.targets(lines 48 & 52), so no.csprojchange is needed.
Verdict table
| # | Dimension | Status | Notes |
|---|---|---|---|
| 1 | Algorithmic Correctness | ✅ PASS | Both root causes are correctly identified and addressed |
| 2 | Threading & Concurrency | ✅ N/A | No shared mutable state involved |
| 3 | Public API Surface & Versioning | ✅ N/A | No new public API introduced |
| 4 | Backward Compatibility | ✅ PASS | Purely additive changes |
| 5 | Test Coverage & Quality | ✅ PASS | Infrastructure fix; validated by CI passing after the change |
| 6 | Error Handling & Resilience | ✅ N/A | Registration calls are fire-and-forget by design |
| 7 | Performance & Memory | ✅ N/A | No performance-sensitive code paths touched |
| 8 | Security | ✅ N/A | No security surface affected |
| 9 | Code Style & Conventions | ✅ PASS | Provider registration order matches MSTest.SourceGeneration.UnitTests/Program.cs exactly |
| 10 | Localization | ✅ N/A | No user-facing strings |
| 11 | Naming | ✅ N/A | No new identifiers |
| 12 | Documentation | ✅ N/A | No public API; no misleading comments |
| 13 | Scope & Focus | ✅ PASS | Two closely related CI-fix items; appropriate to batch |
| 14 | Cross-TFM Correctness | ✅ PASS | AOT reflection project targets net8.0 only; change is TFM-agnostic |
| 15 | Dependency Management | ✅ PASS | OpenTelemetry was already in TestFx.slnx and in Directory.Build.targets; slnf was the only gap |
| 16 | Build & CI Integration | ✅ PASS | Alphabetical placement in slnf is correct; pipeline glob *UnitTests.exe now finds a fully registered host |
| 17 | IPC Contract Stability | ✅ N/A | No wire-format changes |
| 18 | Resource Management | ✅ N/A | No IDisposable / stream usage |
| 19 | Test Infrastructure | ✅ PASS | AOT reflection host now honours every --report-* flag the pipeline passes |
| 20 | Configuration & Extensibility | ✅ N/A | Standard MTP provider registration pattern |
| 21 | Pattern Consistency | ✅ PASS | Registration order (TrxReport → JUnit → AppInsights → AzDO → Ctrf) matches the peer host; AddOpenTelemetryProvider is omitted here as in the host before this PR — see note below |
| 22 | Simplicity | ✅ PASS | Minimal, surgical changes |
Notes
AddOpenTelemetryProvider not wired up (informational, not blocking)
test/Directory.Build.targets (line 57) already adds a ProjectReference to Microsoft.Testing.Extensions.OpenTelemetry for every EnableMSTestRunner=true project, including this one. MSTest.SourceGeneration.UnitTests/Program.cs takes advantage of that by calling AddOpenTelemetryProvider(...) to dogfood the OTel pipeline end-to-end in CI. MSTest.AotReflection.SourceGeneration.UnitTests/Program.cs does not — and that was already the case before this PR. The omission doesn't cause any CI failure (unlike CTRF/JUnit, the OTel provider doesn't need to be registered to avoid "unknown option" errors), so it's reasonable to leave it out of this hotfix. A follow-up to add parity would be welcome but is not required here.
Overall verdict
✅ LGTM — both fixes are correct, minimal, and consistent with codebase conventions. No blocking or major issues found.
🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review (on PR ready) workflow. · 447.1 AIC · ⌖ 12.5 AIC · ◷
🧪 Test quality grade — PR #9038No new or modified test methods were identified in the changed regions Re-run with
|
Fixes two distinct test failures observed on
mainin internal build 2997792.1. Linux
Testsjob —NU1101: Unable to find package Microsoft.Testing.Extensions.OpenTelemetryThe new
SdkTestsdata row added in #8903 (commit c451c6b) exercises the OpenTelemetry extension, butMicrosoft.Testing.Extensions.OpenTelemetrywas not listed inNonWindowsTests.slnf. The non-Windows CI builds and packs only the projects listed in this slnf, so the package was never produced. The acceptance test does not passaddPublicFeeds: true, so the package could not be restored from nuget.org either.Fix: Add
Microsoft.Testing.Extensions.OpenTelemetrytoNonWindowsTests.slnf(alphabetically placed betweenMSBuildandRetry).2. Windows Release
Testjob —Unknown option '--report-ctrf'/'--report-junit'azure-pipelines.ymlinvokes every UnitTests host found by the--test-modulesglob with--report-ctrfand--report-junit.MSTest.AotReflection.SourceGeneration.UnitTests/Program.cswas missing the corresponding provider registrations, so MTP rejected the unknown options and aborted the run. Other UnitTests hosts (e.g.,MSTest.SourceGeneration.UnitTests/Program.cs) already register both providers.Fix: Add
builder.AddCtrfReportProvider()andbuilder.AddJUnitReportProvider()calls to the AOT reflection test host.Validation
dotnet build test\UnitTests\MSTest.AotReflection.SourceGeneration.UnitTests\MSTest.AotReflection.SourceGeneration.UnitTests.csproj -c Release→ 0 warnings, 0 errors.MSTest.AotReflection.SourceGeneration.UnitTests.exe --helpnow lists--report-ctrf/--report-ctrf-filename/--report-junit/--report-junit-filename.--report-ctrf --report-junit --list-testsreturns the proper provider validation error (instead ofUnknown option), confirming the providers are registered.Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com