diff --git a/.github/scripts/publish-coverage.cjs b/.github/scripts/publish-coverage.cjs index ff281fabf..e13f247c8 100644 --- a/.github/scripts/publish-coverage.cjs +++ b/.github/scripts/publish-coverage.cjs @@ -74,6 +74,28 @@ function readResult(check, platform, expected, pr, repo, run) { throw new Error("Invalid coverage counts"); }); } + // Optional scopes preserve compatibility with results produced before the split. + if (result.groups !== undefined) { + if (!["protocol-swift", "protocol-kotlin"].includes(platform.id) || + !result.groups || Array.isArray(result.groups) || + Object.keys(result.groups).sort().join(",") !== "Generated,Runtime" || !result.rows.length) + throw new Error("Invalid coverage scopes"); + for (const scope of ["Runtime", "Generated"]) { + const rows = result.groups[scope]; + if (!Array.isArray(rows) || rows.length !== platform.metrics.length) + throw new Error("Missing scope metrics"); + rows.forEach((row, index) => { + if (!Array.isArray(row) || row.length !== 3 || row[0] !== platform.metrics[index] || + !Number.isSafeInteger(row[1]) || !Number.isSafeInteger(row[2]) || row[1] < 0 || row[2] < row[1]) + throw new Error("Invalid scope counts"); + }); + } + result.rows.forEach((row, index) => { + for (const count of [1, 2]) + if (result.groups.Runtime[index][count] + result.groups.Generated[index][count] !== row[count]) + throw new Error("Coverage scopes do not match totals"); + }); + } return {...result, reportUrl: reportURL(result.reportUrl, repo, platform, run)}; } @@ -135,8 +157,14 @@ function render(results) { for (const target of platform.metrics) lines.push(`| ${emoji} | Swift · ${target} | ${metric(rows.find(([name]) => name === target))} | — | — | ${report} |`); } else { - const find = (name) => rows.find((row) => row[0] === name); - lines.push(`| ${emoji} | ${platform.displayTitle || platform.title} | ${metric(find("Lines"))} | ${metric(find("Branches"))} | ${metric(find("Functions") || find("Methods"))} | ${report} |`); + const scopes = result.groups + ? [["Runtime", result.groups.Runtime], ["Generated", result.groups.Generated], ["Total", rows]] + : [[null, rows]]; + for (const [scope, metrics] of scopes) { + const find = (name) => metrics.find((row) => row[0] === name); + const title = `${platform.displayTitle || platform.title}${scope ? ` · ${scope}` : ""}`; + lines.push(`| ${emoji} | ${title} | ${metric(find("Lines"))} | ${metric(find("Branches"))} | ${metric(find("Functions") || find("Methods"))} | ${report} |`); + } } } return lines.join("\n") + "\n"; diff --git a/.github/scripts/publish-coverage.test.cjs b/.github/scripts/publish-coverage.test.cjs index 740c67af3..c935204d7 100644 --- a/.github/scripts/publish-coverage.test.cjs +++ b/.github/scripts/publish-coverage.test.cjs @@ -233,3 +233,56 @@ test("native protocol results retain their own state while sharing a runner", as assert.ok(row(body, "Embedded Checkout Protocol (Swift)").includes("[Full report]")); assert.equal(f.warnings.length, 0); }); + +function scopes(platform) { + return { + Runtime: platform.metrics.map((name) => [name, 6, 6]), + Generated: platform.metrics.map((name) => [name, 1, 4]), + }; +} + +test("native protocol scopes retain totals and distinguish runtime from generated code", async () => { + const f = fixture(); + for (const id of ["protocol-swift", "protocol-kotlin"]) { + const platform = reporter.platforms.find((platform) => platform.id === id); + f.add(id, {groups: scopes(platform)}); + } + await f.publish(); + assert.equal(f.warnings.length, 0); + for (const language of ["Swift", "Kotlin"]) { + assert.ok(row(f.writes[0].body, `Embedded Checkout Protocol (${language}) · Runtime`).includes("100%")); + assert.ok(row(f.writes[0].body, `Embedded Checkout Protocol (${language}) · Generated`).includes("25%")); + assert.ok(row(f.writes[0].body, `Embedded Checkout Protocol (${language}) · Total`).includes("70%")); + } +}); + +test("rejects forged scope names, partial metrics and counts inconsistent with totals", async () => { + for (const mutate of [ + (result) => { result.groups.Injected = result.groups.Runtime; }, + (result) => { delete result.groups.Generated; }, + (result) => { result.groups.Runtime[0][1] = 7; }, + (result) => { result.groups.Runtime[0][1] = 5; }, + (result) => { result.groups.Runtime[0][0] = "unexpected"; }, + (result) => { result.groups.Runtime.pop(); }, + (result) => { result.groups = null; }, + (result) => { result.state = "skipped"; result.rows = []; }, + ]) { + const f = fixture(); + const platform = reporter.platforms.find((platform) => platform.id === "protocol-kotlin"); + const check = f.add(platform.id, {groups: scopes(platform)}); + const result = JSON.parse(check.output.text); + mutate(result); + check.output.text = JSON.stringify(result); + await f.publish(); + assert.equal(f.warnings.length, 1); + assert.ok(!f.writes[0].body.includes(" · Runtime")); + } +}); + +test("failed protocol tests keep scope measurements and failure status", async () => { + const f = fixture(); + const platform = reporter.platforms.find((platform) => platform.id === "protocol-swift"); + f.add(platform.id, {state: "failed", groups: scopes(platform)}); + await f.publish(); + assert.match(row(f.writes[0].body, `${platform.title} · Runtime`), /❌.*100%/); +}); diff --git a/scripts/lib/coverage_report.rb b/scripts/lib/coverage_report.rb index c14f549d3..6bdd8ba22 100644 --- a/scripts/lib/coverage_report.rb +++ b/scripts/lib/coverage_report.rb @@ -9,13 +9,15 @@ class CoverageReport JAVASCRIPT_METRICS = {"lines" => "Lines", "statements" => "Statements", "branches" => "Branches", "functions" => "Functions"}.freeze TITLES = {"swift" => "Swift", "android" => "Android", "web" => "Web", "react-native" => "React Native", "protocol" => "Embedded Checkout Protocol (TypeScript)", "protocol-swift" => "Embedded Checkout Protocol (Swift)", "protocol-kotlin" => "Embedded Checkout Protocol (Kotlin)"}.freeze - attr_reader :platform, :rows + attr_reader :platform, :rows, :groups def initialize(platform, contents) @platform = platform + @groups = {} @rows = case platform when "swift" then swift_rows(contents) - when "android", "protocol-kotlin" then android_rows(contents) + when "android" then android_rows(contents) + when "protocol-kotlin" then kotlin_protocol_rows(contents) when "protocol-swift" then swift_protocol_rows(contents) when "web", "react-native", "protocol" then javascript_rows(contents) else raise ArgumentError, "Unknown coverage platform: #{platform}" @@ -29,7 +31,13 @@ def marker def markdown(report_url: nil) label = TITLES.fetch(platform) lines = [marker, "# #{label} — Coverage Report", ""] - if platform == "swift" + if groups.any? + lines.concat(["| Scope | #{@rows.map(&:first).join(' | ')} |", "| --- | #{@rows.map { "---" }.join(" | ")} |"]) + groups.merge("Total" => rows).each do |scope, metrics| + cells = metrics.map { |name, covered, total| metric(covered, total, badge: name == "Lines", report_url: report_url) } + lines << "| #{scope} | #{cells.join(' | ')} |" + end + elsif platform == "swift" lines.concat(["| Target | Lines |", "| --- | --- |"]) @rows.each do |name, covered, total| lines << "| #{name} | #{metric(covered, total, badge: true, report_url: report_url)} |" @@ -94,12 +102,42 @@ def swift_protocol_rows(contents) raise "Missing Swift protocol coverage files" if files.empty? raise "Duplicate Swift protocol coverage files" unless files.map { |file| file.fetch("filename") }.uniq.length == files.length - {"lines" => "Lines", "functions" => "Functions"}.map do |key, label| - counts = files.map do |file| - metric = file.fetch("summary").fetch(key) - row(label, metric.fetch("covered"), metric.fetch("count")) + metrics = {"lines" => "Lines", "functions" => "Functions"} + @groups = {"Runtime" => [], "Generated" => []} + files.each do |file| + scope = file.fetch("filename").include?("/EmbeddedCheckoutProtocol/Generated/") ? "Generated" : "Runtime" + @groups.fetch(scope) << metrics.map do |key, label| + counts = file.fetch("summary").fetch(key) + row(label, counts.fetch("covered"), counts.fetch("count")) end - row(label, counts.sum { |entry| entry[1] }, counts.sum { |entry| entry[2] }) + end + @groups.transform_values! { |entries| sum_rows(entries, metrics.values) } + sum_rows(groups.values, metrics.values) + end + + def kotlin_protocol_rows(contents) + totals = android_rows(contents) + document = REXML::Document.new(contents) + files = document.get_elements("report/package/sourcefile") + raise "Missing Kotlin protocol coverage files" if files.empty? + paths = files.map { |file| "#{file.parent.attributes['name']}/#{file.attributes['name']}" } + raise "Duplicate Kotlin protocol coverage files" unless paths.uniq.length == paths.length + + generated = %w[Models.kt EmbeddedCheckoutProtocol.kt].map { |name| "com/shopify/ucp/embedded/checkout/#{name}" } + @groups = {"Runtime" => [], "Generated" => []} + files.zip(paths).each do |file, path| + scope = generated.include?(path) ? "Generated" : "Runtime" + @groups.fetch(scope) << android_counters(file, allow_missing: true) + end + @groups.transform_values! { |entries| sum_rows(entries, ANDROID_METRICS.values) } + raise "Kotlin coverage scopes do not match report totals" unless sum_rows(groups.values, ANDROID_METRICS.values) == totals + + totals + end + + def sum_rows(entries, labels) + labels.each_with_index.map do |label, index| + row(label, entries.sum { |entry| entry.fetch(index)[1] }, entries.sum { |entry| entry.fetch(index)[2] }) end end @@ -108,9 +146,15 @@ def android_rows(contents) require "rexml/document" document = REXML::Document.new(contents) + # Nested package/class counters duplicate the report totals. + android_counters(document.elements["report"]) + end + + def android_counters(element, allow_missing: false) ANDROID_METRICS.map do |type, label| - # Nested package/class counters duplicate the report totals. - counter = document.elements["report/counter[@type='#{type}']"] + counter = element&.elements&.[]("counter[@type='#{type}']") + # JaCoCo omits source-level counters when a file has no such instructions. + next row(label, 0, 0) if !counter && allow_missing raise "Missing Android coverage counter: #{type}" unless counter covered = Integer(counter.attributes["covered"]) @@ -161,6 +205,7 @@ def publish(platform:, state:, report: nil, report_url: nil, preserve_existing: existing = checks.find { |check| check["external_id"] == external_id && check.dig("app", "slug") == @source.fetch("provider") } unless existing && preserve_existing result = {version: 1, platform: platform, pr: @pr_number.to_i, headSha: @sha, source: @source, state: state, rows: report&.rows || [], reportUrl: report_url} + result[:groups] = report.groups if report && report.groups.any? payload = { name: "Coverage — #{CoverageReport::TITLES.fetch(platform)}", external_id: external_id, diff --git a/scripts/test/coverage_report_test.rb b/scripts/test/coverage_report_test.rb index ccf7dd9ac..acf36ecda 100644 --- a/scripts/test/coverage_report_test.rb +++ b/scripts/test/coverage_report_test.rb @@ -49,6 +49,20 @@ def android_xml XML end + def kotlin_xml + runtime = {"LINE" => [3, 0], "INSTRUCTION" => [8, 0], "BRANCH" => [0, 0], "METHOD" => [1, 0]} + generated = {"LINE" => [1, 1], "INSTRUCTION" => [1, 1], "BRANCH" => [0, 0], "METHOD" => [0, 3]} + sources = {"Client.kt" => runtime, "Models.kt" => generated, "EmbeddedCheckoutProtocol.kt" => runtime}.map do |name, counters| + xml = counters.reject { |_, counts| counts == [0, 0] }.map { |type, (covered, missed)| %() }.join + %(#{xml}) + end.join + # Include the generated catalog in the total, but not the runtime scope. + xml = android_xml.sub(%(), %(#{sources})) + xml.sub('type="LINE" covered="4" missed="1"', 'type="LINE" covered="7" missed="1"') + .sub('type="INSTRUCTION" covered="9" missed="1"', 'type="INSTRUCTION" covered="17" missed="1"') + .sub('type="METHOD" covered="1" missed="3"', 'type="METHOD" covered="2" missed="3"') + end + def test_swift_reports_sdk_targets_only_even_below_85_percent markdown = CoverageReport.new("swift", swift_json).markdown @@ -80,7 +94,12 @@ def test_swift_protocol_uses_only_library_files_and_includes_generated_wire_mode report = CoverageReport.new("protocol-swift", JSON.generate("data" => [{"files" => files}])) assert_equal [["Lines", 11, 15], ["Functions", 2, 4]], report.rows assert_includes report.markdown, "Embedded Checkout Protocol (Swift)" - assert_includes report.markdown, "| Lines | Functions |" + assert_equal [["Lines", 8, 10], ["Functions", 1, 2]], report.groups.fetch("Runtime") + assert_equal [["Lines", 3, 5], ["Functions", 1, 2]], report.groups.fetch("Generated") + assert_includes report.markdown, "| Scope | Lines | Functions |" + assert_includes report.markdown, "| Runtime |" + assert_includes report.markdown, "| Generated |" + assert_includes report.markdown, "| Total |" end def test_swift_protocol_rejects_missing_or_duplicate_files @@ -90,12 +109,28 @@ def test_swift_protocol_rejects_missing_or_duplicate_files end def test_kotlin_protocol_has_its_own_title_and_comment_marker - report = CoverageReport.new("protocol-kotlin", android_xml) - assert_equal CoverageReport.new("android", android_xml).rows, report.rows + report = CoverageReport.new("protocol-kotlin", kotlin_xml) + assert_equal CoverageReport.new("android", kotlin_xml).rows, report.rows + assert_equal ["Lines", 3, 3], report.groups.fetch("Runtime").first + assert_equal ["Lines", 4, 5], report.groups.fetch("Generated").first assert_includes report.markdown, "Embedded Checkout Protocol (Kotlin)" refute_equal CoverageReport.new("android", android_xml).marker, report.marker end + def test_kotlin_protocol_rejects_missing_duplicate_and_incomplete_source_totals + assert_raises(RuntimeError) { CoverageReport.new("protocol-kotlin", android_xml) } + assert_raises(RuntimeError) { CoverageReport.new("protocol-kotlin", kotlin_xml.sub('name="Models.kt"', 'name="Client.kt"')) } + assert_raises(RuntimeError) { CoverageReport.new("protocol-kotlin", kotlin_xml.sub('covered="7"', 'covered="8"')) } + end + + def test_empty_generated_scope_remains_visible_without_diluting_runtime + file = {"filename" => "/Sources/UniversalCommerceProtocol/EmbeddedCheckoutProtocol/Client.swift", + "summary" => {"lines" => {"covered" => 8, "count" => 10}, "functions" => {"covered" => 1, "count" => 2}}} + report = CoverageReport.new("protocol-swift", JSON.generate("data" => [{"files" => [file]}])) + assert_equal [["Lines", 0, 0], ["Functions", 0, 0]], report.groups.fetch("Generated") + assert_equal report.rows, report.groups.fetch("Runtime") + end + def test_missing_targets_and_counters_fail_instead_of_showing_partial_coverage assert_raises(RuntimeError) { CoverageReport.new("swift", '{"targets":[]}') } assert_raises(RuntimeError) { CoverageReport.new("android", '') } @@ -210,6 +245,16 @@ def test_persists_numeric_results_against_the_exact_run_without_a_pr_comment assert_equal 1, @client.writes.length end + def test_publishes_protocol_scopes_alongside_backward_compatible_totals + @report = CoverageReport.new("protocol-kotlin", CoverageReportTest.new("unused").kotlin_xml) + @client = FakeClient.new(@pr, []) + CoverageResultPublisher.new(repository: "example/sdk", pr_number: 123, sha: "abc123", token: "test-token", source: @source, client: @client) + .publish(platform: "protocol-kotlin", state: "success", report: @report) + result = JSON.parse(@client.writes.first[2][:output][:text]) + assert_equal @report.rows, result.fetch("rows") + assert_equal @report.groups, result.fetch("groups") + end + def test_updates_the_matching_check_without_creating_duplicates publish([{"id" => 42, "external_id" => "coverage:swift:100:2", "app" => {"slug" => "github-actions"}}]) assert_equal [:patch, "/repos/example/sdk/check-runs/42"], @client.writes.first.take(2)