mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-03 07:18:03 +01:00
fix: pre-landing review fixes (GST-1024: refuse a paid-only selection; validate skipped)
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
a09efc439e
commit
111d7b0f71
4 files changed
+21
-6
No files matched your search
@@ -2,7 +2,7 @@
|
||||
|
||||
### Added
|
||||
- **`.gstack/pregate.json` `remote_gate_runs`:** the `ci-reuse/free-tests` profile, with `scripts/test-free-shards.ts --list` as the inventory (the complete free suite the Free Tests plan shards).
|
||||
- **`remote_gate_runs.skipped`** (optional, `lib/run-evidence.ts`, `lib/pregate/index.ts`, `lib/pregate/run-check.ts`): a command that reads the selected files from `PREGATE_FILES_FILE` and prints the ones the remote gate itself never runs. Those files need no run, and the receipt records them as `skipped=<n>`. An inventory file stays required even when `skipped` names it, and a failing command refuses with `skipped_failed`. gstack's command prints the selected paid test files, the same split `scripts/ubicloud/pregate-remote.sh` makes. Without it, run 41484634592 still refused: all 71 of the selected files it didn't cover are paid.
|
||||
- **`remote_gate_runs.skipped`** (optional, `lib/run-evidence.ts`, `lib/pregate/index.ts`, `lib/pregate/run-check.ts`): a command that reads the selected files from `PREGATE_FILES_FILE` and prints the ones the remote gate itself never runs. Those files need no run, and the receipt records them as `skipped=<n>`. An inventory file stays required even when `skipped` names it, and a failing command refuses with `skipped_failed`. When every selected file is skipped, the clear refuses with `paid_only`, just as the gate's own `tests=0` report never clears. gstack's command prints the selected paid test files, the same split `scripts/ubicloud/pregate-remote.sh` makes. Without it, run 41484634592 still refused: all 71 of the selected files it didn't cover are paid.
|
||||
|
||||
### Tests
|
||||
- `test/pregate-remote-gate-runs.test.ts`: gstack's inventory equals the Free Tests plan's file set, and its skipped command prints exactly the paid files of a mixed list. In a fixture with gstack's real profile, a green Free Tests `pull_request` run clears the gate. A run at another tree, a run of another workflow, a failed or cancelled run, a fork run, a merge commit behind its base, an uncovered inventory file, a skipped list naming an inventory file, a failing skipped command, and a declaration without `skipped` all refuse. On main, all three tests fail.
|
||||
- `test/pregate-remote-gate-runs.test.ts`: gstack's inventory equals the Free Tests plan's file set, and its skipped command prints exactly the paid files of a mixed list. In a fixture with gstack's real profile, a green Free Tests `pull_request` run clears the gate. A run at another tree, a run of another workflow, a failed or cancelled run, a fork run, a merge commit behind its base, an uncovered inventory file, a skipped list naming an inventory file, a failing skipped command, a paid-only selection, and a declaration without `skipped` all refuse. Malformed `skipped` values fail config validation. On main, all three tests fail.
|
||||
+4
-2
@@ -740,7 +740,9 @@ Then, for the lane:
|
||||
(gstack's gate leaves paid test files to the paid PR gate). Those files need
|
||||
no run, and the receipt counts them as `skipped=<n>`. A file the inventory
|
||||
prints stays required even when `skipped` names it, and a failing `skipped`
|
||||
command refuses (`skipped_failed`). Without `skipped`, every selected file
|
||||
command refuses (`skipped_failed`). When `skipped` names every selected file,
|
||||
no selected test ran, so the clear refuses (`paid_only`), just as the gate's
|
||||
own `tests=0` report never clears. Without `skipped`, every selected file
|
||||
must appear in a run's `files`.
|
||||
- Every run is read once more after the inventory: a run that moved to a new
|
||||
attempt or stopped being a success meanwhile refuses (`run_changed`).
|
||||
@@ -753,7 +755,7 @@ verification is named with its own reason (`fork_run`, `event_mismatch`,
|
||||
`os_mismatch`, `api_error`, …); the clear-level codes are `base_mismatch`,
|
||||
`no_verified_run`, `tree_mismatch` (runs on different trees),
|
||||
`tree_not_stamp_only`, `no_tests`, `inventory_failed`, `inventory_empty`,
|
||||
`skipped_failed`, `inventory_uncovered` (the remote gate) and `tests_uncovered` (a lane's own
|
||||
`skipped_failed`, `paid_only`, `inventory_uncovered` (the remote gate) and `tests_uncovered` (a lane's own
|
||||
selected tests). A wrong run count, a repeated run or a lane with no
|
||||
profile is a usage error (exit 2), not a REUSE refusal.
|
||||
|
||||
|
||||
@@ -371,6 +371,7 @@ function clearByRuns(file: PregateFile, lane: string, refs: string[], check: Run
|
||||
if ('error' in skipped) return { refused: 'PREGATE_RECEIPT_INVALID', message: `REUSE: refused reason=skipped_failed — the remote gate's skipped command failed: ${skipped.error}; ${lane} stays uncleared` };
|
||||
const inventory = new Set(listed);
|
||||
for (const f of skipped) if (!inventory.has(f) && required.delete(f)) skippedCount++;
|
||||
if (!required.size) return { refused: 'PREGATE_RECEIPT_INVALID', message: `REUSE: refused reason=paid_only — the remote gate skips every selected test file (${skippedCount}), so no selected test ran; like the gate's own tests=0 report, that never clears; ${lane} stays uncleared` };
|
||||
}
|
||||
for (const f of listed) required.add(f);
|
||||
}
|
||||
|
||||
@@ -121,7 +121,7 @@ describe.skipIf(process.platform === 'win32')('clear --run with the Free Tests p
|
||||
expect(msg(d)).toStartWith(`cleared remote-gate tree=${file.tree.slice(0, 12)} tests=${files.length * 20} skipped=1 runs=41484634592/1 profiles=${PROFILE} heads=${head.slice(0, 12)} verified=`);
|
||||
}, 120_000);
|
||||
|
||||
test('refusals: no skipped command, the wrong tree, the wrong workflow, a failed or cancelled run, a fork run, a merge behind its base, an uncovered inventory file, a skipped list that names an inventory file', async () => {
|
||||
test('refusals: no skipped command, a paid-only selection, the wrong tree, the wrong workflow, a failed or cancelled run, a fork run, a merge behind its base, an uncovered inventory file, a skipped list that names an inventory file', async () => {
|
||||
const repo = fixture({ profiles: [PROFILE], inventory: INVENTORY, skipped: SKIPPED });
|
||||
const older = repo.git('rev-parse', 'HEAD~1');
|
||||
const head = repo.git('rev-parse', 'HEAD');
|
||||
@@ -136,7 +136,19 @@ describe.skipIf(process.platform === 'win32')('clear --run with the Free Tests p
|
||||
expect(clear({ head, files, headRepo: 'attacker/fx' })).toContain('reason=fork_run run 41484634592 attempt 1 ran for attacker/fx, not fx/fx');
|
||||
expect(clear({ head, files, mergeTree: repo.git('rev-parse', `${older}^{tree}`) })).toContain('reason=tree_mismatch');
|
||||
expect(clear({ head, files: files.filter(f => f !== 'test/other.test.js') })).toContain("reason=inventory_uncovered — 1 of the");
|
||||
expect(clear({ head, files: files.filter(f => f !== 'test/other.test.js') }, { gateRuns: { profiles: [PROFILE], inventory: INVENTORY, skipped: 'cat "$PREGATE_FILES_FILE"; echo test/other.test.js' } })).toContain('(test/other.test.js)');
|
||||
expect(clear({ head, files: files.filter(f => f !== 'test/other.test.js') }, { gateRuns: { profiles: [PROFILE], inventory: INVENTORY, skipped: `${SKIPPED}; echo test/other.test.js` } })).toContain('(test/other.test.js)');
|
||||
expect(clear({ head, files }, { gateRuns: { profiles: [PROFILE], inventory: INVENTORY, skipped: 'exit 3' } })).toContain('reason=skipped_failed');
|
||||
const paidOnly = structuredClone(file);
|
||||
paidOnly.requires_remote!.find(r => r.lane === LANE)!.tests = [PAID];
|
||||
expect(msg(clearDecision(paidOnly, LANE, { runs: [RUN], evidence: gateCheck(repo, { head, files }) }, repo.work))).toBe(`PREGATE_RECEIPT_INVALID: REUSE: refused reason=paid_only — the remote gate skips every selected test file (1), so no selected test ran; like the gate's own tests=0 report, that never clears; ${LANE} stays uncleared`);
|
||||
}, 120_000);
|
||||
|
||||
test('remote_gate_runs.skipped must be one command line, and an unknown key names it as valid', () => {
|
||||
const profiles = { p: { workflow: 'w.yml', events: ['push'], jobs: ['j'], artifact: 'a' } };
|
||||
for (const skipped of [3, '', ' ', 'a\nb', 'x'.repeat(513)]) {
|
||||
expect(runProfileErrors(profiles, { profiles: ['p'], inventory: 'echo t', skipped }).map(e => e.path)).toEqual(['remote_gate_runs.skipped']);
|
||||
}
|
||||
expect(runProfileErrors(profiles, { profiles: ['p'], inventory: 'echo t', skipped: 'echo s' })).toEqual([]);
|
||||
expect(runProfileErrors(profiles, { profiles: ['p'], inventory: 'echo t', skip: 'echo s' })).toEqual([{ path: 'remote_gate_runs.skip', message: 'unknown key; valid keys: profiles, inventory, skipped' }]);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user