Skip to content

Commit e35cbd5

Browse files
committed
fix: address fixture generation review feedback
1 parent 763ed63 commit e35cbd5

7 files changed

Lines changed: 144 additions & 26 deletions

File tree

packages/cli/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
"scripts": {
3232
"build": "tsc",
3333
"postbuild": "node scripts/ensure-shebang.mjs",
34-
"test": "FIXTURE_ROOT=$(mktemp -d) && export MRTDOWN_FIXTURE_DATA_DIR=\"$FIXTURE_ROOT/data\" MRTDOWN_FIXTURE_META_PATH=\"$FIXTURE_ROOT/meta.json\" && node ../../scripts/generate-fixtures.mjs --data-dir \"$MRTDOWN_FIXTURE_DATA_DIR\" --meta-path \"$MRTDOWN_FIXTURE_META_PATH\" && npm --workspace @mrtdown/core run build && npm --workspace @mrtdown/fs run build && vitest --run"
34+
"test": "node ../../scripts/run-fixture-command.mjs --build @mrtdown/core --build @mrtdown/fs -- npm exec -- vitest --run"
3535
},
3636
"dependencies": {
3737
"@mrtdown/core": "2.0.0-alpha.23",

packages/core/src/schema/issue/period.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,12 @@ export const PeriodFrequencySchema = z.enum([
1515
]);
1616
export type PeriodFrequency = z.infer<typeof PeriodFrequencySchema>;
1717

18+
export const PeriodTimeZoneSchema = z.enum([
19+
'Asia/Singapore',
20+
'Asia/Hong_Kong',
21+
]);
22+
export type PeriodTimeZone = z.infer<typeof PeriodTimeZoneSchema>;
23+
1824
export const PeriodRecurringSchema = z.object({
1925
kind: z.literal('recurring'),
2026
frequency: PeriodFrequencySchema,
@@ -27,7 +33,7 @@ export const PeriodRecurringSchema = z.object({
2733
startAt: z.iso.time(),
2834
endAt: z.iso.time(),
2935
}),
30-
timeZone: z.literal('Asia/Singapore'),
36+
timeZone: PeriodTimeZoneSchema,
3137
excludedDates: z.array(z.iso.date()).nullable(),
3238
});
3339
export type PeriodRecurring = z.infer<typeof PeriodRecurringSchema>;

packages/fs/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
},
3131
"scripts": {
3232
"build": "tsc",
33-
"test": "FIXTURE_ROOT=$(mktemp -d) && export MRTDOWN_FIXTURE_DATA_DIR=\"$FIXTURE_ROOT/data\" MRTDOWN_FIXTURE_META_PATH=\"$FIXTURE_ROOT/meta.json\" && node ../../scripts/generate-fixtures.mjs --data-dir \"$MRTDOWN_FIXTURE_DATA_DIR\" --meta-path \"$MRTDOWN_FIXTURE_META_PATH\" && npm --workspace @mrtdown/core run build && vitest --run"
33+
"test": "node ../../scripts/run-fixture-command.mjs --build @mrtdown/core -- npm exec -- vitest --run"
3434
},
3535
"dependencies": {
3636
"@mrtdown/core": "2.0.0-alpha.23",

packages/triage/package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@
2424
},
2525
"scripts": {
2626
"build": "tsc",
27-
"test": "FIXTURE_ROOT=$(mktemp -d) && export MRTDOWN_FIXTURE_DATA_DIR=\"$FIXTURE_ROOT/data\" MRTDOWN_FIXTURE_META_PATH=\"$FIXTURE_ROOT/meta.json\" && node ../../scripts/generate-fixtures.mjs --data-dir \"$MRTDOWN_FIXTURE_DATA_DIR\" --meta-path \"$MRTDOWN_FIXTURE_META_PATH\" && vitest --run",
28-
"test:eval": "FIXTURE_ROOT=$(mktemp -d) && export MRTDOWN_FIXTURE_DATA_DIR=\"$FIXTURE_ROOT/data\" MRTDOWN_FIXTURE_META_PATH=\"$FIXTURE_ROOT/meta.json\" && node ../../scripts/generate-fixtures.mjs --data-dir \"$MRTDOWN_FIXTURE_DATA_DIR\" --meta-path \"$MRTDOWN_FIXTURE_META_PATH\" && vitest --run --config vitest.eval.config.ts"
27+
"test": "node ../../scripts/run-fixture-command.mjs -- npm exec -- vitest --run",
28+
"test:eval": "node ../../scripts/run-fixture-command.mjs -- npm exec -- vitest --run --config vitest.eval.config.ts"
2929
},
3030
"dependencies": {
3131
"@mrtdown/core": "2.0.0-alpha.23",

packages/triage/src/llm/functions/triageNewEvidence/eval.test.ts

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { readFileSync } from 'node:fs';
22
import { resolve } from 'node:path';
33
import { FileStore, MRTDownRepository } from '@mrtdown/fs';
44
import { config as loadDotEnv } from 'dotenv';
5+
import { DateTime } from 'luxon';
56
import { describe } from 'vitest';
67
import { describeEval, StructuredOutputScorer } from 'vitest-evals';
78
import {
@@ -33,6 +34,21 @@ const FIXTURE_META = JSON.parse(
3334
};
3435
};
3536

37+
function addSecondsToIsoTimestamp(timestamp: string, seconds: number) {
38+
const isoTimestamp = DateTime.fromISO(timestamp, { setZone: true })
39+
.plus({ seconds })
40+
.toISO({ suppressMilliseconds: true });
41+
if (isoTimestamp == null) {
42+
throw new Error(`Could not format timestamp: ${timestamp}`);
43+
}
44+
return isoTimestamp;
45+
}
46+
47+
const TRAIN_FAULT_FOLLOW_UP_TS = addSecondsToIsoTimestamp(
48+
FIXTURE_META.issues.trainFault.timestamp,
49+
10,
50+
);
51+
3652
describe('triageNewEvidence', () => {
3753
describeEval(
3854
'should triage the new evidence into an existing issue or a new issue',
@@ -46,10 +62,7 @@ describe('triageNewEvidence', () => {
4662
{
4763
input: {
4864
newEvidence: {
49-
ts: FIXTURE_META.issues.trainFault.timestamp.replace(
50-
':00+08:00',
51-
':10+08:00',
52-
),
65+
ts: TRAIN_FAULT_FOLLOW_UP_TS,
5366
text: '[ISL] Due to a track fault at HKU, train services on the Island Line are delayed between Kennedy Town and Admiralty',
5467
},
5568
repo,
@@ -68,10 +81,7 @@ describe('triageNewEvidence', () => {
6881
{
6982
input: {
7083
newEvidence: {
71-
ts: FIXTURE_META.issues.trainFault.timestamp.replace(
72-
':00+08:00',
73-
':10+08:00',
74-
),
84+
ts: TRAIN_FAULT_FOLLOW_UP_TS,
7585
text: '[ISL] Due to a track fault at HKU, train services on the Island Line are delayed between Admiralty and Causeway Bay',
7686
},
7787
repo,
@@ -90,10 +100,7 @@ describe('triageNewEvidence', () => {
90100
{
91101
input: {
92102
newEvidence: {
93-
ts: FIXTURE_META.issues.trainFault.timestamp.replace(
94-
':00+08:00',
95-
':10+08:00',
96-
),
103+
ts: TRAIN_FAULT_FOLLOW_UP_TS,
97104
text: '[ISL] Due to maintenance works, services on the Island Line will end earlier at 11pm tonight.',
98105
},
99106
repo,

scripts/generate-fixtures.mjs

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ const defaultGeneratedRoot = resolve(repoRoot, 'fixtures/generated');
88
const defaultDataDir = resolve(defaultGeneratedRoot, 'data');
99
const defaultMetaPath = resolve(defaultGeneratedRoot, 'meta.json');
1010
const localeKeys = ['en-SG', 'zh-Hans', 'ms', 'ta'];
11-
const hkTimeZone = 'Asia/Singapore';
11+
const hkTimeZone = 'Asia/Hong_Kong';
1212

1313
function translations(name) {
1414
return Object.fromEntries(
@@ -63,13 +63,19 @@ function isPathWithin(parent, child) {
6363
);
6464
}
6565

66-
function assertSafeOutputPath(path, label) {
67-
if (
68-
isPathWithin(repoRoot, path) &&
69-
!isPathWithin(defaultGeneratedRoot, path)
70-
) {
66+
function assertGeneratedOutputPath(path, label) {
67+
if (!isPathWithin(defaultGeneratedRoot, path)) {
7168
throw new Error(
72-
`${label} must be under fixtures/generated or outside the repository: ${path}`,
69+
`${label} must be under fixtures/generated so fixture cleanup stays sandboxed: ${path}`,
70+
);
71+
}
72+
}
73+
74+
function assertSafeDataDir(dataDir) {
75+
assertGeneratedOutputPath(dataDir, 'dataDir');
76+
if (dataDir === defaultGeneratedRoot) {
77+
throw new Error(
78+
`dataDir must be a child of fixtures/generated, not the generated root itself: ${dataDir}`,
7379
);
7480
}
7581
}
@@ -967,8 +973,8 @@ function buildIssues(anchorDate) {
967973
export async function generateFixtures(options = {}) {
968974
const dataDir = options.dataDir ?? defaultDataDir;
969975
const metaPath = options.metaPath ?? defaultMetaPath;
970-
assertSafeOutputPath(dataDir, 'dataDir');
971-
assertSafeOutputPath(metaPath, 'metaPath');
976+
assertSafeDataDir(dataDir);
977+
assertGeneratedOutputPath(metaPath, 'metaPath');
972978

973979
const anchorDate = getFixtureDate(
974980
options.now ?? process.env.MRTDOWN_FIXTURE_NOW,
@@ -1015,6 +1021,8 @@ function usage() {
10151021
Defaults:
10161022
--data-dir fixtures/generated/data
10171023
--meta-path fixtures/generated/meta.json
1024+
1025+
Custom output paths must stay under fixtures/generated.
10181026
`;
10191027
}
10201028

scripts/run-fixture-command.mjs

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
import { spawn } from 'node:child_process';
2+
import { mkdir, mkdtemp, rm } from 'node:fs/promises';
3+
import { dirname, resolve } from 'node:path';
4+
import { fileURLToPath } from 'node:url';
5+
import { generateFixtures } from './generate-fixtures.mjs';
6+
7+
const repoRoot = resolve(dirname(fileURLToPath(import.meta.url)), '..');
8+
const generatedTempRoot = resolve(repoRoot, 'fixtures/generated/tmp');
9+
const npmCommand = process.platform === 'win32' ? 'npm.cmd' : 'npm';
10+
11+
function usage() {
12+
return `Usage:
13+
node scripts/run-fixture-command.mjs [--build <workspace>]... -- <command> [args...]
14+
`;
15+
}
16+
17+
function parseArgs(argv) {
18+
const builds = [];
19+
const args = [...argv];
20+
21+
while (args.length > 0) {
22+
const arg = args.shift();
23+
if (arg === '--help' || arg === '-h') {
24+
return { builds, command: [], help: true };
25+
}
26+
27+
if (arg === '--') {
28+
return { builds, command: args };
29+
}
30+
31+
if (arg !== '--build') {
32+
throw new Error(`Unknown option: ${arg}`);
33+
}
34+
35+
const workspace = args.shift();
36+
if (!workspace || workspace.startsWith('--')) {
37+
throw new Error('--build requires a workspace name');
38+
}
39+
builds.push(workspace);
40+
}
41+
42+
throw new Error('Missing command after --');
43+
}
44+
45+
async function run(command, args, options = {}) {
46+
const child = spawn(command, args, {
47+
cwd: options.cwd ?? process.cwd(),
48+
env: options.env ?? process.env,
49+
stdio: 'inherit',
50+
});
51+
52+
const code = await new Promise((resolveCode, reject) => {
53+
child.on('error', reject);
54+
child.on('close', resolveCode);
55+
});
56+
57+
if (code !== 0) {
58+
throw new Error(`${command} ${args.join(' ')} exited with code ${code}`);
59+
}
60+
}
61+
62+
const { builds, command, help } = parseArgs(process.argv.slice(2));
63+
if (help) {
64+
console.log(usage().trimEnd());
65+
process.exit(0);
66+
}
67+
68+
if (command.length === 0) {
69+
throw new Error('Missing command after --');
70+
}
71+
72+
await mkdir(generatedTempRoot, { recursive: true });
73+
const tempRoot = await mkdtemp(resolve(generatedTempRoot, 'test-'));
74+
const env = {
75+
...process.env,
76+
MRTDOWN_FIXTURE_DATA_DIR: resolve(tempRoot, 'data'),
77+
MRTDOWN_FIXTURE_META_PATH: resolve(tempRoot, 'meta.json'),
78+
};
79+
80+
try {
81+
await generateFixtures({
82+
dataDir: env.MRTDOWN_FIXTURE_DATA_DIR,
83+
metaPath: env.MRTDOWN_FIXTURE_META_PATH,
84+
});
85+
86+
for (const workspace of builds) {
87+
await run(npmCommand, ['--workspace', workspace, 'run', 'build'], {
88+
cwd: repoRoot,
89+
env,
90+
});
91+
}
92+
93+
const executable = command[0] === 'npm' ? npmCommand : command[0];
94+
await run(executable, command.slice(1), { env });
95+
} finally {
96+
await rm(tempRoot, { recursive: true, force: true });
97+
}

0 commit comments

Comments
 (0)