Skip to content

Dogfood --report-azdo-summary and --report-azdo-progress in CI#8934

Open
Evangelink wants to merge 1 commit into
mainfrom
dev/amauryleve/dogfood-cli-options
Open

Dogfood --report-azdo-summary and --report-azdo-progress in CI#8934
Evangelink wants to merge 1 commit into
mainfrom
dev/amauryleve/dogfood-cli-options

Conversation

@Evangelink
Copy link
Copy Markdown
Member

Summary

Wires the recently-merged --report-azdo-summary and --report-azdo-progress AzureDevOpsReport options into both testfx CI test paths so we dogfood them on every run.

Both options are gated on TF_BUILD=true (AzureDevOpsSummaryReporter.cs#L99, AzureDevOpsProgressReporter.cs#L97) so they are a no-op for local dotnet test runs and only emit ##vso[...] commands inside an AzDO pipeline. Smoke-tested locally with Microsoft.Testing.Platform.UnitTests to confirm the options are accepted and produce the expected …skipping… notice outside AzDO.

Changes

  • test/Directory.Build.targets — extends the always-on --report-azdo injection (Debug -p:UsingDotNetTest=true path).
  • azure-pipelines.yml — extends the Release --test-modules script (Windows).
  • eng/pipelines/steps/test-non-windows.yml — same extension for the Linux/macOS template.

Deferred to follow-up PRs

Considered (and rejected for now) — happy to do these as separate PRs if there is interest:

Option Why deferred
--report-html API is [Experimental(""TPEXP"")] and the test apps disable GenerateTestingPlatformEntryPoint, so each of ~15 Program.cs would need an explicit AddHtmlReportProvider() call plus a TPEXP suppression. Worth doing, but a different change.
--report-azdo-upload-artifacts files All modules share one --results-directory, so the uploader's GetFiles(..., AllDirectories) would emit N² uploads. Needs per-module --results-directory isolation or precise include patterns first.
--minimum-expected-tests Redundant — MTP already returns ExitCode.ZeroTests when zero tests run.
--retry-failed-tests Changes test semantics; should be rolled out deliberately.
--publish-azdo-* / --report-azdo-flaky-history / --report-azdo-quarantine-file Larger Arcade-integration work.

Enable the recently merged AzDO reporting options across both the Debug
(-p:UsingDotNetTest=true) and Release (--test-modules) test paths so the
testfx CI exercises them on every run.

Both options no-op outside AzDO (gated on TF_BUILD).

Deferred to follow-up PRs (with reasons):
- --report-html: experimental TPEXP API; requires AddHtmlReportProvider() in
  ~15 Program.cs files since test apps disable GenerateTestingPlatformEntryPoint.
- --report-azdo-upload-artifacts files: shared --results-directory would cause
  N^2 uploads; needs per-module isolation or precise include patterns.
- --minimum-expected-tests: redundant with MTP's default ZeroTests exit code.
- --retry-failed-tests: changes test semantics; needs careful rollout.
- --publish-azdo-* / flaky history / quarantine: bigger Arcade integration changes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 8, 2026 15:10
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR wires the Azure DevOps Report extension’s new --report-azdo-progress and --report-azdo-summary switches into the repo’s CI test invocations so these reporters are exercised on every pipeline run (Debug -p:UsingDotNetTest=true path and Release --test-modules path).

Changes:

  • Inject --report-azdo-progress and --report-azdo-summary into the common TestingPlatformCommandLineArguments for test projects.
  • Extend the Windows (azure-pipelines.yml) and non-Windows (eng template) Release test scripts to pass the new switches.
  • Add inline documentation in the pipeline YAMLs describing the new switches.
Show a summary per file
File Description
test/Directory.Build.targets Adds --report-azdo-progress and --report-azdo-summary to the standard test-host command line args (alongside --report-azdo).
eng/pipelines/steps/test-non-windows.yml Updates Release dotnet test --test-modules invocation to pass the new AzDO reporter switches and documents them.
azure-pipelines.yml Updates Windows Release dotnet test --test-modules invocation to pass the new AzDO reporter switches and documents them.

Copilot's findings

  • Files reviewed: 3/3 changed files
  • Comments generated: 2

Comment on lines +31 to +34
# --report-azdo-progress — emits ##vso[task.progress] records so long-running test sessions
# show a live progress bar on the AzDO timeline. No-op outside AzDO (gated on TF_BUILD).
# --report-azdo-summary — writes a Markdown job summary and uploads it via ##vso[task.uploadsummary]
# so failure context shows up on the build's summary tab. No-op outside AzDO (gated on TF_BUILD).
Comment thread azure-pipelines.yml
Comment on lines +114 to +117
# --report-azdo-progress — emits ##vso[task.progress] records so long-running test sessions
# show a live progress bar on the AzDO timeline. No-op outside AzDO (gated on TF_BUILD).
# --report-azdo-summary — writes a Markdown job summary and uploads it via ##vso[task.uploadsummary]
# so failure context shows up on the build's summary tab. No-op outside AzDO (gated on TF_BUILD).
@Evangelink
Copy link
Copy Markdown
Member Author

🧪 Test quality grade — PR #8934

No new or modified test methods were identified in the changed regions
of this PR. Nothing to grade.

Re-run with /grade-tests.

Generated by Grade Tests on PR (on open / sync) for issue #8934 · sonnet46 659.1K ·

Copy link
Copy Markdown
Member Author

@Evangelink Evangelink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

🤖 This review was generated by GitHub Copilot (workflow run).

# Dimension Verdict
20 Build Infrastructure 🟡 1 MODERATE

✅ 21/22 dimensions clean (dimensions 1–14, 16, 18–19, 21–22 are N/A or clean for this purely CI-config change).

  • Build Infrastructure — --report-azdo-progress and --report-azdo-summary emit output-device warnings when TF_BUILD is absent; injecting them unconditionally in test/Directory.Build.targets adds warning noise to every local dotnet test run.

Overall: The intent is clear and the implementation is correct for CI. The three-file change is tightly scoped, comments are accurate in their CI-only context, and parity between the Windows and Linux/macOS Release paths is maintained. The single concern is the local-developer experience regression described in the inline comment.

Generated by Expert Code Review (on open) for issue #8934 · sonnet46 3.5M

<TestingPlatformCommandLineArguments Condition=" $([MSBuild]::GetTargetFrameworkIdentifier('$(TargetFramework)')) == '.NETCoreApp' ">$(TestingPlatformCommandLineArguments) --crashdump</TestingPlatformCommandLineArguments>
<TestingPlatformCommandLineArguments>$(TestingPlatformCommandLineArguments) --hangdump --hangdump-timeout 15m</TestingPlatformCommandLineArguments>
<TestingPlatformCommandLineArguments>$(TestingPlatformCommandLineArguments) --report-azdo</TestingPlatformCommandLineArguments>
<TestingPlatformCommandLineArguments>$(TestingPlatformCommandLineArguments) --report-azdo --report-azdo-progress --report-azdo-summary</TestingPlatformCommandLineArguments>
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MODERATE] Build Infrastructure

Unlike --report-azdo (which is a pure IDataConsumer with no session lifecycle hooks and is truly silent outside AzDO), both --report-azdo-progress and --report-azdo-summary implement ITestSessionLifetimeHandler. Their OnTestSessionStartingAsync emits a warning on the output device whenever the option is set but TF_BUILD is not "true":

// AzureDevOpsSummaryReporter.OnTestSessionStartingAsync — fires every local run:
await _outputDevice.DisplayAsync(this, new WarningMessageOutputDeviceData(AzureDevOpsResources.SummaryRequiresTfBuildWarning), ...);

// AzureDevOpsProgressReporter.OnTestSessionStartingAsync — same:
await _outputDevice.DisplayAsync(this, new WarningMessageOutputDeviceData(AzureDevOpsResources.ProgressRequiresTfBuildWarning), ...);

Because this PropertyGroup is unconditional, every test project will emit two extra warnings on every local dotnet test run. With the number of test projects in this repo, that adds up to significant console noise that developers may find confusing.

Recommendation: Consider gating the two new options on the TF_BUILD environment variable at the MSBuild level, the same way --crashdump is gated on the TFM:

<TestingPlatformCommandLineArguments>$(TestingPlatformCommandLineArguments) --report-azdo</TestingPlatformCommandLineArguments>
<TestingPlatformCommandLineArguments Condition=" '$(TF_BUILD)' == 'true' ">$(TestingPlatformCommandLineArguments) --report-azdo-progress --report-azdo-summary</TestingPlatformCommandLineArguments>

MSBuild inherits environment variables as properties, so $(TF_BUILD) resolves correctly inside an AzDO build. This keeps --report-azdo (which is silent outside AzDO) on its own unconditional line and limits the two noisier options to CI runs only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants