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.

Sign up for GitHub

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

Jump to bottom

Ensuring completion-listener.ts listen to a single close event emitte… #464

Merged
merged 11 commits into from Mar 24, 2024
Merged

Ensuring completion-listener.ts listen to a single close event emitte… #464

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
37156a9
Ensuring completion-listener.ts listen to a single close event emitte…
Feb 1, 2024
2eee2c2
Format.
Feb 1, 2024
5c9b16f
-Apply Latest.
Feb 25, 2024
f5dd736
Remove IPC support
Feb 25, 2024
1c03da7
Apply PR changes.
Feb 26, 2024
9c8edfb
Faked.
Feb 26, 2024
f81fa00
Exited
Feb 26, 2024
bfd2094
.
Feb 26, 2024
6992c26
Merge branch 'open-cli-tools:main' into main
odeadglaz Feb 27, 2024
c86d247
Check command state, instead of close event state
gustavohenke Mar 24, 2024
54818fb
Use emitFakeCloseEvent util more
gustavohenke Mar 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
268 changes: 155 additions & 113 deletions src/completion-listener.spec.ts
View file
Open in desktop
Expand Up @@ -24,186 +24,228 @@ const createController = (successCondition?: SuccessCondition) =>
const emitFakeCloseEvent = (command: FakeCommand, event?: Partial<CloseEvent>) =>
command.close.next(createFakeCloseEvent({ ...event, command, index: command.index }));

describe('with default success condition set', () => {
it('succeeds if all processes exited with code 0', () => {
const result = createController().listen(commands);
const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms));

describe('listen', () => {
it('check for success once all commands have emitted at least a single close event', async () => {
const finallyCallback = jest.fn();
const result = createController().listen(commands).finally(finallyCallback);

// Emitting multiple close events to mimic calling command `kill/start` APIs.
commands[0].close.next(createFakeCloseEvent({ exitCode: 0 }));
commands[0].close.next(createFakeCloseEvent({ exitCode: 0 }));
commands[0].close.next(createFakeCloseEvent({ exitCode: 0 }));

scheduler.flush();

await sleep(100);
expect(finallyCallback).not.toHaveBeenCalled();

commands[1].close.next(createFakeCloseEvent({ exitCode: 0 }));
commands[2].close.next(createFakeCloseEvent({ exitCode: 0 }));

scheduler.flush();

return expect(result).resolves.toEqual(expect.anything());
await expect(result).resolves.toEqual(expect.anything());

expect(finallyCallback).toHaveBeenCalled();
});

it('fails if one of the processes exited with non-0 code', () => {
it('Takes last event emitted from each command', async () => {
odeadglaz marked this conversation as resolved.
Show resolved Hide resolved
const result = createController().listen(commands);

commands[0].close.next(createFakeCloseEvent({ exitCode: 0 }));
commands[1].close.next(createFakeCloseEvent({ exitCode: 1 }));
commands[0].close.next(createFakeCloseEvent({ exitCode: 1 }));
commands[1].close.next(createFakeCloseEvent({ exitCode: 0 }));
commands[2].close.next(createFakeCloseEvent({ exitCode: 0 }));

scheduler.flush();

return expect(result).rejects.toEqual(expect.anything());
await expect(result).rejects.toEqual(expect.anything());
});
});

describe('with success condition set to first', () => {
it('succeeds if first process to exit has code 0', () => {
const result = createController('first').listen(commands);
describe('Detect commands exit conditions', () => {
describe('with default success condition set', () => {
it('succeeds if all processes exited with code 0', () => {
const result = createController().listen(commands);

commands[1].close.next(createFakeCloseEvent({ exitCode: 0 }));
commands[0].close.next(createFakeCloseEvent({ exitCode: 1 }));
commands[2].close.next(createFakeCloseEvent({ exitCode: 1 }));
commands[0].close.next(createFakeCloseEvent({ exitCode: 0 }));
commands[1].close.next(createFakeCloseEvent({ exitCode: 0 }));
commands[2].close.next(createFakeCloseEvent({ exitCode: 0 }));

scheduler.flush();
scheduler.flush();

return expect(result).resolves.toEqual(expect.anything());
});
return expect(result).resolves.toEqual(expect.anything());
});

it('fails if first process to exit has non-0 code', () => {
const result = createController('first').listen(commands);
it('fails if one of the processes exited with non-0 code', () => {
const result = createController().listen(commands);

commands[1].close.next(createFakeCloseEvent({ exitCode: 1 }));
commands[0].close.next(createFakeCloseEvent({ exitCode: 0 }));
commands[2].close.next(createFakeCloseEvent({ exitCode: 0 }));
commands[0].close.next(createFakeCloseEvent({ exitCode: 0 }));
commands[1].close.next(createFakeCloseEvent({ exitCode: 1 }));
commands[2].close.next(createFakeCloseEvent({ exitCode: 0 }));

scheduler.flush();
scheduler.flush();

return expect(result).rejects.toEqual(expect.anything());
return expect(result).rejects.toEqual(expect.anything());
});
});
});

describe('with success condition set to last', () => {
it('succeeds if last process to exit has code 0', () => {
const result = createController('last').listen(commands);
describe('with success condition set to first', () => {
it('succeeds if first process to exit has code 0', () => {
const result = createController('first').listen(commands);

commands[1].close.next(createFakeCloseEvent({ exitCode: 1 }));
commands[0].close.next(createFakeCloseEvent({ exitCode: 0 }));
commands[2].close.next(createFakeCloseEvent({ exitCode: 0 }));
commands[1].close.next(createFakeCloseEvent({ exitCode: 0 }));
commands[0].close.next(createFakeCloseEvent({ exitCode: 1 }));
commands[2].close.next(createFakeCloseEvent({ exitCode: 1 }));

scheduler.flush();
scheduler.flush();

return expect(result).resolves.toEqual(expect.anything());
});
return expect(result).resolves.toEqual(expect.anything());
});

it('fails if last process to exit has non-0 code', () => {
const result = createController('last').listen(commands);
it('fails if first process to exit has non-0 code', () => {
const result = createController('first').listen(commands);

commands[1].close.next(createFakeCloseEvent({ exitCode: 0 }));
commands[0].close.next(createFakeCloseEvent({ exitCode: 1 }));
commands[2].close.next(createFakeCloseEvent({ exitCode: 1 }));
commands[1].close.next(createFakeCloseEvent({ exitCode: 1 }));
commands[0].close.next(createFakeCloseEvent({ exitCode: 0 }));
commands[2].close.next(createFakeCloseEvent({ exitCode: 0 }));

scheduler.flush();
scheduler.flush();

return expect(result).rejects.toEqual(expect.anything());
return expect(result).rejects.toEqual(expect.anything());
});
});
});

describe.each([
// Use the middle command for both cases to make it more difficult to make a mess up
// in the implementation cause false passes.
['command-bar' as const, 'bar'],
['command-1' as const, 1],
])('with success condition set to %s', (condition, nameOrIndex) => {
it(`succeeds if command ${nameOrIndex} exits with code 0`, () => {
const result = createController(condition).listen(commands);
describe('with success condition set to last', () => {
it('succeeds if last process to exit has code 0', () => {
const result = createController('last').listen(commands);

emitFakeCloseEvent(commands[0], { exitCode: 1 });
emitFakeCloseEvent(commands[1], { exitCode: 0 });
emitFakeCloseEvent(commands[2], { exitCode: 1 });
commands[1].close.next(createFakeCloseEvent({ exitCode: 1 }));
commands[0].close.next(createFakeCloseEvent({ exitCode: 0 }));
commands[2].close.next(createFakeCloseEvent({ exitCode: 0 }));

scheduler.flush();
scheduler.flush();

return expect(result).resolves.toEqual(expect.anything());
});
return expect(result).resolves.toEqual(expect.anything());
});

it(`succeeds if all commands ${nameOrIndex} exit with code 0`, () => {
commands = [commands[0], commands[1], commands[1]];
const result = createController(condition).listen(commands);
it('fails if last process to exit has non-0 code', () => {
const result = createController('last').listen(commands);

emitFakeCloseEvent(commands[0], { exitCode: 1 });
emitFakeCloseEvent(commands[1], { exitCode: 0 });
emitFakeCloseEvent(commands[2], { exitCode: 0 });
commands[1].close.next(createFakeCloseEvent({ exitCode: 0 }));
commands[0].close.next(createFakeCloseEvent({ exitCode: 1 }));
commands[2].close.next(createFakeCloseEvent({ exitCode: 1 }));

scheduler.flush();
scheduler.flush();

return expect(result).resolves.toEqual(expect.anything());
return expect(result).rejects.toEqual(expect.anything());
});
});

it(`fails if command ${nameOrIndex} exits with non-0 code`, () => {
const result = createController(condition).listen(commands);
describe.each([
// Use the middle command for both cases to make it more difficult to make a mess up
// in the implementation cause false passes.
['command-bar' as const, 'bar'],
['command-1' as const, 1],
])('with success condition set to %s', (condition, nameOrIndex) => {
it(`succeeds if command ${nameOrIndex} exits with code 0`, () => {
const result = createController(condition).listen(commands);

emitFakeCloseEvent(commands[0], { exitCode: 0 });
emitFakeCloseEvent(commands[1], { exitCode: 1 });
emitFakeCloseEvent(commands[2], { exitCode: 0 });
emitFakeCloseEvent(commands[0], { exitCode: 1 });
emitFakeCloseEvent(commands[1], { exitCode: 0 });
emitFakeCloseEvent(commands[2], { exitCode: 1 });

scheduler.flush();
scheduler.flush();

return expect(result).rejects.toEqual(expect.anything());
});
return expect(result).resolves.toEqual(expect.anything());
});

it(`fails if some commands ${nameOrIndex} exit with non-0 code`, () => {
commands = [commands[0], commands[1], commands[1]];
const result = createController(condition).listen(commands);
it(`succeeds if all commands ${nameOrIndex} exit with code 0`, () => {
commands = [commands[0], commands[1], commands[1]];
const result = createController(condition).listen(commands);

emitFakeCloseEvent(commands[0], { exitCode: 1 });
emitFakeCloseEvent(commands[1], { exitCode: 0 });
emitFakeCloseEvent(commands[2], { exitCode: 1 });
emitFakeCloseEvent(commands[0], { exitCode: 1 });
emitFakeCloseEvent(commands[1], { exitCode: 0 });
emitFakeCloseEvent(commands[2], { exitCode: 0 });

scheduler.flush();
scheduler.flush();

return expect(result).resolves.toEqual(expect.anything());
});
return expect(result).resolves.toEqual(expect.anything());
});

it(`fails if command ${nameOrIndex} doesn't exist`, () => {
const result = createController(condition).listen([commands[0]]);
it(`fails if command ${nameOrIndex} exits with non-0 code`, () => {
const result = createController(condition).listen(commands);

emitFakeCloseEvent(commands[0], { exitCode: 0 });
scheduler.flush();
emitFakeCloseEvent(commands[0], { exitCode: 0 });
emitFakeCloseEvent(commands[1], { exitCode: 1 });
emitFakeCloseEvent(commands[2], { exitCode: 0 });

return expect(result).rejects.toEqual(expect.anything());
});
});
scheduler.flush();

describe.each([
// Use the middle command for both cases to make it more difficult to make a mess up
// in the implementation cause false passes.
['!command-bar' as const, 'bar'],
['!command-1' as const, 1],
])('with success condition set to %s', (condition, nameOrIndex) => {
it(`succeeds if all commands but ${nameOrIndex} exit with code 0`, () => {
const result = createController(condition).listen(commands);
return expect(result).rejects.toEqual(expect.anything());
});

emitFakeCloseEvent(commands[0], { exitCode: 0 });
emitFakeCloseEvent(commands[1], { exitCode: 1 });
emitFakeCloseEvent(commands[2], { exitCode: 0 });
it(`fails if some commands ${nameOrIndex} exit with non-0 code`, () => {
const result = createController(condition).listen(commands);

scheduler.flush();
emitFakeCloseEvent(commands[0], { exitCode: 1 });
emitFakeCloseEvent(commands[1], { exitCode: 0 });
emitFakeCloseEvent(commands[2], { exitCode: 1 });

return expect(result).resolves.toEqual(expect.anything());
});
scheduler.flush();

it(`fails if any commands but ${nameOrIndex} exit with non-0 code`, () => {
const result = createController(condition).listen(commands);
return expect(result).resolves.toEqual(expect.anything());
});

emitFakeCloseEvent(commands[0], { exitCode: 1 });
emitFakeCloseEvent(commands[1], { exitCode: 1 });
emitFakeCloseEvent(commands[2], { exitCode: 0 });
it(`fails if command ${nameOrIndex} doesn't exist`, () => {
const result = createController(condition).listen([commands[0]]);

scheduler.flush();
emitFakeCloseEvent(commands[0], { exitCode: 0 });
scheduler.flush();

return expect(result).rejects.toEqual(expect.anything());
return expect(result).rejects.toEqual(expect.anything());
});
});

it(`succeeds if command ${nameOrIndex} doesn't exist`, () => {
const result = createController(condition).listen([commands[0]]);
describe.each([
// Use the middle command for both cases to make it more difficult to make a mess up
// in the implementation cause false passes.
['!command-bar' as const, 'bar'],
['!command-1' as const, 1],
])('with success condition set to %s', (condition, nameOrIndex) => {
it(`succeeds if all commands but ${nameOrIndex} exit with code 0`, () => {
const result = createController(condition).listen(commands);

emitFakeCloseEvent(commands[0], { exitCode: 0 });
scheduler.flush();
emitFakeCloseEvent(commands[0], { exitCode: 0 });
emitFakeCloseEvent(commands[1], { exitCode: 1 });
emitFakeCloseEvent(commands[2], { exitCode: 0 });

scheduler.flush();

return expect(result).resolves.toEqual(expect.anything());
});

it(`fails if any commands but ${nameOrIndex} exit with non-0 code`, () => {
const result = createController(condition).listen(commands);

emitFakeCloseEvent(commands[0], { exitCode: 1 });
emitFakeCloseEvent(commands[1], { exitCode: 1 });
emitFakeCloseEvent(commands[2], { exitCode: 0 });

scheduler.flush();

return expect(result).rejects.toEqual(expect.anything());
});

it(`succeeds if command ${nameOrIndex} doesn't exist`, () => {
const result = createController(condition).listen([commands[0]]);

emitFakeCloseEvent(commands[0], { exitCode: 0 });
scheduler.flush();

return expect(result).resolves.toEqual(expect.anything());
return expect(result).resolves.toEqual(expect.anything());
});
});
});
20 changes: 14 additions & 6 deletions src/completion-listener.ts
View file
Open in desktop
@@ -1,5 +1,5 @@
import * as Rx from 'rxjs';
import { bufferCount, switchMap, take } from 'rxjs/operators';
import { filter, map, switchMap, take } from 'rxjs/operators';

import { CloseEvent, Command } from './command';

Expand Down Expand Up @@ -68,12 +68,12 @@ export class CompletionListener {
({ command, index }) => command.name === nameOrIndex || index === Number(nameOrIndex),
);
if (this.successCondition.startsWith('!')) {
// All commands except the specified ones must exit succesfully
// All commands except the specified ones must exit successfully
return events.every(
(event) => targetCommandsEvents.includes(event) || event.exitCode === 0,
);
}
// Only the specified commands must exit succesfully
// Only the specified commands must exit successfully
return (
targetCommandsEvents.length > 0 &&
targetCommandsEvents.every((event) => event.exitCode === 0)
Expand All @@ -86,10 +86,18 @@ export class CompletionListener {
* @returns A Promise that resolves if the success condition is met, or rejects otherwise.
*/
listen(commands: Command[]): Promise<CloseEvent[]> {
const closeStreams = commands.map((command) => command.close);
const closeStreams = commands.map((command) => command.close.pipe());
odeadglaz marked this conversation as resolved.
Show resolved Hide resolved

return Rx.lastValueFrom(
Rx.merge(...closeStreams).pipe(
bufferCount(closeStreams.length),
Rx.combineLatest(closeStreams).pipe(
filter((exitInfos) => exitInfos.every((exitInfo) => exitInfo.exitCode !== null)),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exitCode is never null here. You need to look at the event's command for the current state

Suggested change
filter((exitInfos) => exitInfos.every((exitInfo) => exitInfo.exitCode !== null)),
filter((exitInfos) => exitInfos.every(({ command }) => command.state !== 'started')),

Copy link
Contributor Author

@odeadglaz odeadglaz Feb 26, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea good point 👍 This means to access it we would need to either add it to the CommandInfo which I assume we don't want as it's a public interface or send it explicitly right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heya, I've fixed this myself and added an extra test too.

You were right, adding to CommandInfo is not a good idea, but so wasn't right to pass the state in the close event since that's pretty static. Can check commands variable just outside the loop, though.

map((exitInfos) =>
exitInfos.sort((first, second) => {
return (
first.timings.startDate.getTime() - second.timings.startDate.getTime()
odeadglaz marked this conversation as resolved.
Show resolved Hide resolved
);
}),
),
switchMap((exitInfos) =>
this.isSuccess(exitInfos)
? this.emitWithScheduler(Rx.of(exitInfos))
Expand Down