Skip to content

Commit c442612

Browse files
committed
feat(nevermore-cli): Report per-package pcall duration in aggregated batch test mode
Previously every row in `nevermore batch test` showed the same duration because all packages awaited the same shared batch execution promise — the outer wall-clock was identical for every package. The Luau batch runner now measures pcall time per package, emits it in the END marker and JSON summary, and the result flows through ScriptRunResult → SingleTestResult → BatchTestResult. batch-runner.ts uses the inner duration when provided and falls back to wall-clock otherwise (preserves non-aggregated behavior).
1 parent 2b02240 commit c442612

7 files changed

Lines changed: 79 additions & 15 deletions

File tree

tools/nevermore-cli/src/commands/batch-command/batch-test-command.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,10 @@ async function _runAsync(args: BatchTestArgs): Promise<void> {
238238
placeId: pkg.target.placeId,
239239
success: result.success,
240240
logs: result.logs,
241+
// Aggregated mode reports per-package pcall time; the outer
242+
// wall-clock would otherwise be identical for every package
243+
// (they all await the same shared batch execution).
244+
durationMs: result.durationMs,
241245
progressSummary: result.testCounts
242246
? { kind: 'test-counts' as const, ...result.testCounts }
243247
: undefined,

tools/nevermore-cli/src/utils/batch/batch-runner.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,19 @@ import {
77
} from '@quenty/cli-output-helpers/reporting';
88
import { type TargetPackage } from './changed-packages-utils.js';
99

10+
/** Partial result returned by executeAsync — durationMs is optional. */
11+
export type PartialBatchResult<TResult extends PackageResult> = Omit<
12+
TResult,
13+
'durationMs'
14+
> & {
15+
/**
16+
* Inner-measured duration. When provided, overrides the outer wall-clock
17+
* timing (useful in aggregated batch mode where every package's outer
18+
* await resolves at the same instant).
19+
*/
20+
durationMs?: number;
21+
};
22+
1023
export interface BatchOptions<TResult extends PackageResult> {
1124
packages: TargetPackage[];
1225
concurrency?: number;
@@ -16,7 +29,7 @@ export interface BatchOptions<TResult extends PackageResult> {
1629
executeAsync: (
1730
pkg: TargetPackage,
1831
reporter: Reporter
19-
) => Promise<Omit<TResult, 'durationMs'>>;
32+
) => Promise<PartialBatchResult<TResult>>;
2033
}
2134

2235
export async function runBatchAsync<TResult extends PackageResult>(
@@ -85,7 +98,7 @@ async function _runOneAsync<TResult extends PackageResult>(
8598
executeAsync: (
8699
pkg: TargetPackage,
87100
reporter: Reporter
88-
) => Promise<Omit<TResult, 'durationMs'>>,
101+
) => Promise<PartialBatchResult<TResult>>,
89102
reporter: Reporter,
90103
bufferOutput: boolean,
91104
stateTracker?: IStateTracker
@@ -96,7 +109,8 @@ async function _runOneAsync<TResult extends PackageResult>(
96109
const execute = async (): Promise<TResult> => {
97110
try {
98111
const partial = await executeAsync(pkg, reporter);
99-
return { ...partial, durationMs: Date.now() - startMs } as TResult;
112+
const durationMs = partial.durationMs ?? Date.now() - startMs;
113+
return { ...partial, durationMs } as TResult;
100114
} catch (err) {
101115
const errorMessage = err instanceof Error ? err.message : String(err);
102116
const currentPhase = stateTracker?.getCurrentPhase(pkg.name);

tools/nevermore-cli/src/utils/job-context/batch-script-job-context.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ export class BatchScriptJobContext implements JobContext {
132132
return { success: false };
133133
}
134134

135-
return { success: result.success };
135+
return { success: result.success, durationMs: result.durationMs };
136136
}
137137

138138
async getLogsAsync(deployment: Deployment): Promise<string> {

tools/nevermore-cli/src/utils/job-context/job-context.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,13 @@ export interface RunScriptOptions {
1717
export interface ScriptRunResult {
1818
/** Whether the execution infrastructure succeeded (not test assertions). */
1919
success: boolean;
20+
/**
21+
* Optional inner script execution time, reported when the context can
22+
* measure it directly (e.g. aggregated batch mode reports per-package
23+
* pcall durations). When undefined, callers should fall back to their own
24+
* wall-clock measurement.
25+
*/
26+
durationMs?: number;
2027
}
2128

2229
/**

tools/nevermore-cli/src/utils/testing/parsers/batch-log-parser.ts

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ export interface BatchPackageResult {
55
slug: string;
66
success: boolean;
77
logs: string;
8+
/** Inner pcall execution time reported by the Luau batch runner. */
9+
durationMs?: number;
810
testCounts?: ParsedTestCounts;
911
}
1012

@@ -15,6 +17,7 @@ const SUMMARY_MARKER = '===BATCH_TEST_SUMMARY===';
1517
interface SummaryEntry {
1618
slug: string;
1719
success: boolean;
20+
durationMs?: number;
1821
error?: string;
1922
}
2023

@@ -24,9 +27,9 @@ interface SummaryEntry {
2427
* The batch Luau template prints structured markers around each package's output:
2528
* ===BATCH_TEST_BEGIN <slug>===
2629
* ... test output ...
27-
* ===BATCH_TEST_END <slug> PASS|FAIL===
30+
* ===BATCH_TEST_END <slug> PASS|FAIL <durationMs>===
2831
* ===BATCH_TEST_SUMMARY===
29-
* [{"slug":"maid","success":true}, ...]
32+
* [{"slug":"maid","success":true,"durationMs":1234}, ...]
3033
*
3134
* Success is determined from the JSON summary (based on pcall results), which is
3235
* immune to log reordering. The BEGIN/END markers are used only for splitting logs
@@ -48,10 +51,14 @@ export function parseBatchTestLogs(
4851
// ── Pass 1: Extract per-package log sections from markers ──
4952

5053
const logSections = new Map<string, string>();
54+
const markerDurations = new Map<string, number>();
5155
let currentSlug: string | null = null;
5256
let currentLines: string[] = [];
5357
let summaryLineIndex = -1;
5458

59+
// Matches "<slug> PASS|FAIL [<durationMs>]" — slugs have no whitespace.
60+
const endInnerPattern = /^(\S+)\s+(?:PASS|FAIL)(?:\s+(\d+))?$/;
61+
5562
for (let i = 0; i < lines.length; i++) {
5663
const trimmed = lines[i].trimEnd();
5764

@@ -63,11 +70,15 @@ export function parseBatchTestLogs(
6370

6471
if (trimmed.startsWith(END_MARKER) && trimmed.endsWith('===')) {
6572
const inner = trimmed.slice(END_MARKER.length, -3);
66-
const spaceIndex = inner.lastIndexOf(' ');
67-
const endSlug = spaceIndex >= 0 ? inner.slice(0, spaceIndex) : inner;
73+
const match = endInnerPattern.exec(inner);
74+
const endSlug = match ? match[1] : inner;
75+
const durationStr = match?.[2];
6876

6977
if (currentSlug && endSlug === currentSlug) {
7078
logSections.set(currentSlug, currentLines.join('\n'));
79+
if (durationStr !== undefined) {
80+
markerDurations.set(currentSlug, parseInt(durationStr, 10));
81+
}
7182
currentSlug = null;
7283
currentLines = [];
7384
}
@@ -89,6 +100,7 @@ export function parseBatchTestLogs(
89100
// ── Pass 2: Parse the JSON summary for authoritative pcall results ──
90101

91102
const summaryResults = new Map<string, boolean>();
103+
const summaryDurations = new Map<string, number>();
92104
if (summaryLineIndex >= 0 && summaryLineIndex + 1 < lines.length) {
93105
const jsonLine = lines
94106
.slice(summaryLineIndex + 1)
@@ -98,6 +110,9 @@ export function parseBatchTestLogs(
98110
const entries = JSON.parse(jsonLine) as SummaryEntry[];
99111
for (const entry of entries) {
100112
summaryResults.set(entry.slug, entry.success);
113+
if (typeof entry.durationMs === 'number') {
114+
summaryDurations.set(entry.slug, entry.durationMs);
115+
}
101116
}
102117
// Log any pcall failures from the Luau template
103118
const failures = entries.filter((e) => !e.success);
@@ -144,7 +159,17 @@ export function parseBatchTestLogs(
144159
}
145160

146161
const testCounts = sectionLogs ? parseTestCounts(sectionLogs) : undefined;
147-
results.set(packageName, { slug, success, logs: sectionLogs, testCounts });
162+
// Prefer the JSON summary (authoritative, immune to log reordering);
163+
// fall back to the END-marker value if the summary was truncated.
164+
const durationMs =
165+
summaryDurations.get(slug) ?? markerDurations.get(slug);
166+
results.set(packageName, {
167+
slug,
168+
success,
169+
logs: sectionLogs,
170+
durationMs,
171+
testCounts,
172+
});
148173
}
149174

150175
return results;

tools/nevermore-cli/src/utils/testing/runner/test-runner.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,12 @@ export interface SingleTestResult {
1212
success: boolean;
1313
logs: string;
1414
testCounts?: ParsedTestCounts;
15+
/**
16+
* Inner script execution time, forwarded from the JobContext when it can
17+
* measure pcall duration directly (aggregated batch mode). Undefined for
18+
* non-aggregated cloud/local runs — callers should fall back to wall-clock.
19+
*/
20+
durationMs?: number;
1521
}
1622

1723
export interface SingleTestOptions {
@@ -66,6 +72,7 @@ export async function runSingleTestAsync(
6672
success: result.success && parsed.success,
6773
logs: parsed.logs,
6874
testCounts: parseTestCounts(parsed.logs),
75+
durationMs: result.durationMs,
6976
};
7077
} finally {
7178
await context.releaseAsync(deployment);

tools/nevermore-cli/templates/batch-test-runner.luau

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ end
3232
type Result = {
3333
success: boolean,
3434
slug: string,
35+
durationMs: number,
3536
error: string?,
3637
}
3738

@@ -42,8 +43,8 @@ for _, slug in packageSlugs do
4243
if not scriptSource then
4344
warn(`[BatchTest] No script for {slug}`)
4445
print("===BATCH_TEST_BEGIN " .. slug .. "===")
45-
print("===BATCH_TEST_END " .. slug .. " FAIL===")
46-
table.insert(results, { slug = slug, success = false, error = "No test script found" })
46+
print("===BATCH_TEST_END " .. slug .. " FAIL 0===")
47+
table.insert(results, { slug = slug, success = false, durationMs = 0, error = "No test script found" })
4748
continue
4849
end
4950

@@ -71,6 +72,10 @@ for _, slug in packageSlugs do
7172

7273
print("===BATCH_TEST_BEGIN " .. slug .. "===")
7374

75+
-- Measure pcall execution time only. The surrounding cleanup/yield cost
76+
-- is amortized across all packages and would otherwise dominate fast tests.
77+
local startClock = os.clock()
78+
7479
local testOk, testErr = pcall(function()
7580
local fn, compileErr = loadstring(scriptSource, slug)
7681
if not fn then
@@ -79,6 +84,8 @@ for _, slug in packageSlugs do
7984
fn()
8085
end)
8186

87+
local durationMs = math.floor((os.clock() - startClock) * 1000 + 0.5)
88+
8289
-- Yield before the END marker for the same reason: leaked deferred
8390
-- callbacks may fire during fn() and their output must land before
8491
-- the marker in the log stream.
@@ -112,12 +119,12 @@ for _, slug in packageSlugs do
112119
end
113120

114121
if testOk then
115-
print("===BATCH_TEST_END " .. slug .. " PASS===")
116-
table.insert(results, { slug = slug, success = true })
122+
print("===BATCH_TEST_END " .. slug .. " PASS " .. tostring(durationMs) .. "===")
123+
table.insert(results, { slug = slug, success = true, durationMs = durationMs })
117124
else
118125
warn("[BatchTest] " .. slug .. ": " .. tostring(testErr))
119-
print("===BATCH_TEST_END " .. slug .. " FAIL===")
120-
table.insert(results, { slug = slug, success = false, error = tostring(testErr) })
126+
print("===BATCH_TEST_END " .. slug .. " FAIL " .. tostring(durationMs) .. "===")
127+
table.insert(results, { slug = slug, success = false, durationMs = durationMs, error = tostring(testErr) })
121128
end
122129
end
123130

0 commit comments

Comments
 (0)