Skip to content

Commit 8f446d0

Browse files
committed
Byte-cap the annotation excerpt's primary portion before appending the teardown banner
annotationBody() byte-truncates the whole excerpt before splitting into lines. A single retained primary line that alone approaches the truncation budget got cut mid-line, taking the appended teardown banner and message down with it even though the line-count reservation had left them room.
1 parent a6f943b commit 8f446d0

3 files changed

Lines changed: 55 additions & 9 deletions

File tree

‎tests/test.ts‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -515,17 +515,26 @@ function reportFailure(
515515
);
516516

517517
// 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.
518+
// lines of whatever excerpt it's given, after its own defensive
519+
// truncateUtf8Bytes(excerpt, kExcerptMaxBytes) - a much smaller line
520+
// window than kExcerptLines, and a byte cap the primary message above
521+
// was never bounded against for THIS excerpt. A primary message with as
522+
// few as kAnnotationExcerptLines non-empty lines, or with a single line
523+
// that alone approaches kExcerptMaxBytes, already fills that window,
524+
// silently dropping the teardown banner and message from the annotation
525+
// even though both still fit in the fuller step-summary excerpt. Build
526+
// the annotation body from a dedicated, reserved excerpt instead of
527+
// reusing `excerpt` when a teardown/cleanup failure rides along -
528+
// capping the primary portion's bytes BEFORE appending the banner and
529+
// message, so annotationBody's own byte truncation lands after them
530+
// instead of cutting into the primary and consuming them too; otherwise
531+
// reuse `excerpt` unchanged.
526532
const annotationExcerpt = finallyFailure
527533
? [
528-
...stripAnsi(primary?.message ?? "")
534+
...truncateUtf8Bytes(
535+
stripAnsi(primary?.message ?? ""),
536+
kExcerptMaxBytes / 2,
537+
)
529538
.split("\n")
530539
.filter((line) => line.trim().length > 0)
531540
.slice(0, kAnnotationExcerptLines - kFinallyAnnotationReservedLines),

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,29 @@ unitTest("harness-reporting - a primary message that trips both the line and byt
363363
);
364364
});
365365

366+
unitTest("harness-reporting - a huge single-line primary message still leaves room for the teardown banner and message in the annotation", async () => {
367+
const run = await gha();
368+
const annotations = annotationsFor(
369+
run.stdout,
370+
"huge single-line message and teardown also throws",
371+
);
372+
assertEquals(annotations.length, 1);
373+
assert(
374+
!annotations[0].includes("y".repeat(300 * 1024)),
375+
"the huge primary line must still be bounded, not embedded whole, in the annotation",
376+
);
377+
assert(
378+
annotations[0].includes("TEARDOWN ALSO FAILED:"),
379+
`the annotation's byte cap must not cut into the joined excerpt before the banner; annotation was:\n${
380+
annotations[0]
381+
}`,
382+
);
383+
assert(
384+
annotations[0].includes("FIXTURE_HUGELINE_TEARDOWN_BOOM"),
385+
"the annotation must also name the teardown failure, not just the summary",
386+
);
387+
});
388+
366389
unitTest("harness-reporting - a passing test produces no row and no annotation", async () => {
367390
const run = await gha();
368391
assertEquals(summaryRowsFor(run.summary, "fixture passes").length, 0);

‎tests/unit/harness-reporting-fixture.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,3 +102,17 @@ unitTest("fixture throws a primary message that trips both the line and byte cap
102102
throw new Error("FIXTURE_BOTHCAPS_TEARDOWN_BOOM");
103103
},
104104
});
105+
106+
// A single retained primary line that alone exceeds the annotation's own
107+
// byte cap, paired with a throwing teardown: the annotation excerpt's byte
108+
// truncation (applied AFTER the primary/banner/message are joined) must not
109+
// cut into the joined string before the banner and message, or they're
110+
// dropped from the annotation even though the step summary still shows them.
111+
// deno-lint-ignore require-await
112+
unitTest("fixture throws a huge single-line message and teardown also throws", async () => {
113+
throw new Error("FIXTURE_HUGELINE_PRIMARY_" + "y".repeat(300 * 1024));
114+
}, {
115+
teardown: () => {
116+
throw new Error("FIXTURE_HUGELINE_TEARDOWN_BOOM");
117+
},
118+
});

0 commit comments

Comments
 (0)