Skip to content

Commit

Permalink
Fix compact todo EOL issues, add more tests (#578)
Browse files Browse the repository at this point in the history 
Co-authored-by: Steve Calvert <[email protected]>
  • Loading branch information
@Techn1x @scalvert
Techn1x and scalvert authored Aug 11, 2023
1 parent 2498962 commit 9059e88
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 21 deletions.
4 changes: 2 additions & 2 deletions src/builders.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import TodoMatcher from './todo-matcher';
const SEPARATOR = '|';

export function buildFromTodoOperations(
todoOperations: string[],
todoOperations: Operation[],
engine: Engine
): Map<FilePath, TodoMatcher> {
const existingTodos = new Map<FilePath, TodoMatcher>();
Expand Down Expand Up @@ -66,7 +66,7 @@ export function buildTodoOperations(add: Set<TodoData>, remove: Set<TodoData>):
return ops as Operation[];
}

export function toTodoDatum(todoOperation: string): [OperationType, TodoData] {
export function toTodoDatum(todoOperation: Operation): [OperationType, TodoData] {
const [
operation,
engine,
Expand Down
19 changes: 15 additions & 4 deletions src/io.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,15 +91,16 @@ 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);

writeTodoStorageFile(todoStorageFilePath, operations as Operation[]);
}

return operations.filter(Boolean) as Operation[];
return operations as Operation[];
}

/**
Expand All @@ -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);
}

/**
Expand Down Expand Up @@ -302,7 +313,7 @@ export function applyTodoChanges(
}

try {
appendFileSync(todoStorageFilePath, ops.join(EOL) + EOL);
appendTodoStorageFile(todoStorageFilePath, ops)
} finally {
release();
}
Expand Down
11 changes: 6 additions & 5 deletions tests/builders-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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',
];

Expand Down Expand Up @@ -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',
];

Expand Down Expand Up @@ -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',
];
Expand Down Expand Up @@ -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',
];
Expand All @@ -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',
Expand Down
89 changes: 79 additions & 10 deletions tests/io-test.ts
View file
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -37,6 +38,7 @@ import {
readTodoStorageFile,
resolveConflicts,
writeTodoStorageFile,
appendTodoStorageFile,
} from '../src/io';

function chunk<T>(initial: Set<T>, firstChunk = 1): [Set<T>, Set<T>] {
Expand Down Expand Up @@ -81,6 +83,12 @@ function buildReadOptions(options?: Partial<ReadTodoOptions>) {
);
}

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;

Expand All @@ -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);
Expand All @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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);
});
});

Expand Down Expand Up @@ -295,6 +362,8 @@ remove|eslint|no-unused-vars|30|19|30|33|da39a3ee5e6b4b0d3255bfef95601890afd8070
fixableWarningCount: 0,
source: '',
usedDeprecatedRules: [],
suppressedMessages: [],
fatalErrorCount: 0,
},
];

Expand Down

0 comments on commit 9059e88

Please sign in to comment.