From 1741d81f393363368b323d01974f3937f1057911 Mon Sep 17 00:00:00 2001 From: Cedric van Putten Date: Sun, 17 Mar 2024 14:53:38 +0100 Subject: [PATCH 1/5] refactor: rename `ndjson` to `jsonl` in utils --- .gitignore | 3 + src/data/StatsFileSource.ts | 2 +- .../{ndjson.json => specification.jsonl} | 0 src/utils/__tests__/jsonl.test.ts | 91 +++++++++++++++++++ src/utils/__tests__/ndjson.test.ts | 51 ----------- src/utils/{ndjson.ts => jsonl.ts} | 0 src/utils/stats.ts | 2 +- 7 files changed, 96 insertions(+), 53 deletions(-) rename src/utils/__tests__/fixtures/{ndjson.json => specification.jsonl} (100%) create mode 100644 src/utils/__tests__/jsonl.test.ts delete mode 100644 src/utils/__tests__/ndjson.test.ts rename src/utils/{ndjson.ts => jsonl.ts} (100%) diff --git a/.gitignore b/.gitignore index 43c633a..dd05106 100644 --- a/.gitignore +++ b/.gitignore @@ -4,6 +4,9 @@ /build /webui/dist +# test fixtures +*.temp.jsonl + # dependencies node_modules/ npm-debug.log* diff --git a/src/data/StatsFileSource.ts b/src/data/StatsFileSource.ts index 39bf802..a003a48 100644 --- a/src/data/StatsFileSource.ts +++ b/src/data/StatsFileSource.ts @@ -1,7 +1,7 @@ import assert from 'assert'; import type { PartialStatsEntry, StatsEntry, StatsSource } from './types'; -import { appendJsonLine, forEachJsonLines, parseJsonLine } from '../utils/ndjson'; +import { appendJsonLine, forEachJsonLines, parseJsonLine } from '../utils/jsonl'; export class StatsFileSource implements StatsSource { constructor(public readonly statsPath: string) { diff --git a/src/utils/__tests__/fixtures/ndjson.json b/src/utils/__tests__/fixtures/specification.jsonl similarity index 100% rename from src/utils/__tests__/fixtures/ndjson.json rename to src/utils/__tests__/fixtures/specification.jsonl diff --git a/src/utils/__tests__/jsonl.test.ts b/src/utils/__tests__/jsonl.test.ts new file mode 100644 index 0000000..448909d --- /dev/null +++ b/src/utils/__tests__/jsonl.test.ts @@ -0,0 +1,91 @@ +import { describe, expect, it, mock } from 'bun:test'; +import fs from 'fs'; +import path from 'path'; + +import { appendJsonLine, forEachJsonLines, parseJsonLine } from '../jsonl'; + +describe('forEachJsonLines', () => { + it('iterates each line of file', async () => { + const lines: string[] = []; + await forEachJsonLines(fixture('specification'), (content) => { + lines.push(content); + }); + + expect(lines).toEqual([ + expect.stringContaining('Gilbert'), + expect.stringContaining('Alexa'), + expect.stringContaining('May'), + expect.stringContaining('Deloise'), + ]); + }); + + it('iterates each line with line numbers starting from 1', async () => { + const onReadLine = mock(); + await forEachJsonLines(fixture('specification'), onReadLine); + + // Callback is invoked with (content, line, reader) => ... + expect(onReadLine).not.toHaveBeenCalledWith(expect.any(String), 0, expect.any(Object)); + expect(onReadLine).toHaveBeenCalledWith(expect.any(String), 1, expect.any(Object)); + expect(onReadLine).toHaveBeenCalledWith(expect.any(String), 2, expect.any(Object)); + expect(onReadLine).toHaveBeenCalledWith(expect.any(String), 3, expect.any(Object)); + expect(onReadLine).toHaveBeenCalledWith(expect.any(String), 4, expect.any(Object)); + }); +}); + +describe('parseJsonLine', () => { + it('parses a single line from file', async () => { + expect(await parseJsonLine(fixture('specification'), 1)).toMatchObject({ name: 'Gilbert' }); + expect(await parseJsonLine(fixture('specification'), 2)).toMatchObject({ name: 'Alexa' }); + expect(await parseJsonLine(fixture('specification'), 3)).toMatchObject({ name: 'May' }); + expect(await parseJsonLine(fixture('specification'), 4)).toMatchObject({ name: 'Deloise' }); + }); + + it('throws if single line is not found', async () => { + await expect(parseJsonLine(fixture('specification'), 99999)).rejects.toThrow( + 'Line 99999 not found in file' + ); + }); +}); + +describe('appendJsonLine', () => { + it('appends a single line to file', async () => { + const file = fixture('append-single', { temporary: true }); + await appendJsonLine(file, { name: 'Gilbert' }); + await expect(fs.promises.readFile(file, 'utf-8')).resolves.toBe('{"name":"Gilbert"}\n'); + }); + + it('appends multiple lines to file', async () => { + const file = fixture('append-multiple', { temporary: true }); + const data = [ + { name: 'Gilbert', list: ['some-list'] }, + { name: 'Alexa', nested: { nested: true, list: ['other', 'items'] } }, + { name: 'May', names: 1 }, + { name: 'Deloise', simple: true }, + ]; + + for (const item of data) { + await appendJsonLine(file, item); + } + + await expect(fs.promises.readFile(file, 'utf-8')).resolves.toBe( + data.map((item) => JSON.stringify(item) + '\n').join('') + ); + }); +}); + +/** + * Get the file path to a fixture, by name. + * This automatically adds the required `.jsonl` or `.temp.jsonl` extension. + * Use `temporary: true` to keep it out of the repository, and reset the content automatically. + */ +function fixture(name: string, { temporary = false }: { temporary?: boolean } = {}) { + const file = temporary + ? path.join(__dirname, 'fixtures', `${name}.temp.jsonl`) + : path.join(__dirname, 'fixtures', `${name}.jsonl`); + + if (temporary) { + fs.writeFileSync(file, ''); + } + + return file; +} diff --git a/src/utils/__tests__/ndjson.test.ts b/src/utils/__tests__/ndjson.test.ts deleted file mode 100644 index a4251db..0000000 --- a/src/utils/__tests__/ndjson.test.ts +++ /dev/null @@ -1,51 +0,0 @@ -import { describe, expect, it, mock } from 'bun:test'; -import path from 'path'; - -import { forEachJsonLines, parseJsonLine } from '../ndjson'; - -function fixture(...filePath: string[]) { - return path.join(__dirname, 'fixtures', ...filePath); -} - -describe('forEachJsonLines', () => { - it('iterates each line of file', async () => { - const lines: string[] = []; - await forEachJsonLines(fixture('ndjson.json'), (content) => { - lines.push(content); - }); - - expect(lines).toEqual([ - expect.stringContaining('Gilbert'), - expect.stringContaining('Alexa'), - expect.stringContaining('May'), - expect.stringContaining('Deloise'), - ]); - }); - - it('iterates each line with line numbers starting from 1', async () => { - const onReadLine = mock(); - await forEachJsonLines(fixture('ndjson.json'), onReadLine); - - // Callback is invoked with (content, line, reader) => ... - expect(onReadLine).not.toHaveBeenCalledWith(expect.any(String), 0, expect.any(Object)); - expect(onReadLine).toHaveBeenCalledWith(expect.any(String), 1, expect.any(Object)); - expect(onReadLine).toHaveBeenCalledWith(expect.any(String), 2, expect.any(Object)); - expect(onReadLine).toHaveBeenCalledWith(expect.any(String), 3, expect.any(Object)); - expect(onReadLine).toHaveBeenCalledWith(expect.any(String), 4, expect.any(Object)); - }); -}); - -describe('parseJsonLine', () => { - it('parses a single line from file', async () => { - expect(await parseJsonLine(fixture('ndjson.json'), 1)).toMatchObject({ name: 'Gilbert' }); - expect(await parseJsonLine(fixture('ndjson.json'), 2)).toMatchObject({ name: 'Alexa' }); - expect(await parseJsonLine(fixture('ndjson.json'), 3)).toMatchObject({ name: 'May' }); - expect(await parseJsonLine(fixture('ndjson.json'), 4)).toMatchObject({ name: 'Deloise' }); - }); - - it('throws if single line is not found', async () => { - await expect(parseJsonLine(fixture('ndjson.json'), 99999)).rejects.toThrow( - 'Line 99999 not found in file' - ); - }); -}); diff --git a/src/utils/ndjson.ts b/src/utils/jsonl.ts similarity index 100% rename from src/utils/ndjson.ts rename to src/utils/jsonl.ts diff --git a/src/utils/stats.ts b/src/utils/stats.ts index 8a32f20..f5dddd3 100644 --- a/src/utils/stats.ts +++ b/src/utils/stats.ts @@ -4,7 +4,7 @@ import path from 'path'; import { name, version } from '../../package.json'; import { env } from '../utils/env'; import { AtlasValidationError } from '../utils/errors'; -import { parseJsonLine } from '../utils/ndjson'; +import { parseJsonLine } from './jsonl'; export type StatsMetadata = { name: string; version: string }; From f1548d6dfb22e259d28fa415079b649b49e623d1 Mon Sep 17 00:00:00 2001 From: Cedric van Putten Date: Sun, 17 Mar 2024 15:01:22 +0100 Subject: [PATCH 2/5] chore: fix linting issues --- src/utils/stats.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/stats.ts b/src/utils/stats.ts index f5dddd3..be3e247 100644 --- a/src/utils/stats.ts +++ b/src/utils/stats.ts @@ -1,10 +1,10 @@ import fs from 'fs'; import path from 'path'; +import { parseJsonLine } from './jsonl'; import { name, version } from '../../package.json'; import { env } from '../utils/env'; import { AtlasValidationError } from '../utils/errors'; -import { parseJsonLine } from './jsonl'; export type StatsMetadata = { name: string; version: string }; From 31366b3521a1bcb385519686de7ce00767e3bda3 Mon Sep 17 00:00:00 2001 From: Cedric van Putten Date: Sun, 17 Mar 2024 15:01:46 +0100 Subject: [PATCH 3/5] refactor: rename default stats file extension to `.jsonl` --- src/utils/stats.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/stats.ts b/src/utils/stats.ts index be3e247..81db280 100644 --- a/src/utils/stats.ts +++ b/src/utils/stats.ts @@ -10,7 +10,7 @@ export type StatsMetadata = { name: string; version: string }; /** The default location of the metro stats file */ export function getStatsPath(projectRoot: string) { - return path.join(projectRoot, '.expo/stats.json'); + return path.join(projectRoot, '.expo/stats.jsonl'); } /** The information to validate if a stats file is compatible with this library version */ From 3a9b8602393a923ff6710b1898eac0912e935572 Mon Sep 17 00:00:00 2001 From: Cedric van Putten Date: Sun, 17 Mar 2024 15:26:21 +0100 Subject: [PATCH 4/5] test: add `utils/stats` tests --- .../fixtures/{ => jsonl}/specification.jsonl | 0 src/utils/__tests__/jsonl.test.ts | 4 +- src/utils/__tests__/stats.test.ts | 117 ++++++++++++++++++ src/utils/env.ts | 8 +- src/utils/middleware.ts | 2 +- src/utils/stats.ts | 12 +- 6 files changed, 129 insertions(+), 14 deletions(-) rename src/utils/__tests__/fixtures/{ => jsonl}/specification.jsonl (100%) create mode 100644 src/utils/__tests__/stats.test.ts diff --git a/src/utils/__tests__/fixtures/specification.jsonl b/src/utils/__tests__/fixtures/jsonl/specification.jsonl similarity index 100% rename from src/utils/__tests__/fixtures/specification.jsonl rename to src/utils/__tests__/fixtures/jsonl/specification.jsonl diff --git a/src/utils/__tests__/jsonl.test.ts b/src/utils/__tests__/jsonl.test.ts index 448909d..5a8ad07 100644 --- a/src/utils/__tests__/jsonl.test.ts +++ b/src/utils/__tests__/jsonl.test.ts @@ -80,8 +80,8 @@ describe('appendJsonLine', () => { */ function fixture(name: string, { temporary = false }: { temporary?: boolean } = {}) { const file = temporary - ? path.join(__dirname, 'fixtures', `${name}.temp.jsonl`) - : path.join(__dirname, 'fixtures', `${name}.jsonl`); + ? path.join(__dirname, 'fixtures/jsonl', `${name}.temp.jsonl`) + : path.join(__dirname, 'fixtures/jsonl', `${name}.jsonl`); if (temporary) { fs.writeFileSync(file, ''); diff --git a/src/utils/__tests__/stats.test.ts b/src/utils/__tests__/stats.test.ts new file mode 100644 index 0000000..4a0587c --- /dev/null +++ b/src/utils/__tests__/stats.test.ts @@ -0,0 +1,117 @@ +import { describe, expect, it } from 'bun:test'; +import fs from 'fs'; +import path from 'path'; + +import { name, version } from '../../../package.json'; +import { AtlasValidationError } from '../errors'; +import { getStatsPath, getStatsMetdata, createStatsFile, validateStatsFile } from '../stats'; + +describe('getStatsPath', () => { + it('returns default path `/.expo/stats.jsonl`', () => { + expect(getStatsPath('')).toBe('/.expo/stats.jsonl'); + }); +}); + +describe('getStatsMetadata', () => { + it('returns package name and version', () => { + expect(getStatsMetdata()).toMatchObject({ name, version }); + }); +}); + +describe('createStatsFile', () => { + it('creates a stats file with the correct metadata', async () => { + const file = fixture('create-metadata', { temporary: true }); + await createStatsFile(file); + await expect(fs.promises.readFile(file, 'utf8')).resolves.toBe( + JSON.stringify({ name, version }) + '\n' + ); + }); + + it('overwrites invalid stats file', async () => { + const file = fixture('create-invalid', { temporary: true }); + await fs.promises.writeFile(file, JSON.stringify({ name, version: '0.0.0' }) + '\n'); + await createStatsFile(file); + await expect(fs.promises.readFile(file, 'utf8')).resolves.toBe( + JSON.stringify({ name, version }) + '\n' + ); + }); + + it('reuses valid stats file', async () => { + const file = fixture('create-valid', { temporary: true }); + await fs.promises.writeFile(file, JSON.stringify({ name, version }) + '\n'); + await createStatsFile(file); + await expect(fs.promises.readFile(file, 'utf-8')).resolves.toBe( + JSON.stringify({ name, version }) + '\n' + ); + }); +}); + +describe('validateStatsFile', () => { + it('passes for valid stats file', async () => { + const file = fixture('validate-valid', { temporary: true }); + await createStatsFile(file); + await expect(validateStatsFile(file)).resolves.pass(); + }); + + it('fails for non-existing stats file', async () => { + await expect(validateStatsFile('./this-file-does-not-exists')).rejects.toThrow( + AtlasValidationError + ); + }); + + it('fails for invalid stats file', async () => { + const file = fixture('validate-invalid', { temporary: true }); + await fs.promises.writeFile(file, JSON.stringify({ name, version: '0.0.0' }) + '\n'); + await expect(validateStatsFile(file)).rejects.toThrow(AtlasValidationError); + }); + + it('skips validation when EXPO_ATLAS_NO_STATS_VALIDATION is true-ish', async () => { + using _env = env('EXPO_ATLAS_NO_STATS_VALIDATION', 'true'); + const file = fixture('validate-skip-invalid', { temporary: true }); + await fs.promises.writeFile(file, JSON.stringify({ name, version: '0.0.0' }) + '\n'); + await expect(validateStatsFile(file)).resolves.pass(); + }); +}); + +/** + * Get the file path to a fixture, by name. + * This automatically adds the required `.jsonl` or `.temp.jsonl` extension. + * Use `temporary: true` to keep it out of the repository, and reset the content automatically. + */ +function fixture(name: string, { temporary = false }: { temporary?: boolean } = {}) { + const file = temporary + ? path.join(__dirname, 'fixtures/stats', `${name}.temp.jsonl`) + : path.join(__dirname, 'fixtures/stats', `${name}.jsonl`); + + if (temporary) { + fs.writeFileSync(file, ''); + } + + return file; +} + +/** + * Change the environment variable for the duration of a test. + * This uses explicit resource management to revert the environment variable after the test. + */ +function env(key: string, value?: string): { key: string; value?: string } & Disposable { + const original = process.env[key]; + + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + + return { + key, + value, + [Symbol.dispose]() { + if (original === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + }, + }; +} diff --git a/src/utils/env.ts b/src/utils/env.ts index 7f5ee27..f22a8aa 100644 --- a/src/utils/env.ts +++ b/src/utils/env.ts @@ -1,10 +1,10 @@ import { boolish } from 'getenv'; export const env = { - get EXPO_DEBUG() { - return boolish('EXPO_DEBUG', false); + get EXPO_ATLAS_DEBUG() { + return boolish('EXPO_ATLAS_DEBUG', false); }, - get EXPO_NO_STATS_VALIDATION() { - return boolish('EXPO_NO_STATS_VALIDATION', false); + get EXPO_ATLAS_NO_STATS_VALIDATION() { + return boolish('EXPO_ATLAS_NO_STATS_VALIDATION', false); }, }; diff --git a/src/utils/middleware.ts b/src/utils/middleware.ts index 3c7e95f..5ff0246 100644 --- a/src/utils/middleware.ts +++ b/src/utils/middleware.ts @@ -33,7 +33,7 @@ export function createAtlasMiddleware(source: StatsSource) { const middleware = connect(); - if (env.EXPO_DEBUG) { + if (env.EXPO_ATLAS_DEBUG) { middleware.use(morgan('tiny')); } diff --git a/src/utils/stats.ts b/src/utils/stats.ts index 81db280..7b46773 100644 --- a/src/utils/stats.ts +++ b/src/utils/stats.ts @@ -1,7 +1,7 @@ import fs from 'fs'; import path from 'path'; -import { parseJsonLine } from './jsonl'; +import { appendJsonLine, parseJsonLine } from './jsonl'; import { name, version } from '../../package.json'; import { env } from '../utils/env'; import { AtlasValidationError } from '../utils/errors'; @@ -24,7 +24,7 @@ export async function validateStatsFile(statsFile: string, metadata = getStatsMe throw new AtlasValidationError('STATS_FILE_NOT_FOUND', statsFile); } - if (env.EXPO_NO_STATS_VALIDATION) { + if (env.EXPO_ATLAS_NO_STATS_VALIDATION) { return; } @@ -42,14 +42,12 @@ export async function validateStatsFile(statsFile: string, metadata = getStatsMe export async function createStatsFile(filePath: string) { if (fs.existsSync(filePath)) { try { - await validateStatsFile(filePath); + return await validateStatsFile(filePath); } catch { - await fs.promises.writeFile(filePath, JSON.stringify(getStatsMetdata()) + '\n'); + await fs.promises.writeFile(filePath, ''); } - - return; } await fs.promises.mkdir(path.dirname(filePath), { recursive: true }); - await fs.promises.writeFile(filePath, JSON.stringify(getStatsMetdata()) + '\n'); + await appendJsonLine(filePath, getStatsMetdata()); } From 9dc1185fc31b3ceac3b88a36518279ae18b7b9fa Mon Sep 17 00:00:00 2001 From: Cedric van Putten Date: Sun, 17 Mar 2024 15:28:22 +0100 Subject: [PATCH 5/5] test: fix issue when initializing the fixture files --- src/utils/__tests__/jsonl.test.ts | 2 ++ src/utils/__tests__/stats.test.ts | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/utils/__tests__/jsonl.test.ts b/src/utils/__tests__/jsonl.test.ts index 5a8ad07..bfd49d9 100644 --- a/src/utils/__tests__/jsonl.test.ts +++ b/src/utils/__tests__/jsonl.test.ts @@ -83,6 +83,8 @@ function fixture(name: string, { temporary = false }: { temporary?: boolean } = ? path.join(__dirname, 'fixtures/jsonl', `${name}.temp.jsonl`) : path.join(__dirname, 'fixtures/jsonl', `${name}.jsonl`); + fs.mkdirSync(path.dirname(file), { recursive: true }); + if (temporary) { fs.writeFileSync(file, ''); } diff --git a/src/utils/__tests__/stats.test.ts b/src/utils/__tests__/stats.test.ts index 4a0587c..a0dd202 100644 --- a/src/utils/__tests__/stats.test.ts +++ b/src/utils/__tests__/stats.test.ts @@ -83,6 +83,8 @@ function fixture(name: string, { temporary = false }: { temporary?: boolean } = ? path.join(__dirname, 'fixtures/stats', `${name}.temp.jsonl`) : path.join(__dirname, 'fixtures/stats', `${name}.jsonl`); + fs.mkdirSync(path.dirname(file), { recursive: true }); + if (temporary) { fs.writeFileSync(file, ''); }