Skip to content

Commit b40debf

Browse files
authored
fix(ci): skip release instead of erroring when both fixtures are cached (#2036)
* fix(ci): skip release instead of erroring when both fixtures are cached (#2034) map(select(.build)) yields an empty include list when both the iOS and Android fingerprints already have a trusted artifact, and GitHub Actions rejects an empty strategy.matrix at the workflow level -- so release was never created and the run was marked failure on every push since #1996 merged. Publish has-work alongside matrix and gate release on it, so the both-cached steady state now completes with release skipped instead of erroring the whole workflow. * test(ci): fold has-work regression into the existing fingerprint test Reviewer feedback on #2036: the standalone four-case test duplicated the harness above it and only two states are meaningful for this regression. Reuse the same parsed workflow, temp dir, resolver stub, and Node stub; keep neither-cached (both platforms, has-work=true) and both-cached (empty matrix, has-work=false, release gated). Drops the single-cache permutations, which exercise #1996's unchanged filtering rather than this fix. * test(ci): cover the single-cache matrix cardinality (#2036 review) Reduced coverage to 0-cached and 2-cached, leaving the 1-cached cardinality unchecked -- a mistaken \`length > 1\` in the has-work check would pass while wrongly suppressing a valid single-platform build. Generalize the Node stub to report caching per artifact-name suffix and add the iOS-cached case to the same reused harness. * test(ci): extract the has-work value instead of comparing raw output lines Thermo-nuclear review: matrix was already parsed out of its GITHUB_OUTPUT line (prefix stripped, JSON-parsed), but hasWork returned the raw "has-work=true" line, so assertions compared against a redundant 'has-work=true' string instead of the actual value. Slice the prefix the same way matrix does.
1 parent 67b813c commit b40debf

2 files changed

Lines changed: 69 additions & 22 deletions

File tree

.github/workflows/test-app-build-cache.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ jobs:
3333
timeout-minutes: 10
3434
outputs:
3535
matrix: ${{ steps.fingerprint.outputs.matrix }}
36+
has-work: ${{ steps.fingerprint.outputs.has-work }}
3637
steps:
3738
- name: Checkout
3839
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
@@ -94,10 +95,12 @@ jobs:
9495
}
9596
] | { include: map(select(.build)) }')"
9697
echo "matrix=$MATRIX" >> "$GITHUB_OUTPUT"
98+
echo "has-work=$(jq -r '.include | length > 0' <<<"$MATRIX")" >> "$GITHUB_OUTPUT"
9799
98100
release:
99101
name: ${{ matrix.name }}
100102
needs: fingerprint
103+
if: needs.fingerprint.outputs.has-work == 'true'
101104
runs-on: ${{ matrix.runsOn }}
102105
timeout-minutes: 60
103106
concurrency:

test/ci/trusted-fixture-artifact.test.mjs

Lines changed: 66 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -305,14 +305,22 @@ test('Android APK repack signs the output and preserves its package id', (t) =>
305305
assert.match(mismatch.stderr, /did not preserve the source signing certificate/);
306306
});
307307

308-
test('producer maps each platform to its resolved lookup and matrix artifact name', (t) => {
308+
test('producer maps each platform to its resolved lookup and matrix artifact name, and gates has-work when both are cached', (t) => {
309309
const workflow = parse(fs.readFileSync('.github/workflows/test-app-build-cache.yml', 'utf8'));
310310
const fingerprintStep = workflow.jobs.fingerprint.steps.find((step) => step.id === 'fingerprint');
311+
// #2034: an empty `include` matrix is legal JSON but GitHub Actions rejects it as
312+
// `strategy.matrix`, so `release` must be skipped -- not handed a zero-length
313+
// matrix -- whenever both fixtures are already cached.
314+
assert.equal(
315+
workflow.jobs.fingerprint.outputs['has-work'],
316+
'${{ steps.fingerprint.outputs.has-work }}',
317+
);
318+
assert.equal(workflow.jobs.release.if, "needs.fingerprint.outputs.has-work == 'true'");
319+
311320
const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'fixture-producer-name-'));
312321
t.after(() => fs.rmSync(tempRoot, { force: true, recursive: true }));
313322
const actionDir = path.join(tempRoot, '.github/actions/setup-fixture-app');
314323
const binDir = path.join(tempRoot, 'bin');
315-
const outputPath = path.join(tempRoot, 'output');
316324
const resolverLog = path.join(tempRoot, 'resolver-calls');
317325
const nodeLog = path.join(tempRoot, 'node-calls');
318326
fs.mkdirSync(actionDir, { recursive: true });
@@ -326,43 +334,79 @@ test('producer maps each platform to its resolved lookup and matrix artifact nam
326334
'',
327335
].join('\n'),
328336
);
329-
fs.writeFileSync(
330-
path.join(binDir, 'node'),
331-
['#!/bin/sh', 'printf "%s\\n" "$*" >> "$TEST_NODE_LOG"', ''].join('\n'),
332-
);
337+
// cachedPlatforms: which platform artifact-name suffixes (".ios", ".android")
338+
// the lookup should report as already cached.
339+
const writeNodeStub = (cachedPlatforms) =>
340+
fs.writeFileSync(
341+
path.join(binDir, 'node'),
342+
[
343+
'#!/bin/sh',
344+
'printf "%s\\n" "$*" >> "$TEST_NODE_LOG"',
345+
'case "$4" in',
346+
...cachedPlatforms.map((platform) => ` *.${platform}) printf "111" ;;`),
347+
' *) true ;;',
348+
'esac',
349+
'',
350+
].join('\n'),
351+
);
352+
writeNodeStub([]);
333353
fs.chmodSync(path.join(binDir, 'node'), 0o755);
354+
334355
const run = fingerprintStep.run
335356
.replaceAll('${{ github.event.pull_request.head.sha || github.sha }}', 'current-head')
336357
.replaceAll('${{ github.repository }}', 'octo/repo')
337358
.replaceAll('${{ github.event_name }}', 'pull_request')
338359
.replaceAll('${{ github.event.pull_request.head.repo.full_name }}', 'octo/repo');
339-
const result = spawnSync('bash', ['-c', run], {
340-
cwd: tempRoot,
341-
encoding: 'utf8',
342-
env: {
343-
...process.env,
344-
GITHUB_OUTPUT: outputPath,
345-
PATH: `${binDir}:${process.env.PATH}`,
346-
TEST_NODE_LOG: nodeLog,
347-
TEST_RESOLVER_LOG: resolverLog,
348-
},
349-
});
350-
assert.equal(result.status, 0, result.stderr);
351-
const matrixLine = fs.readFileSync(outputPath, 'utf8').trim();
352-
assert.match(matrixLine, /^matrix=/);
353-
const matrix = JSON.parse(matrixLine.slice('matrix='.length));
360+
const runFingerprint = (outputName) => {
361+
const outputPath = path.join(tempRoot, outputName);
362+
const result = spawnSync('bash', ['-c', run], {
363+
cwd: tempRoot,
364+
encoding: 'utf8',
365+
env: {
366+
...process.env,
367+
GITHUB_OUTPUT: outputPath,
368+
PATH: `${binDir}:${process.env.PATH}`,
369+
TEST_NODE_LOG: nodeLog,
370+
TEST_RESOLVER_LOG: resolverLog,
371+
},
372+
});
373+
assert.equal(result.status, 0, result.stderr);
374+
const lines = fs.readFileSync(outputPath, 'utf8').trim().split('\n');
375+
return {
376+
hasWork: lines.find((line) => line.startsWith('has-work=')).slice('has-work='.length),
377+
matrix: JSON.parse(lines.find((line) => line.startsWith('matrix=')).slice('matrix='.length)),
378+
};
379+
};
380+
381+
const neitherCached = runFingerprint('output-neither-cached');
354382
assert.deepEqual(
355-
matrix.include.map(({ platform, artifactName }) => ({ platform, artifactName })),
383+
neitherCached.matrix.include.map(({ platform, artifactName }) => ({ platform, artifactName })),
356384
[
357385
{ platform: 'ios', artifactName: 'fingerprint.ios-hash.ios' },
358386
{ platform: 'android', artifactName: 'fingerprint.android-hash.android' },
359387
],
360388
);
389+
assert.equal(neitherCached.hasWork, 'true');
361390
assert.deepEqual(fs.readFileSync(resolverLog, 'utf8').trim().split('\n'), ['ios', 'android']);
362391
assert.deepEqual(fs.readFileSync(nodeLog, 'utf8').trim().split('\n'), [
363392
'.github/actions/setup-fixture-app/trusted-artifact.mjs find octo/repo fingerprint.ios-hash.ios current-head',
364393
'.github/actions/setup-fixture-app/trusted-artifact.mjs find octo/repo fingerprint.android-hash.android current-head',
365394
]);
395+
396+
// A mistaken `length > 1` in the has-work check would pass here while
397+
// wrongly suppressing this valid single-platform build.
398+
writeNodeStub(['ios']);
399+
const iosCached = runFingerprint('output-ios-cached');
400+
assert.deepEqual(
401+
iosCached.matrix.include.map(({ platform }) => platform),
402+
['android'],
403+
);
404+
assert.equal(iosCached.hasWork, 'true');
405+
406+
writeNodeStub(['ios', 'android']);
407+
const bothCached = runFingerprint('output-both-cached');
408+
assert.deepEqual(bothCached.matrix, { include: [] });
409+
assert.equal(bothCached.hasWork, 'false');
366410
});
367411

368412
test('artifact name resolver scopes both platforms and rejects invalid output', (t) => {

0 commit comments

Comments
 (0)