Skip to content

Commit 926d33c

Browse files
committed
ci(all-green): merge/upload lcov only, drop per-run Codecov cache
istanbul JSON merging was the actual bottleneck on reruns (far slower than lcov's merge on runs with many cells), for coverage detail (branch/function) only Codecov read. Dropping JSON support outright removes that cost instead of caching around it, at the cost of losing that detail in Codecov. With the merge now cheap, the per-run Codecov-upload cache (a persisted run-id list plus an actions/cache step) no longer earns its complexity — every rerun just redoes download, merge, and upload for every run again. Generated by Claude Code.
1 parent f4a7287 commit 926d33c

7 files changed

Lines changed: 55 additions & 226 deletions

File tree

.github/actions/upload-coverage-artifact/action.yml

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,14 @@ inputs:
1313
runs:
1414
using: composite
1515
steps:
16-
# The reports live at `<report-dir>/node-<version>-<label>/{lcov.info,coverage-final.json}`, one
17-
# level below `report-dir`, so the presence check has to recurse — a top-level-only probe finds
18-
# nothing and silently skips the upload, which is what starved the Datadog coverage upload. Both
19-
# formats ship: Datadog reads the lcov, Codecov reads the istanbul JSON (see
20-
# `scripts/upload-coverage.mjs`).
16+
# The reports live at `<report-dir>/node-<version>-<label>/lcov.info`, one level below
17+
# `report-dir`, so the presence check has to recurse — a top-level-only probe finds nothing and
18+
# silently skips the upload, which is what starved the Datadog coverage upload. Only lcov ships:
19+
# both Datadog and Codecov ingest it (see `scripts/upload-coverage.mjs`).
2120
- id: check
2221
shell: bash
2322
run: |
24-
count=$(find "${{ inputs.report-dir }}" \( -name lcov.info -o -name coverage-final.json \) -type f 2>/dev/null | wc -l | tr -d ' ')
23+
count=$(find "${{ inputs.report-dir }}" -name lcov.info -type f 2>/dev/null | wc -l | tr -d ' ')
2524
echo "has_coverage=$([ "$count" -gt 0 ] && echo true || echo false)" >> "$GITHUB_OUTPUT"
2625
# The artifact name carries the grouping flag plus a per-cell suffix (`__<job>-<matrix-index>`)
2726
# so matrix cells that share a flag — cypress varies `spec` outside its flag — upload distinct
@@ -32,9 +31,7 @@ runs:
3231
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
3332
with:
3433
name: coverage-${{ inputs.flags }}__${{ github.job }}-${{ strategy.job-index }}
35-
path: |
36-
${{ inputs.report-dir }}/**/lcov.info
37-
${{ inputs.report-dir }}/**/coverage-final.json
34+
path: ${{ inputs.report-dir }}/**/lcov.info
3835
retention-days: 1
3936
- if: github.actor != 'dependabot[bot]' && steps.check.outputs.has_coverage == 'true' && steps.upload.outcome == 'failure'
4037
shell: bash
@@ -43,8 +40,6 @@ runs:
4340
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
4441
with:
4542
name: coverage-${{ inputs.flags }}__${{ github.job }}-${{ strategy.job-index }}
46-
path: |
47-
${{ inputs.report-dir }}/**/lcov.info
48-
${{ inputs.report-dir }}/**/coverage-final.json
43+
path: ${{ inputs.report-dir }}/**/lcov.info
4944
retention-days: 1
5045
overwrite: true

.github/workflows/all-green.yml

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -31,29 +31,17 @@ jobs:
3131
- uses: ./.github/actions/node
3232
with:
3333
version: active
34-
- run: yarn add @actions/core @actions/github octokit istanbul-lib-coverage
34+
- run: yarn add @actions/core @actions/github octokit
3535
- uses: ./.github/actions/dd-sts-api-key
3636
id: dd-sts
3737
- uses: ./.github/actions/datadog-ci
3838
- run: pip install --quiet codecov-cli
39-
# Lets a rerun of this job for the same commit (e.g. retrying a flaky sibling workflow) skip
40-
# redownloading artifacts and resubmitting uploads for sibling workflow runs a previous
41-
# attempt already finished — see PROCESSED_RUNS_CACHE_PATH in all-green.mjs. The key includes
42-
# the attempt number so every attempt's save is a new entry (an exact-key hit skips saving,
43-
# which would drop this attempt's newly-processed runs); restore-keys falls back to the most
44-
# recent previous attempt's cache.
45-
- uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
46-
with:
47-
path: .all-green-processed-runs.json
48-
key: all-green-processed-runs-${{ github.event.pull_request.head.sha || github.sha }}-${{ github.run_attempt }}
49-
restore-keys: |
50-
all-green-processed-runs-${{ github.event.pull_request.head.sha || github.sha }}-
5139
# Polls sibling workflows and, as soon as each one reaches a final state, downloads its
5240
# artifacts, merges its reports, and uploads its junit/coverage to Datadog and Codecov —
5341
# instead of waiting for every workflow to finish before downloading or uploading anything.
54-
# Datadog gets the merged lcov, Codecov gets the merged istanbul JSON (it reads branch/function
55-
# coverage from that format, and its own cross-session merge for a shared file has been
56-
# observed overwriting rather than summing when more than one session reports it).
42+
# Both Datadog and Codecov get the same merged lcovCodecov's own cross-session merge for a
43+
# shared file has been observed overwriting rather than summing when more than one session
44+
# reports it, so it's merged into one report per run before upload instead.
5745
# `master-coverage` is the flag `.codecov.yml` gates `codecov/patch` on; attached only on PRs
5846
# targeting master so release-branch PRs auto-pass. The SHA/PR are passed explicitly because
5947
# All Green checks out the merge ref, not the head the other workflows reported coverage for.

scripts/all-green.mjs

Lines changed: 3 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import { Octokit } from 'octokit'
33
import { summary } from '@actions/core'
44
import { context } from '@actions/github'
55
import { downloadArtifacts } from './download-artifacts.mjs'
6-
import { loadProcessedRunIds, saveProcessedRunIds } from './processed-runs.mjs'
76
import { logUploads } from './run-upload.mjs'
87
import { uploadAllJunit } from './upload-junit.mjs'
98
import {
@@ -12,15 +11,6 @@ import {
1211

1312
/* eslint-disable no-console */
1413

15-
// Persisted across All Green job attempts for the same commit (see all-green.yml's cache step).
16-
// Only the Codecov upload is worth caching — it's the slow part (a `codecovcli` child process per
17-
// run) and is confirmable per-run, immediately. The Datadog junit/coverage batch uploads are cheap
18-
// and can only be confirmed as one call covering every currently-downloaded run, so they're always
19-
// redone in full every attempt; caching them would risk silently dropping a run's data if the batch
20-
// call failed after that run had been skipped from download.
21-
const PROCESSED_RUNS_CACHE_PATH = '.all-green-processed-runs.json'
22-
const cachedRunIds = loadProcessedRunIds(PROCESSED_RUNS_CACHE_PATH)
23-
2414
const {
2515
BASE_REF,
2616
DELAY,
@@ -114,41 +104,30 @@ async function getRuns () {
114104
}
115105
}
116106

117-
// Runs already dispatched to `processRun` this attempt, so a run settling across more than one
107+
// Runs already dispatched to `processRun`, so a run settling across more than one
118108
// `scheduleProcessing` call (e.g. after a retry) isn't downloaded/uploaded twice.
119109
const dispatchedRunIds = new Set()
120110
const processingPromises = []
121-
// Runs whose Codecov upload has succeeded, either in a previous job attempt (seeded from
122-
// `cachedRunIds`) or this one. Persisted to disk the moment a new run succeeds — see `processRun`
123-
// — so a mid-run cancellation or crash still leaves that progress on disk for the next attempt,
124-
// instead of only ever saving once at the very end.
125-
const codecovDoneRunIds = new Set(cachedRunIds)
126111

127112
/**
128113
* Download a single finished workflow run's junit and coverage artifacts, merge them, and upload
129-
* the coverage merge to Codecov unless a previous job attempt already did.
114+
* the coverage merge to Codecov.
130115
*
131116
* @param {{ id: number, name: string }} run
132117
* @returns {Promise<void>}
133118
*/
134119
async function processRun (run) {
135120
const { downloaded, failed } = await downloadArtifacts(octokit, { owner, repo, token: GITHUB_TOKEN, runs: [run] })
136121

137-
const alreadyOnCodecov = cachedRunIds.has(run.id)
138122
const coverageResults = await uploadCoverage(run, {
139123
sha: HEAD_SHA,
140124
branch: HEAD_BRANCH,
141125
prNumber: PR_NUMBER,
142126
eventName: GITHUB_EVENT_NAME,
143127
baseRef: BASE_REF,
144-
}, alreadyOnCodecov)
128+
})
145129
const downloadSummary = failed > 0 ? `${downloaded} artifact(s), ${failed} failed` : `${downloaded} artifact(s)`
146130
logUploads(`${run.name} (${downloadSummary})`, coverageResults)
147-
148-
if (!alreadyOnCodecov && failed === 0 && coverageResults.every(result => result.code === 0)) {
149-
codecovDoneRunIds.add(run.id)
150-
saveProcessedRunIds(PROCESSED_RUNS_CACHE_PATH, codecovDoneRunIds)
151-
}
152131
}
153132

154133
/**

scripts/group-coverage.mjs

Lines changed: 26 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,26 @@
11
import { mkdirSync, readdirSync, readFileSync, writeFileSync } from 'node:fs'
22
import { join } from 'node:path'
33

4-
import istanbulLibCoverage from 'istanbul-lib-coverage'
5-
6-
// Merges one workflow run's downloaded per-cell `coverage-*` artifacts into a single lcov file and a
7-
// single istanbul JSON file under `coverage-upload/<run-id>/`, scoped to that run alone. All Green
8-
// calls this as soon as a sibling workflow finishes, instead of waiting for every workflow to
9-
// complete before merging and uploading anything — the goal is for each workflow's coverage to reach
10-
// Datadog and Codecov shortly after that workflow finishes, in parallel with the rest still running.
4+
// Merges one workflow run's downloaded per-cell `coverage-*` artifacts into a single lcov file under
5+
// `coverage-upload/<run-id>/`, scoped to that run alone. All Green calls this as soon as a sibling
6+
// workflow finishes, instead of waiting for every workflow to complete before merging and uploading
7+
// anything — the goal is for each workflow's coverage to reach Datadog and Codecov shortly after that
8+
// workflow finishes, in parallel with the rest still running.
119
//
12-
// Codecov reads branch/function coverage from istanbul's JSON; Datadog only ingests the lcov — see
13-
// `upload-coverage.mjs`. Both formats need a real per-file merge, not concatenation: every matrix
14-
// cell in a workflow run (each Node.js version, each plugin partition) writes its own complete
15-
// report, so a shared source file's coverage shows up once per cell. Concatenating lcov's `SF:`
16-
// blocks produces a report with duplicate `SF:` sections per file, which downstream lcov consumers
17-
// resolve by keeping only the last block for that file rather than summing across blocks — silently
18-
// discarding most of the branch/function data every earlier cell had recorded. A naive object merge
19-
// of istanbul's JSON has the same failure mode. Uploading either format unmerged across sessions has
20-
// the same problem one level up: Codecov's own cross-session merge has been observed overwriting
21-
// rather than summing a shared file's coverage when more than one session reports it, so every
22-
// format is merged down to one report per run before upload instead of relying on the backend to
23-
// reconcile per-session duplicates. `mergeLcov` sums `DA:`/`FNDA:`/`BRDA:` hit counts per file across
24-
// cells (the way `lcov --add-tracefile` does), and `mergeCoverageJson` uses
25-
// `istanbul-lib-coverage`'s `merge` to do the same for the JSON report.
10+
// Both Datadog and Codecov ingest lcov only — istanbul JSON support (which Codecov used to read
11+
// branch/function coverage from) was dropped: it doubled the merge cost on runs with many cells
12+
// (`istanbul-lib-coverage`'s merge is far slower than `mergeLcov`) for coverage the lcov format
13+
// doesn't carry (branch/function hit counts) — an acceptable trade-off. Merging is still required,
14+
// not concatenation: every matrix cell in a workflow run (each Node.js version, each plugin
15+
// partition) writes its own complete report, so a shared source file's coverage shows up once per
16+
// cell. Concatenating lcov's `SF:` blocks produces a report with duplicate `SF:` sections per file,
17+
// which downstream lcov consumers resolve by keeping only the last block for that file rather than
18+
// summing across blocks — silently discarding most of the coverage every earlier cell had recorded.
19+
// Uploading unmerged across sessions has the same problem one level up: Codecov's own cross-session
20+
// merge has been observed overwriting rather than summing a shared file's coverage when more than one
21+
// session reports it, so lcov is merged down to one report per run before upload instead of relying
22+
// on the backend to reconcile per-session duplicates. `mergeLcov` sums `DA:`/`FNDA:`/`BRDA:` hit
23+
// counts per file across cells, the way `lcov --add-tracefile` does.
2624
//
2725
// Per-integration/per-area flags were dropped: `.codecov.yml` only gates the separate
2826
// `master-coverage` flag (attached to every upload regardless of grouping), so a finer-grained flag
@@ -35,7 +33,6 @@ const ARTIFACT_PREFIX = 'coverage-'
3533

3634
const REPORTS = new Map([
3735
['lcov.info', 'lcov'],
38-
['coverage-final.json', 'json'],
3936
])
4037

4138
/**
@@ -231,44 +228,21 @@ function mergeLcov (reportPaths) {
231228
}
232229

233230
/**
234-
* Sum per-statement/branch/function hit counts across every cell's istanbul JSON report, so a
235-
* source file exercised by more than one cell keeps every cell's coverage instead of only the last
236-
* report merged for that file.
237-
*
238-
* @param {string[]} reportPaths
239-
* @returns {object}
240-
*/
241-
function mergeCoverageJson (reportPaths) {
242-
const map = istanbulLibCoverage.createCoverageMap({})
243-
for (const reportPath of reportPaths) {
244-
map.merge(JSON.parse(readFileSync(reportPath, 'utf8')))
245-
}
246-
return map.toJSON()
247-
}
248-
249-
/**
250-
* Merge a single workflow run's downloaded coverage reports into one lcov file and one istanbul
251-
* JSON file for upload.
231+
* Merge a single workflow run's downloaded coverage reports into one lcov file for upload.
252232
*
253233
* @param {string|number} runId
254234
* @param {string} [inputDir]
255235
* @param {string} [outputDir]
256-
* @param {boolean} [skipJson] Skip merging the istanbul JSON reports — only Codecov reads them
257-
* (see `upload-coverage.mjs`), and `istanbul-lib-coverage`'s merge is far slower than `mergeLcov`
258-
* on a run with many cells, so a run whose Codecov upload already succeeded in a previous job
259-
* attempt can skip this merge entirely instead of paying for it only to discard the result.
260-
* @returns {{ lcovDir: string|null, jsonDir: string|null }} Directories containing the merged
261-
* `lcov.info` and `coverage-final.json`, each null if the run produced no report in that format
262-
* (or, for `jsonDir`, if `skipJson` was set).
236+
* @returns {{ lcovDir: string|null }} Directory containing the merged `lcov.info`, null if the run
237+
* produced no coverage report.
263238
*/
264-
function mergeRunCoverage (runId, inputDir = INPUT_DIR, outputDir = OUTPUT_DIR, skipJson = false) {
239+
function mergeRunCoverage (runId, inputDir = INPUT_DIR, outputDir = OUTPUT_DIR) {
265240
const files = collectCoverageFiles(join(inputDir, String(runId)), [], { runId: String(runId) })
266-
if (files.length === 0) return { lcovDir: null, jsonDir: null }
241+
if (files.length === 0) return { lcovDir: null }
267242

268243
const { reportsByArtifact, artifacts } = planCoverageGroups(files)
269244
const reports = artifacts.flatMap(artifact => reportsByArtifact.get(artifact))
270245
const lcovReportPaths = reports.filter(r => r.format === 'lcov').map(r => r.reportPath)
271-
const jsonReportPaths = skipJson ? [] : reports.filter(r => r.format === 'json').map(r => r.reportPath)
272246

273247
let lcovDir = null
274248
if (lcovReportPaths.length > 0) {
@@ -277,14 +251,7 @@ function mergeRunCoverage (runId, inputDir = INPUT_DIR, outputDir = OUTPUT_DIR,
277251
writeFileSync(join(lcovDir, 'lcov.info'), mergeLcov(lcovReportPaths))
278252
}
279253

280-
let jsonDir = null
281-
if (jsonReportPaths.length > 0) {
282-
jsonDir = join(outputDir, String(runId), 'json')
283-
mkdirSync(jsonDir, { recursive: true })
284-
writeFileSync(join(jsonDir, 'coverage-final.json'), JSON.stringify(mergeCoverageJson(jsonReportPaths)))
285-
}
286-
287-
return { lcovDir, jsonDir }
254+
return { lcovDir }
288255
}
289256

290-
export { OUTPUT_DIR, mergeCoverageJson, mergeLcov, mergeRunCoverage, planCoverageGroups }
257+
export { OUTPUT_DIR, mergeLcov, mergeRunCoverage, planCoverageGroups }

scripts/group-coverage.spec.mjs

Lines changed: 5 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { join } from 'node:path'
55

66
import { afterEach, beforeEach, describe, it } from 'mocha'
77

8-
import { mergeCoverageJson, mergeLcov, mergeRunCoverage, planCoverageGroups } from './group-coverage.mjs'
8+
import { mergeLcov, mergeRunCoverage, planCoverageGroups } from './group-coverage.mjs'
99

1010
/**
1111
* One cell's discovered report set: one `lcov` entry per Node.js version the cell ran.
@@ -138,32 +138,6 @@ describe('group-coverage', () => {
138138
})
139139
})
140140

141-
describe('mergeCoverageJson', () => {
142-
let dir
143-
144-
beforeEach(() => {
145-
dir = mkdtempSync(join(tmpdir(), 'group-coverage-json-'))
146-
})
147-
148-
afterEach(() => {
149-
rmSync(dir, { force: true, recursive: true })
150-
})
151-
152-
it('sums statement hit counts for the same file across reports', () => {
153-
const a = join(dir, 'a.json')
154-
const b = join(dir, 'b.json')
155-
const statementMap = { 0: { start: { line: 1, column: 0 }, end: { line: 1, column: 10 } } }
156-
writeFileSync(a, JSON.stringify({
157-
'shared.js': { path: 'shared.js', statementMap, fnMap: {}, branchMap: {}, s: { 0: 1 }, f: {}, b: {} },
158-
}))
159-
writeFileSync(b, JSON.stringify({
160-
'shared.js': { path: 'shared.js', statementMap, fnMap: {}, branchMap: {}, s: { 0: 2 }, f: {}, b: {} },
161-
}))
162-
const merged = mergeCoverageJson([a, b])
163-
assert.equal(merged['shared.js'].s[0], 3)
164-
})
165-
})
166-
167141
describe('mergeRunCoverage', () => {
168142
let dir
169143

@@ -182,57 +156,24 @@ describe('group-coverage', () => {
182156
mkdirSync(cellDir, { recursive: true })
183157
writeFileSync(join(cellDir, 'lcov.info'), 'SF:a.js\nDA:1,1\nend_of_record\n')
184158

185-
const { lcovDir, jsonDir } = mergeRunCoverage('42', input, output)
159+
const { lcovDir } = mergeRunCoverage('42', input, output)
186160

187161
assert.equal(lcovDir, join(output, '42', 'lcov'))
188-
assert.equal(jsonDir, null)
189162
assert.equal(
190163
readFileSync(join(lcovDir, 'lcov.info'), 'utf8'),
191164
'SF:a.js\nDA:1,1\nLF:1\nLH:1\nend_of_record\n'
192165
)
193166
})
194167

195-
it('merges the given run\'s json cells into <output>/<runId>/json/coverage-final.json', () => {
196-
const input = join(dir, 'coverage-results')
197-
const output = join(dir, 'coverage-upload')
198-
const cellDir = join(input, '42', 'coverage-apm-integrations-axios__a-0', 'node-20-x')
199-
mkdirSync(cellDir, { recursive: true })
200-
const statementMap = { 0: { start: { line: 1, column: 0 }, end: { line: 1, column: 10 } } }
201-
writeFileSync(join(cellDir, 'coverage-final.json'), JSON.stringify({
202-
'a.js': { path: 'a.js', statementMap, fnMap: {}, branchMap: {}, s: { 0: 1 }, f: {}, b: {} },
203-
}))
204-
205-
const { lcovDir, jsonDir } = mergeRunCoverage('42', input, output)
206-
207-
assert.equal(lcovDir, null)
208-
assert.equal(jsonDir, join(output, '42', 'json'))
209-
assert.equal(JSON.parse(readFileSync(join(jsonDir, 'coverage-final.json'), 'utf8'))['a.js'].s[0], 1)
210-
})
211-
212-
it('returns null for both directories when the run produced no coverage', () => {
168+
it('returns null when the run produced no coverage', () => {
213169
const input = join(dir, 'coverage-results')
214170
mkdirSync(input, { recursive: true })
215171
assert.deepEqual(
216172
mergeRunCoverage('42', input, join(dir, 'coverage-upload')),
217-
{ lcovDir: null, jsonDir: null }
173+
{ lcovDir: null }
218174
)
219175
})
220176

221-
it('skips the json merge when skipJson is set, but still merges lcov', () => {
222-
const input = join(dir, 'coverage-results')
223-
const output = join(dir, 'coverage-upload')
224-
const cellDir = join(input, '42', 'coverage-apm-integrations-axios__a-0', 'node-20-x')
225-
mkdirSync(cellDir, { recursive: true })
226-
writeFileSync(join(cellDir, 'lcov.info'), 'SF:a.js\nDA:1,1\nend_of_record\n')
227-
writeFileSync(join(cellDir, 'coverage-final.json'), JSON.stringify({}))
228-
229-
const { lcovDir, jsonDir } = mergeRunCoverage('42', input, output, true)
230-
231-
assert.equal(lcovDir, join(output, '42', 'lcov'))
232-
assert.equal(jsonDir, null)
233-
assert.equal(existsSync(join(output, '42', 'json')), false)
234-
})
235-
236177
it('ignores other runs\' cells', () => {
237178
const input = join(dir, 'coverage-results')
238179
const otherCellDir = join(input, '7', 'coverage-appsec-express__job-0', 'node-20-x')
@@ -241,7 +182,7 @@ describe('group-coverage', () => {
241182

242183
assert.deepEqual(
243184
mergeRunCoverage('42', input, join(dir, 'coverage-upload')),
244-
{ lcovDir: null, jsonDir: null }
185+
{ lcovDir: null }
245186
)
246187
assert.equal(existsSync(join(dir, 'coverage-upload', '42')), false)
247188
})

0 commit comments

Comments
 (0)