Skip to content

ci: use MTP GitHub Actions report and upload coverage to GitHub - #800

Draft
askpt wants to merge 6 commits into
mainfrom
askpt/mtp-github-report-and-coverage
Draft

ci: use MTP GitHub Actions report and upload coverage to GitHub#800
askpt wants to merge 6 commits into
mainfrom
askpt/mtp-github-report-and-coverage

Conversation

@askpt

@askpt askpt commented Aug 3, 2026

Copy link
Copy Markdown
Member

Stacked on #801. This PR targets askpt/ci-required-job-merge-queue, so the diff below is only this layer. Merge #801 first; GitHub will then retarget this PR to main.

Why

While evaluating whether Microsoft Testing Platform's native GitHub Actions reports could replace GitHubActionsTestLogger, it turned out that neither reporting path was actually working:

  • --report-github, which the build and e2e jobs have been passing for months, is not a real MTP option. It was silently ignored.
  • GitHubActionsTestLogger is a VSTest logger. Since global.json sets "test": { "runner": "Microsoft.Testing.Platform" }, VSTest loggers are never loaded, so the package was dead weight.

Net effect: the repo has had zero GitHub Actions test reporting. This PR makes it real, and adds GitHub's built-in code coverage alongside the existing Codecov upload.

What changed

Test reporting

  • Replaced GitHubActionsTestLogger with Microsoft.Testing.Extensions.GitHubActionsReport, and switched --report-github to the real --report-gh in reusable-build.yml and reusable-e2e.yml. The extension is auto-registered by Microsoft.Testing.Platform.MSBuild, so no code changes are needed, and it only activates when GITHUB_ACTIONS=true (local runs are unaffected).
  • This floats MTP from 2.2.2 to 2.3.3, which is required: --report-gh was introduced in 2.3.0.

Coverage (reusable-coverage.yml)

  • Coverlet now emits both opencover (for Codecov, unchanged) and cobertura from a single test run.
  • Two Linux-only steps merge the per-project cobertura files with ReportGenerator and upload the result via actions/upload-code-coverage. The Codecov steps are untouched and still run on both Windows and Linux.
  • Dropped the now-stale --coverlet-exclude "[GitHubActionsTestLogger*]*" filter.
  • code-quality: write is granted on the coverage job in ci.yml and declared on the called workflow, since a reusable workflow cannot escalate beyond what the caller grants.

Things worth a careful look

  • The extension is a prerelease. Microsoft.Testing.Extensions.GitHubActionsReport is only published as 1.0.0-alpha.* (pinned to 1.0.0-alpha.26377.5). There is no stable release yet. Renovate will not bump it unless prereleases are explicitly allowed for that package, so it may need a manual refresh later.
  • The package reference is deliberately scoped to $(MSBuildProjectName.EndsWith('Tests')) in build/Common.tests.props. Referencing any MTP extension turns a project into an MTP test application, which made dotnet test start discovering and running OpenFeature.Benchmarks and OpenFeature.AotCompatibility. The condition prevents that.
  • ReportGenerator writes absolute paths into the merged report, which would break GitHub's mapping to the PR diff. The merge step rewrites them to repo-relative and normalizes <source>. It also filters **/obj/** to drop source-generated files.
  • The GitHub coverage upload works today. Confirmed on a full CI run before this was stacked: the report uploaded and GitHub reported processing status: succeeded, so Code Quality is already enabled on this repo. The step still runs with fail-on-error: false so a coverage outage can never block CI, and Codecov remains untouched.
  • Known issue in the alpha reporter: the job summary under-reports test counts. For OpenFeature.Tests the summary shows 391 of 437 tests. The dropped tests are the ones that finish last: the summary is written from a snapshot taken roughly 150ms before the run actually ends. Smaller suites (Hosting.Tests 106, MultiProvider.Tests 255, IntegrationTests 10) all report exactly right. This only affects the rendered summary. The exit code and the console output are still accurate, so a failing test can never be hidden from CI gating by this bug.
  • The coverage checkout pins ref: ${{ github.event.pull_request.head.sha || github.sha }} because upload-code-coverage needs the real head commit rather than the merge commit.

Validation

Confirmed on a full CI run of this change (all 25 checks green, before restacking):

  • --report-gh is accepted and the suite runs normally: 2424 tests, 0 failures, 0 build warnings. Under MTP 2.3.3 an unrecognized option is a hard failure, so this also proves the extension is genuinely loaded.
  • The reporter writes a real job summary with a results table and a slowest-tests list, subject to the undercount noted above.
  • ReportGenerator merged the cobertura files into a 500 KB report and actions/upload-code-coverage uploaded it, with GitHub confirming processing status: succeeded.
  • Codecov is unaffected: codecov/patch and codecov/project both passed on Linux and Windows.

Note on CI while stacked

ci.yml filters on pull_request: branches: [main], so once this PR targets askpt/ci-required-job-merge-queue no further pushes here will trigger CI. The run against the rebased head was triggered while the base was still main. CI resumes automatically once #801 merges and this PR is retargeted to main. Worth considering whether #801 should drop that base filter so stacked PRs stay covered.

Still a draft pending a decision on whether the summary undercount is acceptable while the reporter is prerelease.

Copilot AI review requested due to automatic review settings August 3, 2026 10:26
@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b32e4c44-bb83-4318-a0f4-d2b238eb929c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-code-quality

github-code-quality Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: C#

C# / code-coverage/coverlet

The overall coverage in commit 69c22e1 in the askpt/mtp-github-rep... branch is 96%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main askpt/mtp-github-rep... 69c22e1 +/-
src/OpenFeature...eatureClient.cs 100%
src/OpenFeature.../LoggingHook.cs 100%
src/OpenFeature.../MetricsHook.cs 100%
src/OpenFeature...erExtensions.cs 99%
src/OpenFeature/Model/Value.cs 98%
src/OpenFeature...erExtensions.cs 95%
src/OpenFeature...erRepository.cs 95%
src/OpenFeature/Api.cs 94%
src/OpenFeature...ventExecutor.cs 93%
src/OpenFeature...ultiProvider.cs 92%

Updated August 07, 2026 08:27 UTC

@codecov

codecov Bot commented Aug 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.55%. Comparing base (058cf52) to head (69c22e1).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #800   +/-   ##
=======================================
  Coverage   93.55%   93.55%           
=======================================
  Files          66       66           
  Lines        3010     3010           
  Branches      378      378           
=======================================
  Hits         2816     2816           
  Misses        130      130           
  Partials       64       64           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Replaces ineffective VSTest reporting with native MTP GitHub reporting and adds GitHub code coverage alongside Codecov.

Changes:

  • Adds and configures the MTP GitHub Actions reporter.
  • Generates both OpenCover and Cobertura coverage.
  • Merges and uploads repository-relative coverage to GitHub.

Reviewed changes

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

Show a summary per file
File Description
Directory.Packages.props Replaces the obsolete logger package.
CONTRIBUTING.md Documents dual coverage formats.
build/Common.tests.props Adds the reporter to test projects only.
.github/workflows/e2e.yml Enables MTP reporting for E2E tests.
.github/workflows/code-coverage.yml Merges and uploads Cobertura coverage.
.github/workflows/ci.yml Enables MTP reporting in CI.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

askpt added 3 commits August 3, 2026 13:17
`needs:` cannot reference jobs in another workflow file, so a single required
status check means the gated jobs have to live in one workflow. Fold
code-coverage.yml, dotnet-format.yml, aot-compatibility.yml and e2e.yml into
ci.yml and add a `CI Gate` job that depends on all of them.

Once branch protection requires only `DCO` + `CI Gate`, build, test, coverage,
format, E2E and the AOT matrix all become effectively required, and matrix legs
can be added or removed without editing branch protection.

Along the way this fixes several things that were silently broken:

- ci.yml, code-coverage.yml and dotnet-format.yml had no `merge_group` trigger,
  so the merge queue never validated build, test, coverage or format against the
  merged result. They now run there.

- ci.yml and code-coverage.yml used workflow-level `paths-ignore: "**.md"`.
  A workflow skipped by path filtering reports no check runs at all, so a
  required check inside it would leave docs-only pull requests blocked forever.
  The filter is gone.

- The gate uses `if: always()`. Without it the job inherits the implicit
  `success()` condition and is *skipped* when a dependency fails - and branch
  protection treats a skipped check as satisfied, which would let exactly the
  broken pull requests through. The jq predicate also fails on `cancelled` and
  on any skip that is not explicitly allow-listed.

- `github.event.pull_request.head.repo.fork == false` is true on `push` and
  `merge_group` because GitHub coerces the missing value to 0 before comparing.
  Combined with `startsWith(github.ref, 'refs/heads/')`, which matches
  `refs/heads/gh-readonly-queue/*`, adding a merge_group trigger would have
  published a NuGet package for every queue entry. The packaging job now checks
  `github.event_name` explicitly, always packs as a validation step, and
  publishes only on push or a non-fork pull request.

- Merge queue runs are never cancelled by the concurrency group; a cancelled run
  reports a non-success conclusion and ejects the pull request from the queue.

Coverage is skipped on `merge_group` - the `gh-readonly-queue/*` ref is thrown
away, so uploading it to Codecov is meaningless and `build` already runs the
same tests there. It is the only entry in the gate's allow-list.

The E2E job deliberately keeps the bare `e2e-tests` id so its check run name is
unchanged; it is a required context today and a required check that stops
reporting blocks every open pull request.

Duplicated setup-dotnet and NuGet cache blocks move into a local composite
action.

Branch protection still has to be updated after this merges.

Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
Follow-up to the CI Gate consolidation. Rather than one long ci.yml, each unit
of work becomes a `workflow_call` building block and ci.yml is reduced to an
orchestrator that wires them together and evaluates the gate.

  ci.yml
    build      -> reusable-build.yml
    coverage   -> reusable-coverage.yml
    format     -> reusable-format.yml
    e2e        -> reusable-e2e.yml
    aot        -> reusable-aot.yml
    packaging  -> reusable-packaging.yml
    ci-gate    -- the single required status check

The gate still has to live in ci.yml because `needs:` cannot reference a job in
another workflow file. Everything else moves out.

Beyond readability, actionlint cross-validates the calls, so a typo in an input
or secret name becomes a lint error rather than a runtime failure.

The blocks take no inputs. They have exactly one caller in this repository, so
parameters would only ever be passed their own defaults. The one thing that does
have to cross the boundary is the Codecov token: secrets are not visible to a
called workflow unless they are passed explicitly.

One consequence worth calling out: check runs produced through a reusable
workflow are named `<caller job> / <called job>`, so the checks are now
`Build / ubuntu-latest`, `AOT / linux-arm64`, `Format / dotnet format` and so
on. That means the `e2e-tests` context cannot be preserved the way it was in
the previous commit, and branch protection has to drop it before this merges
rather than after. `CI Gate` is unaffected - it is a plain job in ci.yml, so its
name has no prefix and stays stable as jobs move around underneath it.

Reusable workflows are `workflow_call`-only so they never self-trigger, and
concurrency stays in the caller alone - sharing a group between caller and
callee makes a run cancel itself.

Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
The `--report-github` switch used in CI is not a real option. Neither
Microsoft.Testing.Platform, xUnit v3, coverlet.MTP nor the .NET SDK
register it, so it was silently ignored and no annotations or job
summary were ever produced. GitHubActionsTestLogger was dead weight too:
it is a VSTest logger, and this repo runs tests through Microsoft Testing
Platform, which never loads it.

Replace both with the first-party Microsoft.Testing.Extensions.GitHubActionsReport
extension and its documented `--report-gh` switch (MTP 2.3.0+). This
gives per-assembly log groups, failure/skip annotations and a Markdown
job summary. The extension is scoped to test projects so it does not turn
the benchmark and AOT sample projects into test applications. It is only
published as a prerelease today, so the version is pinned and should be
moved to the stable release once one ships.

Also add GitHub's built-in code coverage alongside Codecov. Coverlet now
emits Cobertura next to OpenCover, the per-project/per-TFM reports are
merged into one file with ReportGenerator, and paths are rewritten to be
repository-relative so coverage maps onto the diff. Codecov keeps using
the unchanged OpenCover reports. The upload is best effort for now, since
it needs Code Quality enabled on the repository.

Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 3, 2026 12:26
@askpt
askpt force-pushed the askpt/mtp-github-report-and-coverage branch from cc0ac5b to 79d25c6 Compare August 3, 2026 12:26
@askpt
askpt changed the base branch from main to askpt/ci-required-job-merge-queue August 3, 2026 12:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (1)

.github/workflows/reusable-coverage.yml:94

  • This newly added reporting path can still block the required CI Gate: a tool-install, glob, ReportGenerator, or sed failure stops this step before the uploader's fail-on-error setting applies, and ci.yml rejects a failed coverage job. Make both GitHub-only steps non-blocking so Codecov remains the coverage gate as intended.
      - name: Merge Cobertura reports (Linux)
        if: runner.os == 'Linux'
        run: |

Base automatically changed from askpt/ci-required-job-merge-queue to main August 6, 2026 09:27
askpt added 3 commits August 6, 2026 10:37
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
@askpt
askpt force-pushed the askpt/mtp-github-report-and-coverage branch from 74bd0f1 to 69c22e1 Compare August 7, 2026 08:26
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