Skip to content

Commit

Permalink
Do not reuse Command() instances in tests, they cache options.
Browse files Browse the repository at this point in the history
  • Loading branch information
@goodov
goodov committed Oct 9, 2024
1 parent 1e96428 commit 2040f36
Show file tree
Hide file tree
Showing 5 changed files with 177 additions and 52 deletions.
12 changes: 7 additions & 5 deletions src/seed_tools/commands/compare_seeds.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@ import { promises as fs } from 'fs';
import { VariationsSeed } from '../../proto/generated/variations_seed';
import diffStrings from '../utils/diff_strings';

export default new Command('compare_seeds')
.description('Compare two seed.bin')
.argument('<seed1_file>', 'seed1 file')
.argument('<seed2_file>', 'seed2 file')
.action(main);
export default function createCommand() {
return new Command('compare_seeds')
.description('Compare two seed.bin')
.argument('<seed1_file>', 'seed1 file')
.argument('<seed2_file>', 'seed2 file')
.action(main);
}

async function main(seed1FilePath: string, seed2FilePath: string) {
const seed1Binary: Buffer = await fs.readFile(seed1FilePath);
Expand Down
139 changes: 127 additions & 12 deletions src/seed_tools/commands/create_seed.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,10 @@ describe('create_seed command', () => {
const outputFile = path.join(tempDir, 'output.bin');
const serialNumberPath = path.join(tempDir, 'serial_number.txt');

await create_seed.parseAsync([
await create_seed().parseAsync([
'node',
'create_seed',
studiesDir,
'--output_seed_file',
outputFile,
'--mock_serial_number',
'1',
Expand All @@ -73,11 +72,10 @@ describe('create_seed command', () => {
const outputFile = path.join(tempDir, 'output.bin');
const serialNumberPath = path.join(tempDir, 'serial_number.txt');

await create_seed.parseAsync([
await create_seed().parseAsync([
'node',
'create_seed',
studiesDir,
'--output_seed_file',
outputFile,
'--version',
'test version value',
Expand All @@ -100,6 +98,21 @@ describe('create_seed command', () => {
);
});

test('no output file no validate', async () => {
const testCaseDir = path.join(testDataDir, 'set_seed_version');
const studiesDir = path.join(testCaseDir, 'studies');

await expect(
create_seed().parseAsync(['node', 'create_seed', studiesDir]),
).rejects.toThrowError('process.exit(1)');

expect(errorMock).toHaveBeenCalledWith(
expect.stringContaining(
'Either output_seed_file or --validate_only must be provided',
),
);
});

describe('invalid studies', () => {
const invalidStudiesDir = path.join(testDataDir, 'invalid_studies');
it.each(fs_sync.readdirSync(invalidStudiesDir))(
Expand All @@ -111,11 +124,10 @@ describe('create_seed command', () => {
const serialNumberPath = path.join(tempDir, 'serial_number.txt');

await expect(
create_seed.parseAsync([
create_seed().parseAsync([
'node',
'create_seed',
studiesDir,
'--output_seed_file',
outputFile,
'--mock_serial_number',
'1',
Expand All @@ -138,11 +150,10 @@ describe('create_seed command', () => {
const serialNumberPath = path.join(tempDir, 'serial_number.txt');

await expect(
create_seed.parseAsync([
create_seed().parseAsync([
'node',
'create_seed',
studiesDir,
'--output_seed_file',
outputFile,
'--mock_serial_number',
'1',
Expand Down Expand Up @@ -175,11 +186,10 @@ describe('create_seed command', () => {
const outputFile = path.join(tempDir, 'output.bin');

await expect(
create_seed.parseAsync([
create_seed().parseAsync([
'node',
'create_seed',
studiesDir,
'--output_seed_file',
outputFile,
]),
).rejects.toThrowError('process.exit(1)');
Expand All @@ -195,6 +205,112 @@ describe('create_seed command', () => {
);
});

describe('validate valid studies', () => {
const unformattedStudiesDir = path.join(testDataDir, 'valid_seeds');

it.each(fs_sync.readdirSync(unformattedStudiesDir))(
'correctly validates %s',
async (testCase) => {
const testCaseDir = path.join(unformattedStudiesDir, testCase);
const studiesDir = path.join(testCaseDir, 'studies');

await create_seed().parseAsync([
'node',
'create_seed',
studiesDir,
'--validate_only',
]);

expect(errorMock).toHaveBeenCalledTimes(0);
},
);
});

describe('validate unformatted studies', () => {
const unformattedStudiesDir = path.join(testDataDir, 'unformatted_studies');

it.each(fs_sync.readdirSync(unformattedStudiesDir))(
'correctly validates %s',
async (testCase) => {
const testCaseDir = path.join(unformattedStudiesDir, testCase);
const studiesDir = path.join(testCaseDir, 'studies');

await expect(
create_seed().parseAsync([
'node',
'create_seed',
studiesDir,
'--validate_only',
]),
).rejects.toThrowError('process.exit(1)');

const expectedOutput = await fs.readFile(
path.join(testCaseDir, 'expected_output.txt'),
'utf-8',
);
expect(errorMock).toHaveBeenCalledWith(
expect.stringContaining(expectedOutput),
);
},
);
});

describe('fix unformatted studies', () => {
const unformattedStudiesDir = path.join(testDataDir, 'unformatted_studies');

it.each(fs_sync.readdirSync(unformattedStudiesDir))(
'correctly fixes %s',
async (testCase) => {
const testCaseDir = path.join(unformattedStudiesDir, testCase);
const studiesDir = path.join(testCaseDir, 'studies');
const tempStudiesDir = path.join(tempDir, 'studies');
const outputFile = path.join(tempDir, 'output.bin');

// copy the unformatted studies to a temp dir
await fs.mkdir(tempStudiesDir);
await fs.copyFile(
path.join(studiesDir, 'TestStudy.json'),
path.join(tempStudiesDir, 'TestStudy.json'),
);

// Validate should fail.
await expect(
create_seed().parseAsync([
'node',
'create_seed',
tempStudiesDir,
'--validate_only',
]),
).rejects.toThrowError('process.exit(1)');

// Fix studies.
await create_seed().parseAsync([
'node',
'create_seed',
tempStudiesDir,
'--validate_only',
'--fix',
]);

// Validate should not fail.
await create_seed().parseAsync([
'node',
'create_seed',
tempStudiesDir,
'--validate_only',
]);

// // Seed creation should not fail.
await create_seed().parseAsync([
'node',
'create_seed',
tempStudiesDir,
outputFile,
]);
},
);
});

describe('studies should be valid in invalid_seed test dir', () => {
const invalidSeedsDir = path.join(testDataDir, 'invalid_seeds');

Expand All @@ -205,11 +321,10 @@ describe('create_seed command', () => {
const outputFile = path.join(tempDir, 'output.bin');

await expect(
create_seed.parseAsync([
create_seed().parseAsync([
'node',
'create_seed',
path.join(testCaseDir, 'studies'),
'--output_seed_file',
outputFile,
]),
).rejects.toThrowError('process.exit(1)');
Expand Down
60 changes: 33 additions & 27 deletions src/seed_tools/commands/create_seed.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,41 +18,51 @@ import * as file_utils from '../utils/file_utils';
import * as seed_validation from '../utils/seed_validation';
import * as study_json_utils from '../utils/study_json_utils';

export default new Command('create_seed')
.description('Create seed.bin from study files')
.argument('<studies_dir>', 'path to the directory containing study files')
.option('--fix', 'fix format errors in-place')
.option('--mock_serial_number <value>', 'mock serial number')
.option('--output_seed_file <path>', 'file path to write the seed')
.option(
'--output_serial_number_file <path>',
'file path to write the seed serial number',
'./serialnumber',
)
.option('--validate_only', 'validate the seed without creating it')
.option('--version <value>', 'seed version to set')
.action(main);
export default function createCommand() {
return new Command('create_seed')
.description('Create seed.bin from study files')
.argument('<studies_dir>', 'path to the directory containing study files')
.argument(
'[output_seed_file]',
'output seed file, may be omitted if --validate_only is provided',
)
.option('--fix', 'fix format errors in-place')
.option('--mock_serial_number <value>', 'mock serial number')
.option(
'--output_serial_number_file <path>',
'file path to write the seed serial number',
'./serialnumber',
)
.option('--validate_only', 'validate the seed without creating it')
.option('--version <value>', 'seed version to set')
.action(main);
}

interface Options {
fix?: true;
mock_serial_number?: string;
output_seed_file?: string;
output_serial_number_file?: string;
validate_only?: true;
version?: string;
}

async function main(studiesDir: string, options: Options) {
if (options.output_seed_file === undefined && !options.validate_only) {
async function main(
studiesDir: string,
outputSeedFile: string | undefined,
options: Options,
) {
console.log(outputSeedFile, options);
if (outputSeedFile === undefined && !options.validate_only) {
console.error(
'Either --output_seed_file or --validate_only option must be provided',
'Either output_seed_file or --validate_only must be provided',
);
process.exit(1);
}

if (options.fix && !options.validate_only) {
if (options.fix && outputSeedFile) {
console.log(`outputSeedFile: "${outputSeedFile}"`);
console.error(
'The --fix option can only be used with --validate_only option',
'The --fix option cannot be used when an output_seed_file is provided',
);
process.exit(1);
}
Expand Down Expand Up @@ -80,17 +90,13 @@ async function main(studiesDir: string, options: Options) {
}

console.log('Seed study count:', variationsSeed.study.length);
if (options.output_seed_file !== undefined) {
if (outputSeedFile !== undefined) {
const seedBinary = VariationsSeed.toBinary(variationsSeed);
await fs.writeFile(options.output_seed_file, seedBinary);
await fs.writeFile(outputSeedFile, seedBinary);
if (options.output_serial_number_file !== undefined) {
await fs.writeFile(options.output_serial_number_file, serialNumber);
}
console.log(
options.output_seed_file,
'created with serial number',
serialNumber,
);
console.log(outputSeedFile, 'created with serial number', serialNumber);
}
}

Expand Down
12 changes: 7 additions & 5 deletions src/seed_tools/commands/split_seed_json.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@ import { type Study } from '../../proto/generated/study';
import { VariationsSeed } from '../../proto/generated/variations_seed';
import * as study_json_utils from '../utils/study_json_utils';

export default new Command('split_seed_json')
.description('Split seed.json into study files')
.argument('<seed_json_path>', 'path to seed.json')
.argument('<output_dir>', 'output directory')
.action(main);
export default function createCommand() {
return new Command('split_seed_json')
.description('Split seed.json into study files')
.argument('<seed_json_path>', 'path to seed.json')
.argument('<output_dir>', 'output directory')
.action(main);
}

async function main(seedPath: string, outputDir: string) {
const seedJson = preprocessSeedJson(
Expand Down
6 changes: 3 additions & 3 deletions src/seed_tools/seed_tools.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import split_seed_json from './commands/split_seed_json';
program
.name('seed_tools')
.description('Seed tools for manipulating study files.')
.addCommand(compare_seeds)
.addCommand(create_seed)
.addCommand(split_seed_json)
.addCommand(compare_seeds())
.addCommand(create_seed())
.addCommand(split_seed_json())
.parse();

0 comments on commit 2040f36

Please sign in to comment.