Skip to content

Commit a6f943b

Browse files
committed
Reserve annotation room for the teardown banner independent of the primary excerpt
annotationBody keeps only its own 5-line window of non-empty lines, distinct from the excerpt's 20-line budget the primary message was reserved against. A primary message with 5+ non-empty lines already filled that window, silently dropping the teardown banner and message from the annotation even though both still fit in the fuller step-summary excerpt.
1 parent f3e0bdf commit a6f943b

3 files changed

Lines changed: 66 additions & 2 deletions

File tree

‎src/tools/github.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -646,6 +646,12 @@ export function summaryClusterBlock(cluster: FailureCluster): string {
646646
// target or claim any row exists; it can only point at the step log.
647647
export type SummaryRowOutcome = "detail" | "name-only" | "none";
648648

649+
// Non-empty excerpt lines annotationBody keeps by default. Exported so
650+
// callers building the excerpt (tests/test.ts) can reserve room within this
651+
// same budget for content that must survive alongside the excerpt, such as a
652+
// teardown-failure banner.
653+
export const kAnnotationExcerptLines = 5;
654+
649655
// Trimmed annotation message: the repro, a blank line, the first `maxLines`
650656
// non-empty excerpt lines (byte-capped defensively, in case a caller passes
651657
// an excerpt it did not already bound), an ellipsis, then a pointer whose
@@ -655,7 +661,7 @@ export function annotationBody(
655661
excerpt: string,
656662
label: string,
657663
outcome: SummaryRowOutcome,
658-
maxLines = 5,
664+
maxLines = kAnnotationExcerptLines,
659665
): string {
660666
const allLines = stripAnsi(truncateUtf8Bytes(excerpt, kExcerptMaxBytes))
661667
.split("\n")

‎tests/test.ts‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import {
3434
error as ghError,
3535
harnessOwnsStep,
3636
isGitHubActions,
37+
kAnnotationExcerptLines,
3738
kExcerptMaxBytes,
3839
stripAnsi,
3940
summaryClusterBlock,
@@ -63,6 +64,11 @@ const kExcerptLines = 20;
6364
// failure will also be reported: the blank separator, the banner, and at
6465
// least the first line of the secondary message (see reportFailure).
6566
const kFinallyReservedLines = 3;
67+
// Same reservation, but sized for annotationBody's much smaller
68+
// kAnnotationExcerptLines window rather than kExcerptLines: the banner and
69+
// the first line of the secondary message, with no separator line (the
70+
// annotation excerpt is built fresh, not sliced from the primary excerpt).
71+
const kFinallyAnnotationReservedLines = 2;
6672
const annotationBudget = new AnnotationBudget();
6773
// Per-file header flag: each failing test file starts its own summary table.
6874
let summaryHeaderEmitted = false;
@@ -508,6 +514,26 @@ function reportFailure(
508514
kExcerptMaxBytes,
509515
);
510516

517+
// annotationBody keeps only its first kAnnotationExcerptLines NON-EMPTY
518+
// lines of whatever excerpt it's given - a much smaller window than the
519+
// kExcerptLines the primary message above was reserved against. A
520+
// primary message with as few as kAnnotationExcerptLines non-empty lines
521+
// already fills that window, silently dropping the teardown banner and
522+
// message from the annotation even though both still fit in the fuller
523+
// step-summary excerpt. Build the annotation body from a dedicated,
524+
// reserved excerpt instead of reusing `excerpt` when a teardown/cleanup
525+
// failure rides along; otherwise reuse `excerpt` unchanged.
526+
const annotationExcerpt = finallyFailure
527+
? [
528+
...stripAnsi(primary?.message ?? "")
529+
.split("\n")
530+
.filter((line) => line.trim().length > 0)
531+
.slice(0, kAnnotationExcerptLines - kFinallyAnnotationReservedLines),
532+
bannerFor(finallyFailure.phase),
533+
describeThrow(finallyFailure.value).message,
534+
].join("\n")
535+
: excerpt;
536+
511537
// Record the failure step-wide (sidecar counter) to get its
512538
// ordinal, then build the navigation label. This runs for EVERY
513539
// CI failure — including orchestrated bucket legs, whose rows
@@ -587,7 +613,7 @@ function reportFailure(
587613
// cross-reference their summary entry.
588614
if (harnessOwnsStep()) {
589615
if (decision.emitAnnotation) {
590-
ghError(annotationBody(repro, excerpt, label, rowOutcome), {
616+
ghError(annotationBody(repro, annotationExcerpt, label, rowOutcome), {
591617
file: annotationFile,
592618
title: `${label} · ${ctx.testName}`,
593619
});

‎tests/unit/harness-failure-reporting.test.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,21 @@ unitTest("harness-reporting - a many-line primary message still leaves room for
310310
run.summary.includes("FIXTURE_MULTILINE_TEARDOWN_BOOM"),
311311
"the teardown failure message must survive too",
312312
);
313+
const annotations = annotationsFor(
314+
run.stdout,
315+
"fixture throws a multi-line message and teardown also throws",
316+
);
317+
assertEquals(annotations.length, 1);
318+
assert(
319+
annotations[0].includes("TEARDOWN ALSO FAILED:"),
320+
`the annotation's own 5-line window must also survive the many-line primary; annotation was:\n${
321+
annotations[0]
322+
}`,
323+
);
324+
assert(
325+
annotations[0].includes("FIXTURE_MULTILINE_TEARDOWN_BOOM"),
326+
"the annotation must also name the teardown failure, not just the summary",
327+
);
313328
});
314329

315330
unitTest("harness-reporting - a primary message that trips both the line and byte caps still leaves room for the teardown banner and message", async () => {
@@ -329,6 +344,23 @@ unitTest("harness-reporting - a primary message that trips both the line and byt
329344
run.summary.includes("FIXTURE_BOTHCAPS_TEARDOWN_BOOM"),
330345
"the teardown failure message must survive too",
331346
);
347+
// escapeProperty encodes "," to "%2C" in the annotation title, so match on
348+
// a comma-free slice of the fixture name rather than the literal string.
349+
const annotations = annotationsFor(
350+
run.stdout,
351+
"with teardown throwing too",
352+
);
353+
assertEquals(annotations.length, 1);
354+
assert(
355+
annotations[0].includes("TEARDOWN ALSO FAILED:"),
356+
`the annotation's own 5-line window must also survive both caps; annotation was:\n${
357+
annotations[0]
358+
}`,
359+
);
360+
assert(
361+
annotations[0].includes("FIXTURE_BOTHCAPS_TEARDOWN_BOOM"),
362+
"the annotation must also name the teardown failure, not just the summary",
363+
);
332364
});
333365

334366
unitTest("harness-reporting - a passing test produces no row and no annotation", async () => {

0 commit comments

Comments
 (0)