Skip to content

Commit 9cd7422

Browse files
crutchcorncrutchcorn
committed
fix: drastically improve sigkill and sigterm behavior
1 parent 59e5da3 commit 9cd7422

File tree

8 files changed

+101
-72
lines changed

8 files changed

+101
-72
lines changed

‎package.json‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,11 @@
4444
"pretty-format": "^27.0.2",
4545
"strip-ansi": "^6.0.1",
4646
"strip-final-newline": "^2.0.0",
47-
"tree-kill": "^1.2.2",
48-
"is-running": "^2.1.0"
47+
"tree-kill": "^1.2.2"
4948
},
5049
"devDependencies": {
5150
"@types/lz-string": "^1.3.34",
5251
"@types/strip-final-newline": "^3.0.0",
53-
"@types/is-running": "^2.1.0",
5452
"inquirer": "^8.2.0",
5553
"jest-in-case": "^1.0.2",
5654
"jest-snapshot-serializer-ansi": "^1.0.0",

‎src/__tests__/events.js‎

Lines changed: 57 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,103 +1,106 @@
11
const {resolve} = require('path')
2-
const isRunning = require('is-running')
3-
const {render} = require('../pure')
2+
const {render, cleanup} = require('../pure')
43
const {fireEvent} = require('../events')
54
const {waitFor} = require("../wait-for");
65
const {default: userEvent} = require("../user-event");
76

7+
afterEach(async () => {
8+
await cleanup();
9+
})
10+
811
test('fireEvent write works', async () => {
9-
const props = await render('node', [
10-
resolve(__dirname, './execute-scripts/stdio-inquirer.js'),
11-
])
12+
const props = await render('node', [
13+
resolve(__dirname, './execute-scripts/stdio-inquirer.js'),
14+
])
1215

13-
const {clear, findByText} = props;
16+
const {clear, findByText} = props;
1417

15-
const instance = await findByText('First option')
18+
const instance = await findByText('First option')
1619

17-
expect(instance).toBeTruthy()
20+
expect(instance).toBeTruthy()
1821

19-
// Windows uses ">", Linux/MacOS use "❯"
20-
expect(await findByText(/[❯>] One/)).toBeTruthy()
22+
// Windows uses ">", Linux/MacOS use "❯"
23+
expect(await findByText(/[❯>] One/)).toBeTruthy()
2124

22-
clear()
25+
clear()
2326

24-
const down = "\x1B\x5B\x42";
25-
fireEvent(instance, 'write', {value: down})
27+
const down = "\x1B\x5B\x42";
28+
fireEvent(instance, 'write', {value: down})
2629

27-
expect(await findByText(/[❯>] Two/)).toBeTruthy()
30+
expect(await findByText(/[❯>] Two/)).toBeTruthy()
2831

29-
clear()
32+
clear()
3033

31-
const enter = "\x0D";
32-
fireEvent(instance, 'write', {value: enter})
34+
const enter = "\x0D";
35+
fireEvent(instance, 'write', {value: enter})
3336

34-
expect(await findByText('First option: Two')).toBeTruthy()
37+
expect(await findByText('First option: Two')).toBeTruthy()
3538
})
3639

3740
// Refactor to use `fireEvent` and not `userEvent` style
38-
test('FireEvent SigTerm works', async () => {
39-
const {findByText} = await render('node', [
40-
resolve(__dirname, './execute-scripts/stdio-inquirer.js'),
41-
])
41+
test.only('FireEvent SigTerm works', async () => {
42+
const {findByText} = await render('node', [
43+
resolve(__dirname, './execute-scripts/stdio-inquirer.js'),
44+
])
4245

43-
const instance = await findByText('First option')
46+
const instance = await findByText('First option')
4447

45-
expect(instance).toBeTruthy()
48+
expect(instance).toBeTruthy()
4649

47-
fireEvent.sigterm(instance);
50+
await fireEvent.sigterm(instance);
4851

49-
await waitFor(() => expect(isRunning(instance.pid)).toBeFalsy())
52+
await waitFor(() => expect(instance.hasExit()).toBeTruthy())
5053
})
5154

5255
test('userEvent basic keyboard works', async () => {
53-
const {findByText} = await render('node', [
54-
resolve(__dirname, './execute-scripts/stdio-inquirer-input.js'),
55-
])
56+
const {findByText} = await render('node', [
57+
resolve(__dirname, './execute-scripts/stdio-inquirer-input.js'),
58+
])
5659

57-
const instance = await findByText('What is your name?');
58-
expect(instance).toBeTruthy();
60+
const instance = await findByText('What is your name?');
61+
expect(instance).toBeTruthy();
5962

60-
userEvent.keyboard(instance, "Test")
63+
userEvent.keyboard(instance, "Test")
6164

62-
expect(await findByText('Test')).toBeTruthy();
65+
expect(await findByText('Test')).toBeTruthy();
6366
})
6467

6568
test('userEvent basic keyboard works when bound', async () => {
66-
const {findByText, userEvent: userEventLocal} = await render('node', [
67-
resolve(__dirname, './execute-scripts/stdio-inquirer-input.js'),
68-
])
69+
const {findByText, userEvent: userEventLocal} = await render('node', [
70+
resolve(__dirname, './execute-scripts/stdio-inquirer-input.js'),
71+
])
6972

70-
const instance = await findByText('What is your name?');
71-
expect(instance).toBeTruthy();
73+
const instance = await findByText('What is your name?');
74+
expect(instance).toBeTruthy();
7275

73-
userEventLocal.keyboard("Test")
76+
userEventLocal.keyboard("Test")
7477

75-
expect(await findByText('Test')).toBeTruthy();
78+
expect(await findByText('Test')).toBeTruthy();
7679
})
7780

7881
test("UserEvent.keyboard enter key works", async () => {
79-
const props = await render('node', [
80-
resolve(__dirname, './execute-scripts/stdio-inquirer.js'),
81-
])
82+
const props = await render('node', [
83+
resolve(__dirname, './execute-scripts/stdio-inquirer.js'),
84+
])
8285

83-
const {clear, findByText, userEvent: userEventLocal} = props;
86+
const {clear, findByText, userEvent: userEventLocal} = props;
8487

85-
const instance = await findByText('First option')
88+
const instance = await findByText('First option')
8689

87-
expect(instance).toBeTruthy()
90+
expect(instance).toBeTruthy()
8891

89-
// Windows uses ">", Linux/MacOS use "❯"
90-
expect(await findByText(/[❯>] One/)).toBeTruthy()
92+
// Windows uses ">", Linux/MacOS use "❯"
93+
expect(await findByText(/[❯>] One/)).toBeTruthy()
9194

92-
clear()
95+
clear()
9396

94-
userEventLocal.keyboard('[ArrowDown]')
97+
userEventLocal.keyboard('[ArrowDown]')
9598

96-
expect(await findByText(/[❯>] Two/)).toBeTruthy()
99+
expect(await findByText(/[❯>] Two/)).toBeTruthy()
97100

98-
clear()
101+
clear()
99102

100-
userEventLocal.keyboard('[Enter]')
103+
userEventLocal.keyboard('[Enter]')
101104

102-
expect(await findByText('First option: Two')).toBeTruthy()
105+
expect(await findByText('First option: Two')).toBeTruthy()
103106
})

‎src/event-map.ts‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,29 @@
1-
import kill from 'tree-kill';
1+
import treeKill from 'tree-kill';
22

3-
import isRunning from 'is-running';
4-
import {TestInstance} from "../types/pure";
3+
import {TestInstance} from "../types";
4+
5+
const kill = (instance: TestInstance, signal: string) => new Promise<void>((resolve, reject) => {
6+
if (!instance.pid || (instance.pid && instance.hasExit())) {
7+
resolve();
8+
return;
9+
}
10+
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
11+
treeKill(instance.pid, signal, err => {
12+
if (err) {
13+
if (err.message.includes("The process") && err.message.includes("not found.")) {
14+
resolve()
15+
return;
16+
}
17+
reject(err)
18+
}
19+
else resolve()
20+
})
21+
});
522

623
const eventMap = {
7-
sigterm: (instance: TestInstance) => instance.pid && isRunning(instance.pid) && kill(instance.pid),
8-
sigkill: (instance: TestInstance) => instance.pid && isRunning(instance.pid) && kill(instance.pid, 'SIGKILL'),
9-
write: (instance: TestInstance, props: {value: string}) => instance.stdin.write(props.value)
24+
sigterm: (instance: TestInstance) => kill(instance, "SIGTERM"),
25+
sigkill: (instance: TestInstance) => kill(instance, "SIGKILL"),
26+
write: (instance: TestInstance, props: { value: string }) => instance.stdin.write(props.value)
1027
}
1128

1229
export {eventMap}

‎src/index.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,15 @@ if (typeof process === 'undefined' || !process.env?.CTL_SKIP_AUTO_CLEANUP) {
88
// ignore teardown() in code coverage because Jest does not support it
99
/* istanbul ignore else */
1010
if (typeof afterEach === 'function') {
11-
afterEach(() => {
12-
cleanup();
11+
afterEach(async () => {
12+
await cleanup();
1313
})
1414
} else if (typeof teardown === 'function') {
1515
// Block is guarded by `typeof` check.
1616
// eslint does not support `typeof` guards.
1717
// eslint-disable-next-line no-undef
18-
teardown(() => {
19-
cleanup()
18+
teardown(async () => {
19+
await cleanup()
2020
})
2121
}
2222
}

‎src/pure.ts‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ async function render(
3030
let _isReadyResolved = false;
3131

3232
const execOutputAPI = {
33+
__exitCode: null as null | number,
3334
_isOutputAPI: true,
3435
_isReady: new Promise(
3536
(resolve, reject) => (_readyPromiseInternals = {resolve, reject}),
@@ -45,6 +46,9 @@ async function render(
4546
return this.stdoutArr.join('\n')
4647
},
4748
set stdoutStr(val: string) {},
49+
hasExit() {
50+
return this.__exitCode === null ? null : {exitCode: this.__exitCode};
51+
}
4852
}
4953

5054
mountedInstances.add(execOutputAPI as unknown as TestInstance)
@@ -100,6 +104,10 @@ async function render(
100104
}
101105
})
102106

107+
exec.on('exit', (code) => {
108+
execOutputAPI.__exitCode = code;
109+
})
110+
103111
// TODO: Replace with `debug()` function
104112
if (opts.debug) {
105113
exec.stdout.pipe(process.stdout)
@@ -123,13 +131,14 @@ async function render(
123131
}
124132

125133
function cleanup() {
126-
mountedInstances.forEach(cleanupAtInstance)
134+
return Promise.all([...mountedInstances].map(cleanupAtInstance))
127135
}
128136

129137
// maybe one day we'll expose this (perhaps even as a utility returned by render).
130138
// but let's wait until someone asks for it.
131-
function cleanupAtInstance(instance: TestInstance) {
132-
fireEvent.sigkill(instance)
139+
async function cleanupAtInstance(instance: TestInstance) {
140+
await fireEvent.sigkill(instance)
141+
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
133142
mountedInstances.delete(instance)
134143
}
135144

‎tests/setup-env.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import jestSnapshotSerializerAnsi from 'jest-snapshot-serializer-ansi'
2+
import {cleanup} from "../src";
23

34
expect.addSnapshotSerializer(jestSnapshotSerializerAnsi)
45
// add serializer for MutationRecord
@@ -35,7 +36,7 @@ beforeAll(() => {
3536
})
3637
})
3738

38-
afterEach(() => {
39+
afterEach(async () => {
3940
if (jest.isMockFunction(global.setTimeout)) {
4041
jest.useRealTimers()
4142
}

‎types/__tests__/type-tests.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ export function testBoundFunctions() {
4141
export async function eventTest() {
4242
const instance = await render('command', []);
4343

44-
fireEvent.sigterm(instance)
44+
await fireEvent.sigterm(instance)
4545

4646
fireEvent.write(instance, {value: 'test'});
4747
}

‎types/pure.d.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ export interface TestInstance {
1414
stderr: Readable
1515
stdoutStr: string
1616
pid: number | undefined
17+
hasExit(): null | {exitCode: number}
1718
}
1819

1920
export interface RenderOptions {

0 commit comments

Comments
 (0)