diff --git a/.github/tests/consumer_coverage.py b/.github/tests/consumer_coverage.py index 68af6cd..a453d62 100644 --- a/.github/tests/consumer_coverage.py +++ b/.github/tests/consumer_coverage.py @@ -88,10 +88,11 @@ def measure(args): for file in actual["files"] } assert actual_files == files, f"Per-file mismatch: native {files}; DotCov {actual_files}" - # ItsDangerous's upstream reporting config also names tests as a source root. - # Keep the diagnostic and prove file attribution against the native JSON above. + # ItsDangerous's upstream reporting config also names tests as a source root. Every + # measured file exists under exactly one root, so DotCov attributes it without the + # multi-root warning; the per-file comparison above proves that attribution. warnings = [warning["kind"] for warning in actual.get("warnings", [])] - assert warnings == (["FileIdentityAmbiguous"] if args.language == "python" else []), warnings + assert warnings == [], warnings # Both real suites have missing lines and branches. No report is edited to manufacture failure. line_hit, line_total, branch_hit, branch_total = total assert 0.9 <= line_hit / line_total < 1 diff --git a/README.md b/README.md index c625a78..8dd840f 100644 --- a/README.md +++ b/README.md @@ -54,6 +54,8 @@ dotcov check coverage.xml --min-line 90 --min-branch 90 Pass the XML file directly to the badge action's `reports` input. This also avoids directory search defaults: `coverage.xml` does not match `**/*cobertura*.xml`. DotCov checks line and branch percentages separately; coverage.py's displayed total combines them. +With several coverage.py `source` roots, each file name is relative to the root that contains it. +DotCov uses the root under which the file exists, so run it where the sources are checked out. CI runs the full suites of two pinned upstream projects on Linux, Windows, and macOS: diff --git a/src/DotCov/CoberturaParser.cs b/src/DotCov/CoberturaParser.cs index f788f6d..5a554d4 100644 --- a/src/DotCov/CoberturaParser.cs +++ b/src/DotCov/CoberturaParser.cs @@ -184,9 +184,9 @@ private static async ValueTask VisitAsync( /// Capture one <source> root. No-op spellings (".", "./") collapse to the "" /// sentinel, which reads as "leave relative filenames /// unprefixed". The sentinel is recorded, not discarded: a no-op declared alongside real - /// roots is a second resolution convention, so it counts toward the multi-root warning and - /// keeps its document-order slot (the FIRST declared root wins resolution). Respellings of - /// one root are deduplicated so they cannot fake that multiplicity. + /// roots is a second resolution convention, so it is a candidate root like any other and + /// keeps its document-order slot (the FIRST declared root wins when the disk cannot + /// decide). Respellings of one root are deduplicated so they cannot fake that multiplicity. /// /// The root is the element's own character content: every text and CDATA child, /// concatenated. A comment or processing instruction splits that text without ending it, @@ -213,17 +213,7 @@ private static bool IsOwnText(XmlReader reader, int elementDepth) => private static void RecordSource(string value, DocumentContext document) { var root = PathIdentity.NormalizeRoot(value); - if (document.SourceRoots.Contains(root)) return; // List.Contains is Ordinal for strings - - document.SourceRoots.Add(root); - if (document.SourceRoots.Count is 2) - document.Warn(new CoverageWarning( - CoverageWarningKind.FileIdentityAmbiguous, - "", - 0, - $"multiple roots - {(document.SourceRoots[0].Length is 0 - ? "leaving relative filenames unprefixed (the first declared root is a no-op)" - : $"resolving relative filenames against the first ('{document.SourceRoots[0]}')")}; files may not be attributable to a unique root")); + if (!document.SourceRoots.Contains(root)) document.SourceRoots.Add(root); // List.Contains is Ordinal for strings } /// @@ -294,14 +284,14 @@ public void Visit(XmlReader sub) /// /// The file identity key of a <class filename>: separators normalized to '/' - /// (Windows coverlet writes '\'), the document's first <source> root applied + /// (Windows coverlet writes '\'), the document's <source> root applied /// to relative names, and a leading drive letter uppercased. Both aggregations key on this /// string, so method entries join against on the same key. /// Returns null for a class without a filename, which carries no attributable data. /// private static string? DecodeFileName(XmlReader reader, DocumentContext document) => reader.GetAttribute("filename") is { } filename - ? ResolveFileKey(filename.Replace('\\', '/'), document.SourceRoots) + ? ResolveFileKey(filename.Replace('\\', '/'), document) : null; /// @@ -353,22 +343,20 @@ private static bool TryDecodeLine(XmlReader reader, string file, DocumentContext } /// - /// Prepend the report's first <source> root to a relative filename. Cobertura + /// Prepend the report's <source> root to a relative filename. Cobertura /// emitters (coverage.py, gcovr, cover2cover) write filenames relative to a source root; /// discarding it would make two different files that share a relative name collide on one key. /// The rooted check is manual (leading '/' or a drive-letter prefix) because - /// Path.IsPathRooted("C:/x") is false on Linux and reports cross machines. The root - /// is applied unconditionally, never only on collision — a conditional prefix would make a - /// file's identity unstable across runs. A first-declared no-op root (the "" sentinel) - /// resolves as "no prefix": first-wins applies to the DECLARED order. + /// Path.IsPathRooted("C:/x") is false on Linux and reports cross machines. A single + /// root is applied unconditionally, never only on collision — a conditional prefix would make + /// a file's identity unstable across runs. Several roots follow . + /// A no-op root (the "" sentinel) resolves as "no prefix". /// - private static string ResolveFileKey(string filename, List sourceRoots) + private static string ResolveFileKey(string filename, DocumentContext document) { - if (sourceRoots.Count > 0 && sourceRoots[0].Length > 0 && !IsRooted(filename)) - { - var root = sourceRoots[0]; - filename = root.EndsWith('/') ? root + filename : $"{root}/{filename}"; - } + var roots = document.SourceRoots; + if (roots.Count > 0 && !IsRooted(filename)) + filename = JoinRoot(roots.Count is 1 ? roots[0] : ResolveRoot(filename, document), filename); // Uppercase a leading drive letter so `c:\x\A.cs` and `C:/x/A.cs` — the same file // emitted by different Windows toolchains — produce one Ordinal key. Key normalization, @@ -379,6 +367,42 @@ private static string ResolveFileKey(string filename, List sourceRoots) return filename; } + /// + /// Pick the root for a relative filename in a document that declares several. coverage.py + /// writes each file relative to the source root that contains it, so the one root under which + /// the file exists on disk is the file's own. When it exists under none (a report read away + /// from its sources) or under several, the first declared root applies and the document + /// carries one report-scoped warning. + /// + private static string ResolveRoot(string filename, DocumentContext document) + { + string? match = null; + foreach (var root in document.SourceRoots) + { + if (!File.Exists(JoinRoot(root, filename))) continue; + if (match is not null) return FirstRootWithWarning(document); + match = root; + } + + return match ?? FirstRootWithWarning(document); + } + + private static string FirstRootWithWarning(DocumentContext document) + { + var first = document.SourceRoots[0]; + document.Warn(new CoverageWarning( + CoverageWarningKind.FileIdentityAmbiguous, + "", + 0, + $"multiple roots - {(first.Length is 0 + ? "leaving relative filenames unprefixed (the first declared root is a no-op)" + : $"resolving relative filenames against the first ('{first}')")}; files may not be attributable to a unique root")); + return first; + } + + private static string JoinRoot(string root, string filename) => + root.Length is 0 ? filename : root.EndsWith('/') ? root + filename : $"{root}/{filename}"; + private static bool IsRooted(string path) => path.StartsWith('/') || (path.Length >= 2 && char.IsAsciiLetter(path[0]) && path[1] == ':'); diff --git a/src/DotCov/PathIdentity.cs b/src/DotCov/PathIdentity.cs index 1fe2fc6..78c0860 100644 --- a/src/DotCov/PathIdentity.cs +++ b/src/DotCov/PathIdentity.cs @@ -40,9 +40,9 @@ internal static string NormalizeRoot(string root) /// Whether two reports' declared roots genuinely differ — the single predicate behind /// both the merge's ambiguity scan and the diff's unconditional name-pairing arm, so /// the two can never disagree about what counts as a convention change. Compares - /// normalized SEQUENCES, not sets: root order is semantic (relative filenames resolve - /// against the FIRST root), so the same roots reordered can key a relative file - /// differently and must count as differing. Root-less on both sides is "same" — + /// normalized SEQUENCES, not sets: root order is semantic (a relative filename falls back to + /// the FIRST root when the disk cannot decide), so the same roots reordered can key a + /// relative file differently and must count as differing. Root-less on both sides is "same" — /// hand-built reports never trip the cross-convention machinery. /// internal static bool RootsDiffer(IReadOnlyList a, IReadOnlyList b) diff --git a/tests/DotCov.Tests/ParserSourceTests.cs b/tests/DotCov.Tests/ParserSourceTests.cs index 80ee0ac..e496b57 100644 --- a/tests/DotCov.Tests/ParserSourceTests.cs +++ b/tests/DotCov.Tests/ParserSourceTests.cs @@ -1,4 +1,5 @@ using System.Text; +using DotCov.Tests.Infrastructure; namespace DotCov.Tests; @@ -10,15 +11,18 @@ private static CoverageReport ParseXml(string xml) return CoberturaParser.Parse(stream); } - /// One-class document with optional <source> roots (raw, unescaped). - private static string Doc(string sources, string filename) => + /// One class per filename, with optional <source> roots (raw, unescaped). + private static string Doc(string sources, params string[] filenames) => $""" {sources} - + {string.Concat(filenames.Select(f => $""""""))} """; + /// A workspace directory as a declared root: forward slashes, like the parser's keys. + private static string RootOf(TempWorkspace ws, string relative) => ws.CreateDirectory(relative).Replace('\\', '/'); + [Test] public async Task Parse_EmptySourceElement_DoesNotSwallowTheFollowingRoot() { @@ -50,4 +54,51 @@ public async Task Parse_MultipleRoots_WarningIsReportScopedWithEmptyFile() await Assert.That(w.Line).IsEqualTo(0); await Assert.That(w.Detail).Contains("'/a'"); } + + [Test] + public async Task Parse_MultipleRoots_EachFileResolvesAgainstTheRootThatHasIt() + { + // coverage.py with `source = ["src", "tests"]` names each file relative to its own root. + using var ws = TempWorkspace.Create("dotcov-source-roots-"); + var src = RootOf(ws, "src"); + var tests = RootOf(ws, "tests"); + ws.Write("src/pkg/app.py", ""); + ws.Write("tests/test_app.py", ""); + + var report = ParseXml(Doc($"{src}{tests}", "pkg/app.py", "test_app.py")); + + await Assert.That(report.Files.Select(f => f.Path).Order(StringComparer.Ordinal)) + .IsEquivalentTo(new[] { $"{src}/pkg/app.py", $"{tests}/test_app.py" }.Order(StringComparer.Ordinal)); + await Assert.That(report.Warnings).IsEmpty(); + } + + [Test] + public async Task Parse_MultipleRoots_FileUnderEveryRoot_UsesTheFirstAndWarns() + { + using var ws = TempWorkspace.Create("dotcov-source-roots-"); + var a = RootOf(ws, "a"); + var b = RootOf(ws, "b"); + ws.Write("a/x.py", ""); + ws.Write("b/x.py", ""); + + var report = ParseXml(Doc($"{a}{b}", "x.py")); + + await Assert.That(report.Files.Single().Path).IsEqualTo($"{a}/x.py"); + var w = await Assert.That(report.Warnings).HasSingleItem(); + await Assert.That(w.Kind).IsEqualTo(CoverageWarningKind.FileIdentityAmbiguous); + await Assert.That(w.Detail).Contains($"'{a}'"); + } + + [Test] + public async Task Parse_NoOpRootBesideARealRoot_UsesTheRootThatHasTheFile() + { + using var ws = TempWorkspace.Create("dotcov-source-roots-"); + var src = RootOf(ws, "src"); + ws.Write("src/pkg/app.py", ""); + + var report = ParseXml(Doc($".{src}", "pkg/app.py")); + + await Assert.That(report.Files.Single().Path).IsEqualTo($"{src}/pkg/app.py"); + await Assert.That(report.Warnings).IsEmpty(); + } }