-
Notifications
You must be signed in to change notification settings - Fork 0
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
Execute iterator exploration #1
Changes from all commits
7107dd1
7fd6664
4569668
0e800f0
946af54
7683a4f
49e2303
2e4bb41
693eae5
1fa76be
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,7 @@ import { | |
describe('NodeExecuteBuilder', () => { | ||
let testOptions: NodeExecuteBuilderOptions; | ||
let context: ExecutorContext; | ||
let mockSubProcess: { on: jest.Mock; exitCode: number }; | ||
|
||
beforeEach(async () => { | ||
buildOptions = {}; | ||
|
@@ -34,13 +35,16 @@ describe('NodeExecuteBuilder', () => { | |
); | ||
|
||
fork.mockImplementation(() => { | ||
return { | ||
on: (eventName, cb) => { | ||
mockSubProcess = { | ||
on: jest.fn().mockImplementation((eventName, cb) => { | ||
if (eventName === 'exit') { | ||
cb(); | ||
mockSubProcess.exitCode = 0; | ||
cb(mockSubProcess.exitCode); | ||
} | ||
}, | ||
}), | ||
exitCode: null, | ||
}; | ||
return mockSubProcess; | ||
}); | ||
|
||
treeKill.mockImplementation((pid, signal, callback) => { | ||
|
@@ -74,6 +78,7 @@ describe('NodeExecuteBuilder', () => { | |
waitUntilTargets: [], | ||
host: 'localhost', | ||
watch: true, | ||
emitSubprocessEvents: false, | ||
}; | ||
}); | ||
|
||
|
@@ -83,6 +88,7 @@ describe('NodeExecuteBuilder', () => { | |
|
||
it('should build the application and start the built file', async () => { | ||
for await (const event of executeExecutor(testOptions, context)) { | ||
if (!('success' in event)) throw new Error('Got Unexpected event'); | ||
expect(event.success).toEqual(true); | ||
} | ||
expect(require('@nrwl/devkit').runExecutor).toHaveBeenCalledWith( | ||
|
@@ -105,7 +111,6 @@ describe('NodeExecuteBuilder', () => { | |
expect(treeKill).toHaveBeenCalledTimes(0); | ||
expect(fork).toHaveBeenCalledTimes(1); | ||
}); | ||
|
||
describe('--inspect', () => { | ||
describe('inspect', () => { | ||
it('should inspect the process', async () => { | ||
|
@@ -215,6 +220,10 @@ describe('NodeExecuteBuilder', () => { | |
}); | ||
|
||
it('should log errors from killing the process', async () => { | ||
(devkit.runExecutor as any).mockImplementation(function* () { | ||
yield { success: true, outfile: 'outfile.js' }; | ||
yield { success: true, outfile: 'outfile.js' }; | ||
}); | ||
Comment on lines
+223
to
+226
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The old test cases for this spec and the one below were non-deterministic: The state of With that subProcess state encapsulated, there was no way to make this test pass without altering it. |
||
treeKill.mockImplementation((pid, signal, callback) => { | ||
callback(new Error('Error Message')); | ||
}); | ||
|
@@ -227,6 +236,10 @@ describe('NodeExecuteBuilder', () => { | |
}); | ||
|
||
it('should log errors from killing the process on windows', async () => { | ||
(devkit.runExecutor as any).mockImplementation(function* () { | ||
yield { success: true, outfile: 'outfile.js' }; | ||
yield { success: true, outfile: 'outfile.js' }; | ||
}); | ||
treeKill.mockImplementation((pid, signal, callback) => { | ||
callback([new Error('error'), '', 'Error Message']); | ||
}); | ||
|
@@ -329,4 +342,230 @@ describe('NodeExecuteBuilder', () => { | |
} | ||
}); | ||
}); | ||
|
||
describe('--watch', () => { | ||
let mockSubProcessExit; | ||
let mockSubProcessMessage; | ||
let mockSubProcessError; | ||
|
||
beforeEach(() => { | ||
fork.mockImplementation(() => { | ||
mockSubProcess = { | ||
on: jest.fn().mockImplementation((eventName, cb) => { | ||
if (eventName === 'exit') { | ||
mockSubProcessExit = (exitCode: number) => { | ||
cb(exitCode); | ||
mockSubProcess.exitCode = exitCode; | ||
}; | ||
} | ||
|
||
if (eventName === 'error') { | ||
mockSubProcessError = cb; | ||
} | ||
|
||
if (eventName === 'message') { | ||
mockSubProcessMessage = cb; | ||
} | ||
}), | ||
exitCode: null, | ||
}; | ||
|
||
return mockSubProcess; | ||
}); | ||
}); | ||
|
||
describe('false', () => { | ||
it('Should not complete until the child process has exited', async () => { | ||
const firstEvent = Symbol('first'); | ||
const secondEvent = Symbol('second'); | ||
const events = []; | ||
const executorIterator = executeExecutor( | ||
{ | ||
...testOptions, | ||
watch: false, | ||
}, | ||
context | ||
); | ||
|
||
expect(await executorIterator.next()).toEqual({ | ||
done: false, | ||
value: { | ||
outfile: 'outfile.js', | ||
success: true, | ||
}, | ||
}); | ||
|
||
const result = await Promise.race([ | ||
executorIterator.next(), | ||
new Promise(() => { | ||
mockSubProcessExit(1); | ||
events.push(firstEvent); | ||
}), | ||
]); | ||
|
||
events.push(secondEvent); | ||
|
||
expect(result).toEqual({ | ||
value: undefined, | ||
done: true, | ||
}); | ||
expect(events).toEqual([firstEvent, secondEvent]); | ||
}); | ||
|
||
it('When sub process emits, it should propagate when configured', async () => { | ||
const expectedExitCode = 42; | ||
const executorIterator = executeExecutor( | ||
{ | ||
...testOptions, | ||
watch: false, | ||
emitSubprocessEvents: true, | ||
}, | ||
context | ||
); | ||
|
||
expect(await executorIterator.next()).toEqual({ | ||
done: false, | ||
value: { | ||
outfile: 'outfile.js', | ||
success: true, | ||
}, | ||
}); | ||
|
||
mockSubProcessExit(expectedExitCode); | ||
|
||
expect(await executorIterator.next()).toEqual({ | ||
done: false, | ||
value: { | ||
exitCode: expectedExitCode, | ||
}, | ||
}); | ||
}); | ||
|
||
it('Should propagate sub process events in expected order', async () => { | ||
const expectedSubProcessEvents = [ | ||
{ | ||
message: 'hello', | ||
}, | ||
{ | ||
message: 'world', | ||
}, | ||
{ | ||
error: new Error('error'), | ||
}, | ||
{ | ||
exitCode: 1, | ||
}, | ||
]; | ||
const expectedTotalEvents = 5; | ||
|
||
let subProcessEvents = []; | ||
let totalEvents = 0; | ||
|
||
const doSubProcessWork = () => { | ||
mockSubProcessMessage('hello'); | ||
mockSubProcessMessage('world'); | ||
mockSubProcessError(new Error('error')); | ||
mockSubProcessExit(1); | ||
}; | ||
|
||
for await (const event of executeExecutor( | ||
{ | ||
...testOptions, | ||
watch: false, | ||
emitSubprocessEvents: true, | ||
}, | ||
context | ||
)) { | ||
if (totalEvents === 0) { | ||
expect(event).toEqual({ | ||
outfile: 'outfile.js', | ||
success: true, | ||
}); | ||
doSubProcessWork(); | ||
} else { | ||
subProcessEvents.push(event); | ||
} | ||
|
||
totalEvents += 1; | ||
} | ||
|
||
expect(subProcessEvents).toEqual(expectedSubProcessEvents); | ||
expect(totalEvents).toEqual(expectedTotalEvents); | ||
}); | ||
}); | ||
|
||
describe('true', () => { | ||
beforeEach(() => { | ||
(devkit.runExecutor as any).mockImplementation(async function* () { | ||
yield { success: true, outfile: 'outfile.js' }; | ||
await new Promise((resolve) => { | ||
setTimeout(resolve, 100); | ||
}); | ||
yield { success: true, outfile: 'outfile.js' }; | ||
}); | ||
}); | ||
|
||
it('Should not block build events with process events', async () => { | ||
const executorIterator = executeExecutor( | ||
{ | ||
...testOptions, | ||
watch: true, | ||
}, | ||
context | ||
); | ||
|
||
expect(await executorIterator.next()).toEqual({ | ||
done: false, | ||
value: { | ||
outfile: 'outfile.js', | ||
success: true, | ||
}, | ||
}); | ||
|
||
// sub process has not exited yet, but we'll receive a new build event | ||
|
||
expect(await executorIterator.next()).toEqual({ | ||
done: false, | ||
value: { | ||
outfile: 'outfile.js', | ||
success: true, | ||
}, | ||
}); | ||
|
||
// there's no more build events, but process still needs to exit | ||
|
||
const result = await Promise.race([ | ||
executorIterator.next(), | ||
new Promise(() => { | ||
mockSubProcessExit(); | ||
}), | ||
]); | ||
|
||
expect(result).toEqual({ | ||
value: undefined, | ||
done: true, | ||
}); | ||
}); | ||
|
||
it('Yield build and process events cooperatively', async () => { | ||
const events = []; | ||
for await (const event of executeExecutor( | ||
{ | ||
...testOptions, | ||
watch: true, | ||
emitSubprocessEvents: true, | ||
}, | ||
context | ||
)) { | ||
if ('outfile' in event) { | ||
mockSubProcessExit(1); | ||
} | ||
|
||
events.push(event); | ||
} | ||
|
||
expect(events).toHaveLength(4); | ||
}); | ||
}); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Represents a breaking change: the type of event yielded by
executeExecutor
is now https://github.com/dpchamps/nx/pull/1/files#diff-8b1daad050dbc745646b89f91cd20c497a23f51caf7eeaddb96a517a80b8f01fR28