fix(coverage): sort irongut tables without leading pipes + merge cobertura files before summarizing#19
Merged
Merged
Conversation
…#18) irongut/CodeCoverageSummary v1.3.0 format: markdown emits tables without leading pipes, so the line.startswith('|') detection never matched and the sort was a byte-for-byte no-op in production. Detect a table as a header line (>=2 cells when split on '|') immediately followed by a separator line of '-', ':' and whitespace cells — with or without leading pipes — and extract the first cell from either shape. Add a byte-exact irongut fixture (badge line, pipe-less separator, Summary row, sticky-comment trailer) so the regression cannot reappear, and run the python tests in build-and-test.yaml, which previously only ran actionlint and the shell tests.
irongut/CodeCoverageSummary concatenates multiple cobertura files but does not union-merge packages across them: with several test projects covering the same assemblies, every package appears multiple times with partial rates and the summary dilutes (observed ~85% -> 43% on daml-codegen-csharp-internal#311). Reinstate v1's known-good merge: expand tests-glob under globstar+nullglob, fail loud when nothing matches, and dotnet-coverage merge into coverage/merged.cobertura.xml, which irongut now reads (composed against working-directory with the v2-style '.'/empty-safe expression). dotnet-coverage 18.0.6 is pinned inline via an env var — the removed dotnet-coverage-version input is not re-added, keeping the v2 input contract unchanged for a patch release.
…oot merged path Apply multi-agent review findings on the two coverage fixes: - document inline why the cobertura merge must precede irongut (CodeCoverageSummary v1.3.0 concatenates instead of union-merging), so the step does not get dropped again like in v2.0.0 - write merged.cobertura.xml under $GITHUB_WORKSPACE so irongut gets a constant filename, dropping the working-directory ternary - reword the two ::error:: guards to caller-actionable causes (tests-glob / merge step) instead of a non-existent filename input - emit a ::warning:: from sort.py when no markdown table is detected, so a future irongut output drift cannot silently no-op again; new contains_table helper with unit tests - add a fixture matching the real go-ci document shape: pipe-less coverage table followed by a leading-pipe gocyclo details table
…ory.Packages.props The workflow installed dotnet-coverage 18.0.6 while callers pin Microsoft.Testing.Extensions.CodeCoverage independently (e.g. daml-codegen-csharp-internal pins 18.8.0 in tests/Directory.Packages.props), so merge tool and in-test collector could drift apart. Following the v2.0.0 global.json philosophy, a new helper script scripts/resolve-dotnet-coverage-version.py recursively scans every Directory.Packages.props under working-directory for the Microsoft.Testing.Extensions.CodeCoverage pin (Include or Update) and prints the single literal version; it fails loud on a missing pin, conflicting versions (listing files), or an MSBuild-property version, and skips unparseable XML with a warning. The helpers checkout moves up to just after Setup .NET so the resolve step can run before the tool install; the sort step reuses the same checkout. The version bump is now a caller-side props change only.
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
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.
Closes #18.
What
Coverage-table fixes in csharp-ci@v2 (input/secret contract unchanged):
1. sort-coverage-table: silent no-op on real irongut output (#18)
sort.pydetected table rows withline.startswith('|'), but irongut/CodeCoverageSummary v1.3.0format: markdownemits tables without leading pipes — so production sorting was a byte-for-byte no-op while tests passed on pipe-prefixed fixtures. Tables are now detected as a header line (≥2 pipe-split cells) followed by a---separator line, with or without leading pipes;**Summary**stays pinned last and only the first table is sorted. The action now emits a::warning::when no table is detected, so future irongut output drift can't regress silently again. New fixtures are byte-exact irongut shapes, including the real go-ci document (pipe-less coverage table + leading-pipe gocyclo<details>table).The Python tests also turned out not to run in CI at all —
build-and-test.yamlnow runs them.2. csharp-ci: union-merge cobertura files before irongut summarizes
v2.0.0 (#17) dropped the
dotnet-coverage mergestep on the claim that irongut aggregates multiple cobertura files itself. It concatenates but does not union-merge: on daml-codegen-csharp-internal#311 (7 test projects covering the same assemblies) every package appeared up to 5× with partial rates and the summary diluted from 85% to 43% (broken comment). The merge step is reinstated, writing to$GITHUB_WORKSPACE/coverage/merged.cobertura.xmlso irongut gets one constant workspace-relative path. A workaround comment on the step names irongut's concatenate-not-merge behavior so the step doesn't get deleted as 'redundant' a second time.3. csharp-ci: dotnet-coverage version resolved from the caller — no pin in this repo
Same philosophy as v2.0.0's
global.jsonSDK resolution: the workflow resolves thedotnet-coveragetool version from the caller'sDirectory.Packages.propspin ofMicrosoft.Testing.Extensions.CodeCoverage(searched recursively — daml pins it intests/). When dependabot bumps the package in a caller repo, the merge tool follows automatically on the next run — no version literal here to drift. Resolution hard-fails loudly on a missing, non-literal, or conflicting pin (the package is required for MTP to emit cobertura files at all). The two packages publish in lockstep on nuget.org (verified 18.3.1→18.8.0 identical version lists); a skipped tool release would fail the install step loudly rather than merge with a mismatched version. The removeddotnet-coverage-versioninput is NOT re-added.Evidence
Tested end-to-end from daml-codegen-csharp-internal (throwaway PRs #312, #313; self-hosted Hetzner shard): sticky comment shows each package exactly once, summary 85% (3942/4628), rows alphabetical, Summary last. Run 27408846283 logs confirm
DOTNET_COVERAGE_VERSION: 18.8.0resolved from the caller's props ("Tool 'dotnet-coverage' was successfully updated from version '18.0.6' to version '18.8.0'").Reviewed pre-open by a 7-angle multi-agent pass; applied findings: no-table warning, workspace-root merged path, caller-actionable error messages, workaround comment, real-shape go fixture, caller-resolved tool version.
Deliberately not done (follow-up candidates): caching the dotnet-coverage tool install keyed on the resolved version (removes the per-run nuget.org dependency); extracting the shared coverage-report tail (irongut → sort → title → comment → summary) used by csharp/go/scala workflows into a composite action.
Release
Tag
v2.1.0(new caller-side resolution behavior, backward-compatible) and advancev2after merge.v1stays frozen at d072766.