github-advanced-security[bot] commented on code in PR #414:
URL:
https://github.com/apache/fineract-backoffice-ui/pull/414#discussion_r3822008608
##########
scripts/e2e-summary.mjs:
##########
@@ -21,81 +21,294 @@
* Renders the Playwright JSON report as Markdown for the run summary and the
PR
* comment. Prints to stdout; the workflow redirects it.
*
- * Never exits non-zero: this is reporting, and it runs with `if: always()`,
so a
- * missing or malformed report must not turn a passing run red or mask a real
- * failure with a crash here.
+ * ## What it reports, and why
+ *
+ * The first version printed totals and, on failure, the failures. That
answers "did it pass"
+ * and nothing else — a green run said `188 passed` and left no record of
*what* passed, so
+ * a spec that quietly stopped running (renamed, `test.skip`ped, dropped from
a project's
+ * `testMatch`) looked exactly like a spec that ran and passed. Sharding makes
that worse:
+ * a test can now vanish because a shard died before reaching it, and the
total is the only
+ * place that would show it.
+ *
+ * So this reports three things beyond the verdict:
+ *
+ * - a per-spec-file table, always visible, so a file that produced no tests
is obvious;
+ * - the full per-test list, folded into a <details> block so it does not
bury the verdict;
+ * - the slowest tests, which is what the shard counts should be tuned
against.
+ *
+ * ## Size
+ *
+ * A GitHub PR comment caps at 65536 characters and the API rejects anything
longer, so a
+ * suite large enough to overflow would silently lose its comment entirely —
the workflow
+ * step is `continue-on-error`. `E2E_SUMMARY_MAX_BYTES` bounds the output:
sections are
+ * dropped from the least important upward until it fits, and **what was
dropped is always
+ * stated**. A summary that silently omits half the suite is worse than one
that admits it.
+ *
+ * Never exits non-zero: this is reporting, and it runs with `if: always()`,
so a missing or
+ * malformed report must not turn a passing run red or mask a real failure
with a crash here.
*/
import { readFileSync } from 'node:fs';
const REPORT = process.env.PLAYWRIGHT_JSON_OUTPUT_NAME ??
'playwright-results.json';
-function collect(suite, out) {
+/** Output budget. The default is generous; the PR-comment invocation passes a
smaller one. */
+const MAX_BYTES = Number(process.env.E2E_SUMMARY_MAX_BYTES ?? 60000);
+
+/** How many tests the folded full listing will name before it truncates. */
+const MAX_LISTED = Number(process.env.E2E_SUMMARY_MAX_LISTED ?? 400);
+
+const ICON = {
+ passed: '✅',
+ failed: '❌',
+ timedOut: '⏱️',
+ skipped: '⏭️',
+ interrupted: '⚠️',
+};
+
+/**
+ * Strips ANSI escape sequences.
+ *
+ * Playwright embeds terminal colour codes in `error.message` — an assertion
failure arrives as
+ * `Error: \x1b[2mexpect(\x1b[22m...`. They are invisible in a terminal and
unreadable
+ * everywhere else, and GitHub renders them literally.
+ */
+// eslint-disable-next-line no-control-regex -- escape sequences are exactly
the target
+const ANSI = /\u001B\[[0-9;]*m/g;
+
+/**
+ * Flattens the suite tree, accumulating `describe` titles.
+ *
+ * The root suite's title is the spec's file path, which is already the
heading every test is
+ * rendered under, so including it would print `client.spec.ts ›
client.spec.ts › creates a
+ * client`. Ancestry below that is kept: a nested `describe` is context worth
having.
+ */
+function collect(suite, out, titlePath = []) {
+ const file = suite.file ?? '';
+ const isFileSuite = !suite.title || suite.title === file || suite.title ===
suite.location?.file;
+ const path = isFileSuite ? titlePath : [...titlePath, suite.title];
+
for (const spec of suite.specs ?? []) {
for (const test of spec.tests ?? []) {
const result = test.results?.[test.results.length - 1];
out.push({
- title: [...(suite.title ? [suite.title] : []), spec.title].join(' › '),
+ title: [...path, spec.title].join(' › '),
file: spec.file ?? suite.file ?? '',
+ project: test.projectName ?? '',
status: test.status === 'skipped' ? 'skipped' : (result?.status ??
test.status),
expected: test.expectedStatus,
+ // A test that failed and then passed on retry is green overall but
worth surfacing:
+ // it is the shape a flake takes, and flakes are what a sharded suite
hides best.
+ retries: Math.max(0, (test.results?.length ?? 1) - 1),
durationMs: result?.duration ?? 0,
- error: result?.error?.message ?? '',
+ error: (result?.error?.message ?? '').replace(ANSI, ''),
});
}
}
- for (const child of suite.suites ?? []) collect(child, out);
+ for (const child of suite.suites ?? []) collect(child, out, path);
+}
+
+/** `1.2s`, or `340ms` below a second — a duration nobody has to convert in
their head. */
+function humanDuration(ms) {
+ if (ms < 1000) return `${Math.round(ms)}ms`;
+ if (ms < 60000) return `${(ms / 1000).toFixed(1)}s`;
+ const minutes = Math.floor(ms / 60000);
+ return `${minutes}m ${Math.round((ms % 60000) / 1000)}s`;
+}
+
+/** Pipes would break out of the markdown table cell they are rendered into. */
+function cell(text) {
+ return String(text).replace(/\|/g, '\\|');
Review Comment:
## CodeQL / Incomplete string escaping or encoding
This does not escape backslash characters in the input.
[Show more
details](https://github.com/apache/fineract-backoffice-ui/security/code-scanning/16)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
