From 9059e88f4a396461eda3e452e56998e3fe27c2d9 Mon Sep 17 00:00:00 2001 From: Brad Overton Date: Sat, 12 Aug 2023 02:45:28 +1000 Subject: [PATCH] Fix compact todo EOL issues, add more tests (#578) Co-authored-by: Steve Calvert --- src/builders.ts | 4 +- src/io.ts | 19 +++++++-- tests/builders-test.ts | 11 +++--- tests/io-test.ts | 89 +++++++++++++++++++++++++++++++++++++----- 4 files changed, 102 insertions(+), 21 deletions(-) diff --git a/src/builders.ts b/src/builders.ts index 29355fcd..ab39a0cc 100644 --- a/src/builders.ts +++ b/src/builders.ts @@ -18,7 +18,7 @@ import TodoMatcher from './todo-matcher'; const SEPARATOR = '|'; export function buildFromTodoOperations( - todoOperations: string[], + todoOperations: Operation[], engine: Engine ): Map { const existingTodos = new Map(); @@ -66,7 +66,7 @@ export function buildTodoOperations(add: Set, remove: Set): return ops as Operation[]; } -export function toTodoDatum(todoOperation: string): [OperationType, TodoData] { +export function toTodoDatum(todoOperation: Operation): [OperationType, TodoData] { const [ operation, engine, diff --git a/src/io.ts b/src/io.ts index 3e3aaeb0..90f46924 100644 --- a/src/io.ts +++ b/src/io.ts @@ -91,7 +91,8 @@ export function readTodoStorageFile(todoStorageFilePath: string): Operation[] { encoding: 'utf8', }); - let operations = todoContents.split(EOL) as OperationOrConflictLine[]; + // when splitting by EOL, make sure to filter off the '' caused by the final EOL + let operations = todoContents.split(EOL).filter((op: string) => op !== '') as OperationOrConflictLine[] if (hasConflicts(todoContents)) { operations = resolveConflicts(operations); @@ -99,7 +100,7 @@ export function readTodoStorageFile(todoStorageFilePath: string): Operation[] { writeTodoStorageFile(todoStorageFilePath, operations as Operation[]); } - return operations.filter(Boolean) as Operation[]; + return operations as Operation[]; } /** @@ -109,7 +110,17 @@ export function readTodoStorageFile(todoStorageFilePath: string): Operation[] { * @param operations - An array of string operations that are used to recreate todos. */ export function writeTodoStorageFile(todoStorageFilePath: string, operations: Operation[]): void { - writeFileSync(todoStorageFilePath, operations.join(EOL)); + writeFileSync(todoStorageFilePath, operations.join(EOL) + EOL); +} + +/** + * Appends the operations to the .lint-todo storage file to the path provided by the todoStorageFilePath + * + * @param todoStorageFilePath - The .lint-todo storage file path. + * @param operations - An array of string operations that are used to recreate todos. + */ +export function appendTodoStorageFile(todoStorageFilePath: string, operations: Operation[]): void { + appendFileSync(todoStorageFilePath, operations.join(EOL) + EOL); } /** @@ -302,7 +313,7 @@ export function applyTodoChanges( } try { - appendFileSync(todoStorageFilePath, ops.join(EOL) + EOL); + appendTodoStorageFile(todoStorageFilePath, ops) } finally { release(); } diff --git a/tests/builders-test.ts b/tests/builders-test.ts index e1b7b758..db3dde12 100644 --- a/tests/builders-test.ts +++ b/tests/builders-test.ts @@ -3,6 +3,7 @@ import { describe, beforeEach, it, expect } from 'vitest'; import { differenceInDays } from 'date-fns'; import { getDatePart } from '../src/date-utils'; import { buildFromTodoOperations, buildTodoDatum, buildTodoOperations } from '../src/builders'; +import { Operation } from '../src/types'; import { createTmpDir } from './utils/tmp-dir'; import { buildMaybeTodosFromFixture } from './utils/build-todo-data'; @@ -225,7 +226,7 @@ describe('builders', () => { describe('buildFromOperations', () => { it('builds single todo from single add', () => { - const todoOperations: string[] = [ + const todoOperations: Operation[] = [ 'add|ember-template-lint|no-implicit-this|174|8|174|8|864e3ef2438ac413d96a032cdd141e567fcc04b3|1629331200000|2493248400000|2493334800000|addon/templates/components/foo.hbs', ]; @@ -262,7 +263,7 @@ describe('builders', () => { }); it('builds single todo from single add with pipes in filePath', () => { - const todoOperations: string[] = [ + const todoOperations: Operation[] = [ 'add|ember-template-lint|no-implicit-this|174|8|174|8|864e3ef2438ac413d96a032cdd141e567fcc04b3|1629331200000|2493248400000|2493334800000|addon/templates/components/fo|o.hbs', ]; @@ -299,7 +300,7 @@ describe('builders', () => { }); it('builds single todo from multiple identical adds', () => { - const todoOperations: string[] = [ + const todoOperations: Operation[] = [ 'add|ember-template-lint|no-implicit-this|174|8|174|8|864e3ef2438ac413d96a032cdd141e567fcc04b3|1629331200000|2493248400000|2493334800000|addon/templates/components/foo.hbs', 'add|ember-template-lint|no-implicit-this|174|8|174|8|864e3ef2438ac413d96a032cdd141e567fcc04b3|1629331200000|2493248400000|2493334800000|addon/templates/components/foo.hbs', ]; @@ -334,7 +335,7 @@ describe('builders', () => { }); it('builds empty todos from single add and remove', () => { - const todoOperations: string[] = [ + const todoOperations: Operation[] = [ 'add|ember-template-lint|no-implicit-this|174|8|174|8|864e3ef2438ac413d96a032cdd141e567fcc04b3|1629331200000|2493248400000|2493334800000|addon/templates/components/foo.hbs', 'remove|ember-template-lint|no-implicit-this|174|8|174|8|864e3ef2438ac413d96a032cdd141e567fcc04b3|1629331200000|2493248400000|2493334800000|addon/templates/components/foo.hbs', ]; @@ -345,7 +346,7 @@ describe('builders', () => { }); it('builds empty todos from single add and multiple identical removes', () => { - const todoOperations: string[] = [ + const todoOperations: Operation[] = [ 'add|ember-template-lint|no-implicit-this|174|8|174|8|864e3ef2438ac413d96a032cdd141e567fcc04b3|1629331200000|2493248400000|2493334800000|addon/templates/components/foo.hbs', 'remove|ember-template-lint|no-implicit-this|174|8|174|8|864e3ef2438ac413d96a032cdd141e567fcc04b3|1629331200000|2493248400000|2493334800000|addon/templates/components/foo.hbs', 'remove|ember-template-lint|no-implicit-this|174|8|174|8|864e3ef2438ac413d96a032cdd141e567fcc04b3|1629331200000|2493248400000|2493334800000|addon/templates/components/foo.hbs', diff --git a/tests/io-test.ts b/tests/io-test.ts index 6595454e..e7ccfffa 100644 --- a/tests/io-test.ts +++ b/tests/io-test.ts @@ -1,6 +1,7 @@ import { describe, beforeEach, it, expect } from 'vitest'; -import { existsSync } from 'fs-extra'; +import { existsSync, readFileSync } from 'fs-extra'; import { subDays } from 'date-fns'; +import { EOL } from 'node:os'; import { getDatePart, getTodoStorageFilePath, @@ -37,6 +38,7 @@ import { readTodoStorageFile, resolveConflicts, writeTodoStorageFile, + appendTodoStorageFile, } from '../src/io'; function chunk(initial: Set, firstChunk = 1): [Set, Set] { @@ -81,6 +83,12 @@ function buildReadOptions(options?: Partial) { ); } +function joinOperations(...operations: (Operation[] | Operation)[]): string { + // eslint-disable-next-line unicorn/prefer-spread + const flatOperations = ([] as Operation[]).concat(...operations); + return flatOperations.join(EOL) + EOL; +} + describe('io', () => { let tmp: string; @@ -100,6 +108,43 @@ describe('io', () => { }); }); + describe('writeTodoStorageFile', () => { + it('writes operations, joining with EOL and ending with EOL', () => { + const todoStorageFilePath = getTodoStorageFilePath(tmp); + const operations: Operation[] = [ + 'add|eslint|no-prototype-builtins|25|21|25|35|da39a3ee5e6b4b0d3255bfef95601890afd80709|1637107200000|||app/controllers/settings.js', + 'add|eslint|no-prototype-builtins|26|19|26|33|da39a3ee5e6b4b0d3255bfef95601890afd80709|1637107200000|||app/controllers/settings.js', + ]; + + writeTodoStorageFile(todoStorageFilePath, operations); + + const todoContents = readFileSync(todoStorageFilePath, { encoding: 'utf8' }); + expect(todoContents).toEqual(joinOperations(operations)); + }); + }); + + describe('appendTodoStorageFile', () => { + it('appends operations, joining with EOL and ending with EOL', () => { + const todoStorageFilePath = getTodoStorageFilePath(tmp); + const operations: Operation[] = [ + 'add|eslint|no-prototype-builtins|25|21|25|35|da39a3ee5e6b4b0d3255bfef95601890afd80709|1637107200000|||app/controllers/settings.js', + 'add|eslint|no-prototype-builtins|26|19|26|33|da39a3ee5e6b4b0d3255bfef95601890afd80709|1637107200000|||app/controllers/settings.js', + ]; + + writeTodoStorageFile(todoStorageFilePath, operations); + + const moreOperations: Operation[] = [ + 'add|eslint|no-prototype-builtins|27|21|25|35|da39a3ee5e6b4b0d3255bfef95601890afd80709|1637107200000|||app/controllers/settings.js', + 'add|eslint|no-prototype-builtins|28|19|26|33|da39a3ee5e6b4b0d3255bfef95601890afd80709|1637107200000|||app/controllers/settings.js', + ]; + + appendTodoStorageFile(todoStorageFilePath, moreOperations); + + const todoContents = readFileSync(todoStorageFilePath, { encoding: 'utf8' }); + expect(todoContents).toEqual(joinOperations(operations, moreOperations)); + }); + }); + describe('compactTodoStorageFile', () => { it('preserves existing file when no remove operations are present', () => { const todoStorageFilePath = getTodoStorageFilePath(tmp); @@ -110,9 +155,16 @@ describe('io', () => { writeTodoStorageFile(todoStorageFilePath, operations); - compactTodoStorageFile(tmp, buildReadOptions()); + const { originalOperations, compactedOperations, compacted } = compactTodoStorageFile(tmp); expect(readTodoStorageFile(todoStorageFilePath)).toEqual(operations); + + const todoContents = readFileSync(todoStorageFilePath, { encoding: 'utf8' }); + expect(todoContents.endsWith(EOL)).toEqual(true); + + expect(originalOperations).toEqual(operations); + expect(compactedOperations).toEqual(operations); + expect(compacted).toEqual(0); }); it('compacts existing file when remove operations are present', () => { @@ -121,17 +173,24 @@ describe('io', () => { 'add|eslint|no-prototype-builtins|25|21|25|35|da39a3ee5e6b4b0d3255bfef95601890afd80709|1637107200000|||app/controllers/settings.js', 'add|eslint|no-prototype-builtins|26|19|26|33|da39a3ee5e6b4b0d3255bfef95601890afd80709|1637107200000|||app/controllers/settings.js', ]; - const removeOperations: Operation[] = [ 'remove|eslint|no-prototype-builtins|25|21|25|35|da39a3ee5e6b4b0d3255bfef95601890afd80709|1637107200000|||app/controllers/settings.js', 'remove|eslint|no-prototype-builtins|26|19|26|33|da39a3ee5e6b4b0d3255bfef95601890afd80709|1637107200000|||app/controllers/settings.js', ]; + const operations = [...addOperations, ...removeOperations]; + + writeTodoStorageFile(todoStorageFilePath, operations); + + const { originalOperations, compactedOperations, compacted } = compactTodoStorageFile(tmp); - writeTodoStorageFile(todoStorageFilePath, [...addOperations, ...removeOperations]); + expect(readTodoStorageFile(todoStorageFilePath)).toEqual(compactedOperations); - compactTodoStorageFile(tmp); + const todoContents = readFileSync(todoStorageFilePath, { encoding: 'utf8' }); + expect(todoContents.endsWith(EOL)).toEqual(true); - expect(readTodoStorageFile(todoStorageFilePath)).toEqual([]); + expect(originalOperations).toEqual(operations); + expect(compactedOperations).toEqual([]); + expect(compacted).toEqual(4); }); it('compacts existing file when interleaved remove operations are present', () => { @@ -147,12 +206,16 @@ describe('io', () => { writeTodoStorageFile(todoStorageFilePath, operations); - compactTodoStorageFile(tmp); + const { originalOperations, compactedOperations, compacted } = compactTodoStorageFile(tmp); - expect(readTodoStorageFile(todoStorageFilePath)).toEqual([ + expect(readTodoStorageFile(todoStorageFilePath)).toEqual(compactedOperations); + + expect(originalOperations).toEqual(operations); + expect(compactedOperations).toEqual([ 'add|eslint|no-prototype-builtins|65|27|65|41|da39a3ee5e6b4b0d3255bfef95601890afd80709|1637107200000|||tests/unit/services/insights-test.js', 'add|eslint|no-prototype-builtins|80|27|80|41|da39a3ee5e6b4b0d3255bfef95601890afd80709|1637107200000|||tests/unit/services/insights-test.js', ]); + expect(compacted).toEqual(4); }); it('compacts respects multiple co-existing engines', () => { @@ -170,15 +233,19 @@ describe('io', () => { writeTodoStorageFile(todoStorageFilePath, operations); - compactTodoStorageFile(tmp); + const { originalOperations, compactedOperations, compacted } = compactTodoStorageFile(tmp); + + expect(readTodoStorageFile(todoStorageFilePath)).toEqual(compactedOperations); - expect(readTodoStorageFile(todoStorageFilePath)).toEqual([ + expect(originalOperations).toEqual(operations); + expect(compactedOperations).toEqual([ 'add|eslint|no-prototype-builtins|30|21|25|35|da39a3ee5e6b4b0d3255bfef95601890afd80709|1637107200000|||app/controllers/settings.js', 'add|ember-template-lint|no-html-comments|26|19|26|33|da39a3ee5e6b4b0d3255bfef95601890afd80709|1637107200000|||app/templates/settings.hbs', 'add|eslint|no-prototype-builtins|65|27|65|41|da39a3ee5e6b4b0d3255bfef95601890afd80709|1637107200000|||tests/unit/services/insights-test.js', 'add|eslint|no-prototype-builtins|90|27|65|41|da39a3ee5e6b4b0d3255bfef95601890afd80709|1637107200000|||tests/unit/services/insights-test.js', 'add|ember-template-lint|no-html-comments|80|27|80|41|da39a3ee5e6b4b0d3255bfef95601890afd80709|1637107200000|||app/templates/insights.hbs', ]); + expect(compacted).toEqual(3); }); }); @@ -295,6 +362,8 @@ remove|eslint|no-unused-vars|30|19|30|33|da39a3ee5e6b4b0d3255bfef95601890afd8070 fixableWarningCount: 0, source: '', usedDeprecatedRules: [], + suppressedMessages: [], + fatalErrorCount: 0, }, ];