Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run ESLint and Prettier linting in worker threads #548

Merged
merged 43 commits into from
Sep 24, 2021
Merged
Show file tree
Hide file tree
Changes from 42 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
296d225
Drop `fs-extra` in favour of vanilla `fs`
72636c Sep 21, 2021
ed7e6c9
Run ESLint and Prettier formatting in process
72636c Sep 21, 2021
943ae62
Merge branch 'master' into format-in-process
72636c Sep 21, 2021
c33da9c
Fix bad merge
72636c Sep 21, 2021
8423cde
Merge branch 'master' into format-in-process
72636c Sep 22, 2021
cefc549
Recommend `yarn link` for local integration
72636c Sep 22, 2021
a912cd3
Log elapsed timing information
72636c Sep 22, 2021
f194811
Write Prettified output
72636c Sep 22, 2021
c950660
Add TODO for ESLint caching
72636c Sep 22, 2021
10e7578
Improve logging output further
72636c Sep 22, 2021
468200f
Merge branch 'master' into format-in-process
72636c Sep 22, 2021
b68850a
Update doco
72636c Sep 22, 2021
7ddfc06
Remove lint spoilers that slipped through
72636c Sep 23, 2021
cd5c77f
Merge branch 'master' into format-in-process
72636c Sep 23, 2021
72dc802
Update slimy-cars-wash.md
72636c Sep 23, 2021
035f470
Merge branch 'master' into format-in-process
72636c Sep 23, 2021
e0c46b0
Revert Buildkite spoiler
72636c Sep 23, 2021
37256ba
Merge branch 'master' into format-in-process
72636c Sep 23, 2021
2cb9e61
Test format command
72636c Sep 23, 2021
cdc5943
Add leading newline to diff snapshots
72636c Sep 23, 2021
db25721
Avoid experimental `fs.promises.cp`
72636c Sep 23, 2021
830469e
Try to fix TypeScript confusion in tests
72636c Sep 23, 2021
5d45137
Restore original cwd
72636c Sep 23, 2021
e8af956
Remove spoiler again
72636c Sep 23, 2021
e576007
Clean up logging snapshot
72636c Sep 23, 2021
3282788
Check exit code last
72636c Sep 23, 2021
346bcd3
Run ESLint and Prettier linting in worker threads
72636c Sep 23, 2021
72b8bc0
Support serial execution
72636c Sep 23, 2021
92d0b6f
Prettify
72636c Sep 23, 2021
e84c504
Split up files further
72636c Sep 23, 2021
759df52
Tidy some changesets
72636c Sep 23, 2021
7b80124
Reset exit code between test cases
72636c Sep 23, 2021
054ed69
Remove sleep hack
72636c Sep 23, 2021
091ef99
Merge branch 'master' into lint-in-process
72636c Sep 24, 2021
fc514e2
Fix bad fs-extra merge
72636c Sep 24, 2021
39c759f
Rebase lint snapshots
72636c Sep 24, 2021
241bcdf
Restore comment from bad merge
72636c Sep 24, 2021
4303e2e
Reformat changesets
72636c Sep 24, 2021
a8d1fef
Rebase snapshot
72636c Sep 24, 2021
9df026a
Merge branch 'master' into lint-in-process
72636c Sep 24, 2021
16adeb9
Align newlining in format
72636c Sep 24, 2021
cc561a0
Merge branch 'master' into lint-in-process
72636c Sep 24, 2021
79a6b6e
De-confuse some worker functions
72636c Sep 24, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/few-rules-marry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"skuba": minor
---

lint: Run ESLint and Prettier in worker threads

This reduces the number of Node.js processes spawned by `skuba lint`. We've also been able to significantly enhance our logging output as a result, particularly when the `--debug` flag is supplied.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,5 @@ yarn-error.log
# end managed by skuba

/integration/format/
/integration/lint/
/template/**/yarn.lock
8 changes: 3 additions & 5 deletions docs/cli/lint.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,9 @@ This command should be run in CI to verify that [`skuba format`] was applied and
```shell
skuba lint

# Prettier | Checking formatting...
# Prettier | All matched files use Prettier code style!
# Prettier | prettier --check . exited with code 0
# ESLint | eslint --ext=js,ts,tsx --report-unused-disable-directives . exited with code 0
# tsc | TSFILE: ...
# ESLint | Processed 123 files in 1.23s.
# Prettier | Processed 123 files in 1.23s.
# tsc | TSFILE: /lib/tsconfig.tsbuildinfo
# tsc | tsc --noEmit exited with code 0
```

Expand Down
1 change: 1 addition & 0 deletions src/cli/__snapshots__/format.int.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ Formatted 1 file:
a/a/a.ts

ESLint found issues that require triage.

"
`;

Expand Down
122 changes: 122 additions & 0 deletions src/cli/__snapshots__/lint.int.test.ts.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`fixable 1`] = `
"
ESLint | Processed 2 files in <random>s.
ESLint |
<random>/a/a/a.ts
3:1 error \`fs\` import should occur before import of \`path\` import/order

✖ 1 problem (1 error, 0 warnings)
1 error and 0 warnings potentially fixable with the \`--fix\` option.

Prettier | Processed 6 files in <random>s.
Prettier | Flagged 3 files:
Prettier | b.md
Prettier | c.json
Prettier | d.js
tsc | TSFILE: <random>/lib/tsconfig.tsbuildinfo
tsc | tsc --noEmit exited with code 0

ESLint, Prettier found issues that require triage.

"
`;

exports[`ok --debug 1`] = `
"
ESLint | Initialising ESLint...
ESLint | Processing files...
ESLint | Processed 2 files in <random>s.
ESLint | ○ a/a/a.ts
ESLint | ○ d.js
Prettier | Initialising Prettier...
Prettier | Detected project root: <random>
Prettier | Discovering files...
Prettier | Discovered 6 files.
Prettier | Reading files...
Prettier | Linting files...
Prettier | b.md
Prettier | parser: markdown
Prettier | c.json
Prettier | parser: json
Prettier | d.js
Prettier | parser: babel
Prettier | package.json
Prettier | parser: json-stringify
Prettier | tsconfig.json
Prettier | parser: json
Prettier | a/a/a.ts
Prettier | parser: typescript
Prettier | Writing 0 files...
Prettier | Processed 6 files in <random>s.
tsc | TSFILE: <random>/lib/tsconfig.tsbuildinfo
tsc | Files: <random>
tsc | Lines of Library: <random>
tsc | Lines of Definitions: <random>
tsc | Lines of TypeScript: <random>
tsc | Lines of JavaScript: <random>
tsc | Lines of JSON: <random>
tsc | Lines of Other: <random>
tsc | Nodes of Library: <random>
tsc | Nodes of Definitions: <random>
tsc | Nodes of TypeScript: <random>
tsc | Nodes of JavaScript: <random>
tsc | Nodes of JSON: <random>
tsc | Nodes of Other: <random>
tsc | Identifiers: <random>
tsc | Symbols: <random>
tsc | Types: <random>
tsc | Instantiations: <random>
tsc | Memory used: <random>K
tsc | Assignability cache size: <random>
tsc | Identity cache size: <random>
tsc | Subtype cache size: <random>
tsc | Strict subtype cache size: <random>
tsc | I/O Read time: <random>s
tsc | Parse time: <random>s
tsc | ResolveModule time: <random>s
tsc | ResolveTypeReference time: <random>s
tsc | Program time: <random>s
tsc | Bind time: <random>s
tsc | Check time: <random>s
tsc | I/O Write time: <random>s
tsc | printTime time: <random>s
tsc | Emit time: <random>s
tsc | Total time: <random>s
tsc | tsc --extendedDiagnostics --noEmit exited with code 0

"
`;

exports[`ok 1`] = `
"
ESLint | Processed 2 files in <random>s.
Prettier | Processed 6 files in <random>s.
tsc | TSFILE: <random>/lib/tsconfig.tsbuildinfo
tsc | tsc --noEmit exited with code 0

"
`;

exports[`unfixable 1`] = `
"
ESLint | Processed 2 files in <random>s.
ESLint |
<random>/a/a/a.ts
3:1 error \`fs\` import should occur before import of \`path\` import/order
8:3 error Unexpected console statement no-console

✖ 2 problems (2 errors, 0 warnings)
1 error and 0 warnings potentially fixable with the \`--fix\` option.

Prettier | Processed 6 files in <random>s.
Prettier | Flagged 1 file:
Prettier | a/a/a.ts
tsc | TSFILE: <random>/lib/tsconfig.tsbuildinfo
tsc | tsc --noEmit exited with code 0

ESLint, Prettier found issues that require triage.

"
`;
1 change: 1 addition & 0 deletions src/cli/format.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export const format = async (args = process.argv) => {
];

log.err(tools.join(', '), 'found issues that require triage.');
log.newline();

process.exitCode = 1;
};
108 changes: 108 additions & 0 deletions src/cli/lint.int.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
import crypto from 'crypto';
import fs from 'fs';
import path from 'path';
import stream from 'stream';

import { copy } from 'fs-extra';

import { lint } from './lint';

jest.setTimeout(30_000);

const stdoutMock = jest.fn();

jest
.spyOn(console, 'log')
.mockImplementation((...args) => stdoutMock(`${args.join(' ')}\n`));

const tscOutputStream = new stream.PassThrough().on('data', stdoutMock);

const BASE_PATH = path.join(__dirname, '..', '..', 'integration', 'base');

const TEMP_PATH = path.join(__dirname, '..', '..', 'integration', 'lint');

const stdout = (randomMatcher: RegExp) =>
stdoutMock.mock.calls
.flat(1)
.join('')
.replace(/ in [\d\.]+s\./g, ' in <random>s.')
.replace(
/tsc \| Lines of ([^:]+):[ ]+\d+/g,
'tsc | Lines of $1: <random>',
)
.replace(
/tsc \| Nodes of ([^:]+):[ ]+\d+/g,
'tsc | Nodes of $1: <random>',
)
.replace(
/tsc \| (Files|Identifiers|Symbols|Types|Instantiations|Memory used):[ ]+\d+/g,
'tsc | $1: <random>',
)
.replace(
/tsc \| (.+) cache size:[ ]+\d+/g,
'tsc | $1 cache size: <random>',
)
.replace(
/tsc \| (.+) time:[ ]+[\d\.]+s/g,
'tsc | $1 time: <random>s',
)
.replace(randomMatcher, '<random>');

const prepareTempDirectory = async (baseDir: string, tempDir: string) => {
await copy(baseDir, tempDir, { recursive: true });

process.chdir(tempDir);
};

const originalCwd = process.cwd();

beforeEach(() => {
jest.clearAllMocks();

process.exitCode = undefined;
});

afterAll(() => {
// Restore the original working directory to avoid confusion in other tests.
process.chdir(originalCwd);

// Clean up temporary directories to avoid subsequent formatting and linting
// warnings and to save on disk space. This can be commented out to inspect
// the output.
return fs.promises.rm(TEMP_PATH, {
force: true,
recursive: true,
});
});

interface Args {
args: string[];
base: string;
exitCode: number | undefined;
}

test.each`
description | args | base | exitCode
${'fixable'} | ${[]} | ${'fixable'} | ${1}
${'ok'} | ${[]} | ${'ok'} | ${undefined}
${'ok --debug'} | ${['--debug']} | ${'ok'} | ${undefined}
${'unfixable'} | ${[]} | ${'unfixable'} | ${1}
`('$description', async ({ args, base, exitCode }: Args) => {
const baseDir = path.join(BASE_PATH, base);

const tempDir = path.join(
TEMP_PATH,
`${base}-${crypto.randomBytes(32).toString('hex')}`,
);

await prepareTempDirectory(baseDir, tempDir);

await expect(
// Force `--serial` as worker threads can't run in a TypeScript context.
lint([...args, '--serial'], tscOutputStream),
).resolves.toBeUndefined();

expect(stdout(new RegExp(tempDir, 'g'))).toMatchSnapshot();

expect(process.exitCode).toBe(exitCode);
});
90 changes: 1 addition & 89 deletions src/cli/lint.test.ts
Original file line number Diff line number Diff line change
@@ -1,95 +1,7 @@
// Uncomment the following line when there's an intentionally skipped test.
// /* eslint-disable jest/no-disabled-tests */

import concurrently from 'concurrently';

import { internalLint, lint } from './lint';

jest.mock('concurrently');

const concurrentlyCalls = () =>
(concurrently as unknown as jest.Mock<typeof concurrently>).mock.calls
.flat(2)
.map(({ env, maxProcesses, ...rest }) => ({
...(env !== undefined && { env: 'REDACTED' }),
...(maxProcesses !== undefined && { maxProcesses: 'REDACTED' }),
...rest,
}));

describe('lint', () => {
afterEach(jest.clearAllMocks);

it('handles no flags', async () => {
await expect(lint([])).resolves.toBeUndefined();

expect(concurrentlyCalls()).toMatchInlineSnapshot(`
Array [
Object {
"command": "eslint --ext=js,ts,tsx --report-unused-disable-directives .",
"env": "REDACTED",
"name": "ESLint ",
"prefixColor": "magenta",
},
Object {
"command": "tsc --noEmit",
"env": "REDACTED",
"name": "tsc ",
"prefixColor": "blue",
},
Object {
"command": "prettier --check .",
"env": "REDACTED",
"name": "Prettier",
"prefixColor": "cyan",
},
Object {
"maxProcesses": "REDACTED",
"outputStream": undefined,
"prefix": "{name} |",
},
]
`);
});

it('handles debug flag', async () => {
await expect(
lint(['something', '--DeBuG', 'else']),
).resolves.toBeUndefined();

expect(concurrentlyCalls()).toMatchInlineSnapshot(`
Array [
Object {
"command": "eslint --debug --ext=js,ts,tsx --report-unused-disable-directives .",
"env": "REDACTED",
"name": "ESLint ",
"prefixColor": "magenta",
},
Object {
"command": "tsc --extendedDiagnostics --noEmit",
"env": "REDACTED",
"name": "tsc ",
"prefixColor": "blue",
},
Object {
"command": "prettier --check --loglevel debug .",
"env": "REDACTED",
"name": "Prettier",
"prefixColor": "cyan",
},
Object {
"maxProcesses": "REDACTED",
"outputStream": undefined,
"prefix": "{name} |",
},
]
`);
});
});

describe('internalLint', () => {
it('passes on skuba itself', () =>
expect(internalLint()).resolves.toBeUndefined());
});
import './lint';

/**
* Ensure compatibility between our lint command and new syntax features.
Expand Down
Loading