Skip to content

Commit 154605e

Browse files
committed
refactor(ci): drop the artifact-policy changes and the narration comments
The hidden-path upload fixes and their guard move to their own PR, per review: one rule, ten uploads, artifacts with different owners. This branch keeps the diagnose loop's own behaviour. Comments cut to the AGENTS.md rule added in #2087. The reader's narration is gone; what it was explaining now lives in names and types — `MeasuredVerdict` separates xcodebuild's three words from the two the loop derives, and `exitContradictsMeasured` names the rule that a nonzero exit outranks a green measured test. The shape table's per-case comments are gone too; each case's `name` already says what it is. What remains is two constants citing measured CI numbers, which cannot be encoded.
1 parent be6622f commit 154605e

11 files changed

Lines changed: 12 additions & 101 deletions

‎.github/workflows/1874-diagnose.yml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,6 @@ jobs:
131131
# so every per-iteration log this loop has ever kept was silently dropped, including on
132132
# the run that motivated the slow-pass capture. Same input the shared diagnostics action
133133
# sets, for the same reason.
134-
include-hidden-files: true
135134
path: |
136135
stall-summary.txt
137136
.tmp/stall-evidence-*.log

‎.github/workflows/macos.yml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,6 @@ jobs:
122122
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
123123
with:
124124
name: xctest-host-results-${{ github.run_id }}-${{ github.run_attempt }}
125-
include-hidden-files: true
126125
path: .tmp/xctest-host
127126
if-no-files-found: warn
128127

‎.github/workflows/mutation-affected.yml‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,6 @@ jobs:
8181
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
8282
with:
8383
name: mutation-affected-select
84-
include-hidden-files: true
8584
path: .tmp/mutation/lane-envelope.json
8685
if-no-files-found: warn
8786

@@ -123,7 +122,6 @@ jobs:
123122
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
124123
with:
125124
name: mutation-affected-shard-${{ matrix.name }}
126-
include-hidden-files: true
127125
path: |
128126
.tmp/mutation/mutation.json
129127
.tmp/mutation/mutation.html
@@ -177,7 +175,6 @@ jobs:
177175
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
178176
with:
179177
name: mutation-affected
180-
include-hidden-files: true
181178
path: |
182179
.tmp/mutation/shards
183180
.tmp/mutation/lane-envelope.json

‎.github/workflows/mutation-weekly.yml‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,6 @@ jobs:
8383
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
8484
with:
8585
name: mutation-shard-${{ matrix.name }}
86-
include-hidden-files: true
8786
path: |
8887
.tmp/mutation/mutation.json
8988
.tmp/mutation/lane-envelope.json
@@ -161,7 +160,6 @@ jobs:
161160
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
162161
with:
163162
name: mutation-decision-kernels
164-
include-hidden-files: true
165163
path: |
166164
.tmp/mutation/shards
167165
.tmp/mutation/lane-envelope.json

‎.github/workflows/replays-nightly.yml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,6 @@ jobs:
9292
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
9393
with:
9494
name: parser-fuzz-run-${{ github.run_id }}-${{ github.run_attempt }}
95-
include-hidden-files: true
9695
path: .tmp/fuzz
9796
if-no-files-found: warn
9897

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,6 @@ jobs:
230230
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
231231
with:
232232
name: ${{ matrix.artifactName }}
233-
include-hidden-files: true
234233
path: .tmp/test-app-artifact/binary.tar.gz
235234
if-no-files-found: error
236235
compression-level: 0 # already gzipped

‎.github/workflows/xctest-nightly.yml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,5 @@ jobs:
186186
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
187187
with:
188188
name: xctest-nightly-results-${{ github.run_id }}-${{ github.run_attempt }}
189-
include-hidden-files: true
190189
path: .tmp/xctest-nightly
191190
if-no-files-found: warn

‎scripts/__tests__/diagnose-1874-iteration.test.ts‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ const script = path.join(
1212
);
1313
const NAME = 'testBareTypeUsesTappedInputWhenSoftwareKeyboardIsHidden';
1414
const POLL = '[DEBUG-1874] poll t=1ms observedLen=0 expectedPrefixLen=0';
15-
/** A real wait brackets its polls, so cadence lines and poll count are not the same number. */
15+
/** A wait brackets its polls, so cadence lines and poll count differ. */
1616
const CADENCE = [
1717
'[DEBUG-1874] wait start expectedLen=17',
1818
POLL,
@@ -21,7 +21,7 @@ const CADENCE = [
2121

2222
const verdictLine = (test: string, word: string) =>
2323
`Test Case '-[X.RunnerTests ${test}]' ${word} (1.0 seconds).`;
24-
/** One `type` logs three phases; only `type-all` is the dispatch the threshold judges. */
24+
/** One `type` logs three phases; the threshold reads `type-all`. */
2525
const phases = (typeAllMs: string) =>
2626
[
2727
`phase=focus durationMs=644.6`,
@@ -46,7 +46,6 @@ const SHAPES: readonly (Partial<IterationReport> & { name: string; log: string[]
4646
lines: ['iter=1 verdict=passed rc=0 durationMs=14146 polls=1', ...CADENCE],
4747
},
4848
{
49-
// Since #2035 the test skips on an ambient software keyboard, and a skip logs no phase at all.
5049
name: 'skipped — an environment flip, not a stall',
5150
log: [verdictLine(NAME, 'skipped')],
5251
verdict: 'skipped',
@@ -65,8 +64,6 @@ const SHAPES: readonly (Partial<IterationReport> & { name: string; log: string[]
6564
verdict: 'no-result',
6665
},
6766
{
68-
// `pair` mode runs testBareDelayedType… alongside, and it sorts first, so its phases and
69-
// verdict land in the same log ahead of the measured test's — whose 812 ms is the answer.
7067
name: 'pair mode — the neighbour test logs first',
7168
log: [
7269
verdictLine('testBareDelayedTypeFailsWhenTappedInputDisappearsMidCommand', 'passed'),
@@ -78,15 +75,12 @@ const SHAPES: readonly (Partial<IterationReport> & { name: string; log: string[]
7875
keepEvidence: false,
7976
},
8077
{
81-
// xcodebuild restarts a test after an unexpected exit, so one test can report twice.
8278
name: 'restarted — the final word is the outcome',
8379
log: [verdictLine(NAME, 'failed'), ...phases('900.0'), verdictLine(NAME, 'passed')],
8480
verdict: 'passed',
8581
keepEvidence: false,
8682
},
8783
{
88-
// `pair` mode: the measured test passed, but the neighbour or the runner made xcodebuild exit
89-
// nonzero. Counting that as a clean pass would both miscount it and throw its log away.
9084
name: 'measured test passed, xcodebuild did not',
9185
log: [verdictLine(NAME, 'passed'), ...phases('796.1'), ...CADENCE],
9286
rc: 65,
@@ -110,14 +104,10 @@ test.each(SHAPES)('reads a $name iteration', (shape) => {
110104
const report = readIteration(shape.log.join('\n'), NAME, 1, shape.rc ?? 0);
111105
expect(report.verdict).toBe(shape.verdict);
112106
if (shape.keepEvidence !== undefined) expect(report.keepEvidence).toBe(shape.keepEvidence);
113-
// Asserting the rendered lines rather than a duration field: it is what the threshold judges
114-
// and what a reader of the artifact sees, so an extraction off by a digit has to show here.
115107
if (shape.lines) expect(report.lines).toEqual(shape.lines);
116108
});
117109

118110
test('the test name is matched literally, not as a pattern', () => {
119-
// Built as a regex, `testA.B` would match this line and report it passed (CodeQL: regex
120-
// injection from a command-line argument).
121111
const log = verdictLine('testAXB', 'passed');
122112
expect(readIteration(log, 'testA.B', 1, 0).verdict).toBe('no-result');
123113
expect(readIteration(log, 'testAXB', 1, 0).verdict).toBe('passed');

‎scripts/diagnose-1874-iteration.ts‎

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,20 @@
1-
// Reads one iteration of the #1874 diagnose loop (.github/workflows/1874-diagnose.yml) and writes
2-
// what is worth keeping. A script rather than shell in the workflow, for the reason
3-
// `xctest-run-summary.ts` gives: parsing inside YAML inside shell. Four defects shipped while this
4-
// was shell, every one of them a pipeline treating "nothing matched" as failure under `pipefail`.
1+
// Reads one iteration of the #1874 diagnose loop (.github/workflows/1874-diagnose.yml).
52

63
import fs from 'node:fs';
74
import path from 'node:path';
85
import { pathToFileURL } from 'node:url';
96

10-
/** Healthy iterations ran 628-1131 ms (n=50); the episode on record is 2487 ms. */
7+
/** Measured on CI: healthy iterations 628-1131 ms (n=50), the episode on record 2487 ms. */
118
const SLOW_PASS_MS = 2000;
129
const CADENCE_LIMIT = 40;
1310
const SUMMARY = 'stall-summary.txt';
1411
const EVIDENCE_DIR = '.tmp';
1512

13+
/** xcodebuild's own word for the measured test. */
14+
type MeasuredVerdict = 'passed' | 'skipped' | 'failed';
15+
1616
export type IterationReport = {
17-
/**
18-
* xcodebuild's own word for the measured test, plus two the loop needs itself: `no-result` when
19-
* the run produced no verdict — the selection matched nothing or the bundle never ran — and
20-
* `run-failed` when the measured test passed but xcodebuild still exited nonzero, which in
21-
* `pair` mode means the neighbour test or the runner failed. Both are ours, not the product's,
22-
* and neither is evidence about the stall.
23-
*/
24-
readonly verdict: 'passed' | 'skipped' | 'failed' | 'no-result' | 'run-failed';
25-
/** Since #2035 an absorbed episode passes, so a slow pass is evidence too. */
17+
readonly verdict: MeasuredVerdict | 'no-result' | 'run-failed';
2618
readonly keepEvidence: boolean;
2719
readonly lines: readonly string[];
2820
};
@@ -34,8 +26,6 @@ export function readIteration(
3426
rc: number,
3527
): IterationReport {
3628
const lines = log.split('\n');
37-
// Substring, not a pattern built from the argument: a test name is not a regex, and treating it
38-
// as one lets a metacharacter match a different test.
3929
const marker = `${testName}]' `;
4030
const word = lines
4131
.filter((line) => line.includes(marker))
@@ -47,8 +37,6 @@ export function readIteration(
4737
// it as a clean pass would both miscount it and throw the log away.
4838
const verdict =
4939
rc !== 0 && measured !== 'failed' && measured !== 'no-result' ? 'run-failed' : measured;
50-
// Qualified on `type-all`: an iteration also logs `phase=focus` and `phase=total`, and the
51-
// threshold is calibrated on the dispatch.
5240
const durationMs = Number(
5341
lines.flatMap((line) => /phase=type-all durationMs=(\d+)/.exec(line)?.[1] ?? []).at(-1) ?? 0,
5442
);
@@ -66,12 +54,15 @@ export function readIteration(
6654
: [
6755
summary,
6856
...cadence.slice(0, CADENCE_LIMIT),
69-
// A cap that hides how much it dropped reads as "this is all there was".
7057
...(dropped > 0 ? [`… ${dropped} more DEBUG-1874 lines (full log in the artifact)`] : []),
7158
],
7259
};
7360
}
7461

62+
function isMeasuredVerdict(word: string | undefined): word is MeasuredVerdict {
63+
return word === 'passed' || word === 'skipped' || word === 'failed';
64+
}
65+
7566
function main(): number {
7667
const [logPath, testName, iteration, rc] = process.argv.slice(2);
7768
if (!logPath || !testName || !iteration || !rc) {

‎test/ci/upload-artifact-hidden-paths.test.ts‎

Lines changed: 0 additions & 57 deletions
This file was deleted.

0 commit comments

Comments
 (0)