Skip to content

Commit 687e2ed

Browse files
authored
Identify failed jobs in retry reports (#59569) (#59615)
Summary: Replace opaque retry-report failures with summaries and annotations identifying the failing job, configuration, steps, and direct log link. Preserve separate matrix failure details and retain a run-linked fallback when job diagnostics are unavailable. Apply the shared reporter to Android/iOS RNTester, template-app, and Fantom workflows while keeping intermediate retries non-fatal and failing the final gate. Changelog: [Internal] Reviewed By: cipolleschi Differential Revision: D128039792
1 parent 4930592 commit 687e2ed

8 files changed

Lines changed: 718 additions & 50 deletions

File tree

Lines changed: 388 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,388 @@
1+
/**
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @format
8+
*/
9+
10+
const {recordJobFailure, reportTestResults} = require('../report-test-results');
11+
12+
const context = {
13+
serverUrl: 'https://github.com',
14+
repo: {owner: 'react', repo: 'react-native'},
15+
runId: 40446700902,
16+
job: 'test',
17+
};
18+
const jobUrl =
19+
'https://github.com/react/react-native/actions/runs/40446700902/job/122302861049';
20+
const failedTest = {outcome: 'failure', conclusion: 'success'};
21+
22+
function createCore() {
23+
const core = {
24+
setOutput: jest.fn(),
25+
setFailed: jest.fn(),
26+
warning: jest.fn(),
27+
summary: {addRaw: jest.fn(), write: jest.fn().mockResolvedValue(undefined)},
28+
};
29+
core.summary.addRaw.mockReturnValue(core.summary);
30+
return core;
31+
}
32+
33+
function createGithub(jobs) {
34+
return {
35+
paginate: jest.fn().mockResolvedValue(jobs),
36+
rest: {actions: {listJobsForWorkflowRunAttempt: jest.fn()}},
37+
};
38+
}
39+
40+
function job(name, url = jobUrl) {
41+
return {name, html_url: url, runner_name: 'runner', status: 'in_progress'};
42+
}
43+
44+
function recordArguments(core, github, steps = {'run-tests': failedTest}) {
45+
return {
46+
context,
47+
core,
48+
github,
49+
steps,
50+
flavor: 'debug',
51+
runnerName: 'runner',
52+
attempt: 2,
53+
jobStatus: 'success',
54+
};
55+
}
56+
57+
async function render(needs, failOnError = true) {
58+
const core = createCore();
59+
await reportTestResults({context, core, needs, failOnError, attempt: 3});
60+
return core;
61+
}
62+
63+
describe('test job failure recording', () => {
64+
it('records tolerated test failures with their exact job name and log link', async () => {
65+
const core = createCore();
66+
const name = 'test_e2e_android_rntester_retry_2 / test (debug)';
67+
const github = createGithub([job(name)]);
68+
await recordJobFailure(recordArguments(core, github));
69+
expect(core.setOutput).toHaveBeenCalledWith('status', 'failure');
70+
expect(core.setOutput).toHaveBeenCalledWith(
71+
'failure-debug',
72+
JSON.stringify({name, url: jobUrl, steps: ['run-tests']}),
73+
);
74+
expect(github.paginate).toHaveBeenCalledWith(
75+
github.rest.actions.listJobsForWorkflowRunAttempt,
76+
{
77+
owner: 'react',
78+
repo: 'react-native',
79+
run_id: context.runId,
80+
attempt_number: 2,
81+
per_page: 100,
82+
},
83+
);
84+
});
85+
86+
it('records failures in steps without an explicit id using the job status and API', async () => {
87+
const core = createCore();
88+
const github = createGithub([
89+
{
90+
...job('template / build (Debug)'),
91+
steps: [{name: 'Build the app', conclusion: 'failure'}],
92+
},
93+
]);
94+
await recordJobFailure({
95+
...recordArguments(core, github, {}),
96+
context: {...context, job: 'build'},
97+
flavor: 'Debug',
98+
jobStatus: 'failure',
99+
});
100+
expect(JSON.parse(core.setOutput.mock.calls[1][1])).toEqual({
101+
name: 'template / build (Debug)',
102+
url: jobUrl,
103+
steps: ['Build the app'],
104+
});
105+
});
106+
107+
it('does not overwrite matrix failures when another configuration passes', async () => {
108+
const core = createCore();
109+
const github = createGithub([]);
110+
await recordJobFailure(
111+
recordArguments(core, github, {
112+
'run-tests': {outcome: 'success', conclusion: 'success'},
113+
}),
114+
);
115+
expect(core.setOutput).not.toHaveBeenCalled();
116+
expect(github.paginate).not.toHaveBeenCalled();
117+
});
118+
119+
it('ignores tolerated state-download failures when tests pass', async () => {
120+
const core = createCore();
121+
const github = createGithub([]);
122+
await recordJobFailure(
123+
recordArguments(core, github, {
124+
download: {outcome: 'failure', conclusion: 'success'},
125+
'run-tests': {outcome: 'success', conclusion: 'success'},
126+
}),
127+
);
128+
expect(core.setOutput).not.toHaveBeenCalled();
129+
});
130+
131+
it.each(['failure', 'cancelled'])(
132+
'records an upstream step with outcome %s',
133+
async outcome => {
134+
const core = createCore();
135+
const github = createGithub([job('test (debug)')]);
136+
await recordJobFailure(
137+
recordArguments(core, github, {
138+
setup: {outcome, conclusion: outcome},
139+
'run-tests': {outcome: 'skipped', conclusion: 'skipped'},
140+
}),
141+
);
142+
expect(JSON.parse(core.setOutput.mock.calls[1][1]).steps).toEqual([
143+
'setup',
144+
]);
145+
},
146+
);
147+
148+
it('preserves the failure when the API lookup fails', async () => {
149+
const core = createCore();
150+
const github = createGithub([]);
151+
github.paginate.mockRejectedValue(new Error('permission denied'));
152+
await recordJobFailure(recordArguments(core, github));
153+
expect(core.setOutput).toHaveBeenCalledWith('status', 'failure');
154+
expect(JSON.parse(core.setOutput.mock.calls[1][1])).toEqual({
155+
name: 'test (debug)',
156+
url: `${context.serverUrl}/react/react-native/actions/runs/${context.runId}/attempts/2`,
157+
steps: ['run-tests'],
158+
});
159+
expect(core.warning).toHaveBeenCalled();
160+
});
161+
162+
it.each(['debug', 'release'])(
163+
'identifies the %s job when concurrent matrix jobs share a runner name',
164+
async flavor => {
165+
const core = createCore();
166+
const name = `caller / test (${flavor})`;
167+
const url = `${jobUrl}-${flavor}`;
168+
const github = createGithub([
169+
job('caller / test (release)', `${jobUrl}-release`),
170+
job('caller / test (debug)', `${jobUrl}-debug`),
171+
]);
172+
await recordJobFailure({...recordArguments(core, github), flavor});
173+
expect(core.setOutput).toHaveBeenCalledWith(
174+
`failure-${flavor}`,
175+
JSON.stringify({name, url, steps: ['run-tests']}),
176+
);
177+
expect(core.warning).not.toHaveBeenCalled();
178+
},
179+
);
180+
181+
it('distinguishes build and test jobs on the same runner', async () => {
182+
const core = createCore();
183+
const github = createGithub([
184+
job('caller / test (debug)'),
185+
job('caller / build (debug)', `${jobUrl}-build`),
186+
]);
187+
await recordJobFailure({
188+
...recordArguments(core, github, {}),
189+
context: {...context, job: 'build'},
190+
jobStatus: 'failure',
191+
});
192+
expect(JSON.parse(core.setOutput.mock.calls[1][1])).toEqual({
193+
name: 'caller / build (debug)',
194+
url: `${jobUrl}-build`,
195+
steps: ['job failure'],
196+
});
197+
});
198+
199+
it('does not link a completed retry when the current job is not listed', async () => {
200+
const core = createCore();
201+
const github = createGithub([
202+
{...job('earlier / test (debug)'), status: 'completed'},
203+
]);
204+
await recordJobFailure(recordArguments(core, github));
205+
expect(JSON.parse(core.setOutput.mock.calls[1][1]).url).toBe(
206+
`${context.serverUrl}/react/react-native/actions/runs/${context.runId}/attempts/2`,
207+
);
208+
expect(core.warning).toHaveBeenCalled();
209+
});
210+
211+
it('selects only the current job on a reused runner', async () => {
212+
const core = createCore();
213+
const github = createGithub([
214+
{...job('earlier / test (debug)'), status: 'completed'},
215+
job('current / test (debug)'),
216+
{...job('other / test (debug)'), runner_name: 'other-runner'},
217+
]);
218+
await recordJobFailure(recordArguments(core, github));
219+
expect(JSON.parse(core.setOutput.mock.calls[1][1]).name).toBe(
220+
'current / test (debug)',
221+
);
222+
});
223+
224+
it('falls back rather than linking to an ambiguous job', async () => {
225+
const core = createCore();
226+
const github = createGithub([
227+
job('first / test (debug)'),
228+
job('second / test (debug)'),
229+
]);
230+
await recordJobFailure(recordArguments(core, github));
231+
expect(JSON.parse(core.setOutput.mock.calls[1][1]).name).toBe(
232+
'test (debug)',
233+
);
234+
expect(core.warning).toHaveBeenCalled();
235+
});
236+
237+
it('normalises iOS matrix names and supports non-matrix jobs', async () => {
238+
const core = createCore();
239+
const args = recordArguments(
240+
core,
241+
createGithub([job('caller / test (Release)'), job('caller / test')]),
242+
);
243+
await recordJobFailure({...args, flavor: 'Release'});
244+
expect(core.setOutput.mock.calls[1][0]).toBe('failure-release');
245+
expect(JSON.parse(core.setOutput.mock.calls[1][1]).name).toBe(
246+
'caller / test (Release)',
247+
);
248+
await recordJobFailure({...args, flavor: undefined});
249+
expect(core.setOutput.mock.calls[3][0]).toBe('failure');
250+
expect(JSON.parse(core.setOutput.mock.calls[3][1]).name).toBe(
251+
'caller / test',
252+
);
253+
});
254+
});
255+
256+
describe('test report jobs', () => {
257+
it('reports both failing configurations even though their job conclusion is success', async () => {
258+
const debug = {
259+
name: 'test / test (debug)',
260+
url: jobUrl,
261+
steps: ['run-tests'],
262+
};
263+
const release = {
264+
...debug,
265+
name: 'test / test (release)',
266+
url: `${jobUrl}-release`,
267+
};
268+
const core = await render({
269+
test: {
270+
result: 'success',
271+
outputs: {
272+
status: 'failure',
273+
'failure-debug': JSON.stringify(debug),
274+
'failure-release': JSON.stringify(release),
275+
},
276+
},
277+
});
278+
expect(core.setOutput).toHaveBeenCalledWith('status', 'failure');
279+
expect(core.setFailed).toHaveBeenCalledWith(
280+
expect.stringContaining(debug.name),
281+
);
282+
expect(core.setFailed).toHaveBeenCalledWith(
283+
expect.stringContaining(release.name),
284+
);
285+
const summary = core.summary.addRaw.mock.calls[0][0];
286+
expect(summary).toContain(`](${debug.url})`);
287+
expect(summary).toContain(`](${release.url})`);
288+
});
289+
290+
it('does not hide an unrecorded setup failure behind another matrix failure', async () => {
291+
const core = await render({
292+
test: {
293+
result: 'failure',
294+
outputs: {
295+
status: 'failure',
296+
'failure-release': JSON.stringify({
297+
name: 'test (release)',
298+
url: jobUrl,
299+
steps: ['run-tests'],
300+
}),
301+
},
302+
},
303+
});
304+
expect(core.setFailed).toHaveBeenCalledWith(
305+
expect.stringContaining('test (release)'),
306+
);
307+
expect(core.setFailed).toHaveBeenCalledWith(
308+
expect.stringContaining('test: job failure'),
309+
);
310+
});
311+
312+
it('keeps intermediate retries non-fatal while reporting their failures', async () => {
313+
const core = await render(
314+
{test: {result: 'success', outputs: {status: 'failure'}}},
315+
false,
316+
);
317+
expect(core.setOutput).toHaveBeenCalledWith('status', 'failure');
318+
expect(core.setFailed).not.toHaveBeenCalled();
319+
expect(core.warning).toHaveBeenCalledWith(
320+
expect.stringContaining('test: run-tests'),
321+
);
322+
});
323+
324+
it.each(['failure', 'cancelled', 'skipped'])(
325+
'does not report an upstream %s as success',
326+
async result => {
327+
const core = await render({test: {result, outputs: {}}});
328+
expect(core.setFailed).toHaveBeenCalledWith(
329+
expect.stringContaining(`job ${result}`),
330+
);
331+
expect(core.setOutput).toHaveBeenCalledWith('status', 'failure');
332+
},
333+
);
334+
335+
it('reports build failures and skipped dependent tests', async () => {
336+
const core = await render({
337+
build: {
338+
result: 'failure',
339+
outputs: {
340+
'failure-debug': JSON.stringify({
341+
name: 'template / build (Debug)',
342+
url: jobUrl,
343+
steps: ['build-app'],
344+
}),
345+
},
346+
},
347+
test: {result: 'skipped', outputs: {}},
348+
});
349+
expect(core.setFailed).toHaveBeenCalledWith(
350+
expect.stringContaining('template / build (Debug)'),
351+
);
352+
expect(core.setFailed).toHaveBeenCalledWith(
353+
expect.stringContaining('test: job skipped'),
354+
);
355+
});
356+
357+
it('reports success without failing or warning', async () => {
358+
const core = await render({
359+
test: {
360+
result: 'success',
361+
outputs: {'failure-debug': '', 'failure-release': ''},
362+
},
363+
});
364+
expect(core.setOutput).toHaveBeenCalledWith('status', 'success');
365+
expect(core.setFailed).not.toHaveBeenCalled();
366+
expect(core.warning).not.toHaveBeenCalled();
367+
expect(core.summary.write).toHaveBeenCalled();
368+
});
369+
370+
it('escapes job and step names in the Markdown summary', async () => {
371+
const core = await render({
372+
test: {
373+
result: 'success',
374+
outputs: {
375+
failure: JSON.stringify({
376+
name: '<script> [job]',
377+
url: jobUrl,
378+
steps: ['a\nb*'],
379+
}),
380+
},
381+
},
382+
});
383+
expect(core.summary.addRaw.mock.calls[0][0]).toContain(
384+
'\\<script\\> \\[job\\]',
385+
);
386+
expect(core.summary.addRaw.mock.calls[0][0]).toContain('a b\\*');
387+
});
388+
});

0 commit comments

Comments
 (0)