From 5b23fbafd3503c2e7c8a8b0b9dd618ed8e53c1fa Mon Sep 17 00:00:00 2001 From: Alexander Marks Date: Sun, 23 Oct 2022 17:32:34 -0700 Subject: [PATCH 1/4] Fix bug: missing await was causing finally to run too soon --- src/execution/standard.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/execution/standard.ts b/src/execution/standard.ts index 04904d1af..f73e6939c 100644 --- a/src/execution/standard.ts +++ b/src/execution/standard.ts @@ -75,7 +75,7 @@ export class StandardScriptExecution extends BaseExecutionWithCommand { + return await this._acquireSystemLockIfNeeded(async () => { // Note we must wait for dependencies to finish before generating the // cache key, because a dependency could create or modify an input file to // this script, which would affect the key. From a29ebd38555f104e1c1a72ff8225b07ea30b59b2 Mon Sep 17 00:00:00 2001 From: Alexander Marks Date: Sun, 23 Oct 2022 17:28:12 -0700 Subject: [PATCH 2/4] Add a way to check if a test rig command is currently running --- src/test/util/test-rig-command.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/test/util/test-rig-command.ts b/src/test/util/test-rig-command.ts index 30d6ae1b2..7b0d883d3 100644 --- a/src/test/util/test-rig-command.ts +++ b/src/test/util/test-rig-command.ts @@ -194,6 +194,13 @@ export class WireitTestRigCommandInvocation extends IpcClient< return this._closed.promise; } + /** + * Return whether this invocation is still running. + */ + get running(): boolean { + return this._state === 'connected'; + } + /** * Tell this invocation to exit with the given code. */ From 5a4e5cd27ffcd85ec3c1a5be5b357f76372e5b5d Mon Sep 17 00:00:00 2001 From: Alexander Marks Date: Sun, 23 Oct 2022 17:28:30 -0700 Subject: [PATCH 3/4] Implement and test simple consumer -> service case --- src/execution/service.ts | 176 ++++++++++++++++++++++++++++++++++++--- src/test/service.test.ts | 54 ++++++++++++ 2 files changed, 219 insertions(+), 11 deletions(-) diff --git a/src/execution/service.ts b/src/execution/service.ts index 2fdf3bc83..6068e5252 100644 --- a/src/execution/service.ts +++ b/src/execution/service.ts @@ -7,6 +7,7 @@ import {BaseExecutionWithCommand} from './base.js'; import {Fingerprint} from '../fingerprint.js'; import {Deferred} from '../util/deferred.js'; +import {ScriptChildProcess} from '../script-child-process.js'; import type {ExecutionResult} from './base.js'; import type {ServiceScriptConfig} from '../config.js'; @@ -15,6 +16,32 @@ import type {Logger} from '../logging/logger.js'; import type {Failure} from '../event.js'; import type {Result} from '../error.js'; +type ServiceState = + | {id: 'initial'} + | {id: 'fingerprinting'} + | {id: 'unstarted'} + | { + id: 'starting'; + child: ScriptChildProcess; + started: Deferred>; + } + | { + id: 'started'; + child: ScriptChildProcess; + } + | {id: 'stopping'} + | {id: 'stopped'}; + +function unknownState(state: never) { + return new Error( + `Unknown service state ${String((state as ServiceState).id)}` + ); +} + +function unexpectedState(state: ServiceState) { + return new Error(`Unexpected service state ${state.id}`); +} + /** * Execution for a {@link ServiceScriptConfig}. * @@ -82,6 +109,7 @@ import type {Result} from '../error.js'; * ``` */ export class ServiceScriptExecution extends BaseExecutionWithCommand { + private _state: ServiceState = {id: 'initial'}; private readonly _terminated = new Deferred>(); /** @@ -109,22 +137,148 @@ export class ServiceScriptExecution extends BaseExecutionWithCommand { - const dependencyFingerprints = await this._executeDependencies(); - if (!dependencyFingerprints.ok) { - return dependencyFingerprints; + switch (this._state.id) { + case 'initial': { + this._state = {id: 'fingerprinting'}; + const dependencyFingerprints = await this._executeDependencies(); + if (!dependencyFingerprints.ok) { + return dependencyFingerprints; + } + const fingerprint = await Fingerprint.compute( + this._config, + dependencyFingerprints.value + ); + this._state = {id: 'unstarted'}; + return {ok: true, value: fingerprint}; + } + case 'fingerprinting': + case 'unstarted': + case 'starting': + case 'started': + case 'stopping': + case 'stopped': { + throw unexpectedState(this._state); + } + default: { + throw unknownState(this._state); + } } - const fingerprint = await Fingerprint.compute( - this._config, - dependencyFingerprints.value - ); - return {ok: true, value: fingerprint}; } /** * Start this service if it isn't already started. */ - start(): Promise> { - // TODO(aomarks) Implement service starting/stopping. - throw new Error('Not implemented'); + async start(): Promise> { + switch (this._state.id) { + case 'unstarted': { + this._state = { + id: 'starting', + child: new ScriptChildProcess(this._config), + started: new Deferred(), + }; + void this._state.child.started.then(() => { + this._onChildStarted(); + }); + void this._state.child.completed.then(() => { + this._onChildExited(); + }); + return this._state.started.promise; + } + case 'initial': + case 'fingerprinting': + case 'starting': + case 'started': + case 'stopping': + case 'stopped': { + throw unexpectedState(this._state); + } + default: { + throw unknownState(this._state); + } + } + } + + private _onChildStarted() { + switch (this._state.id) { + case 'starting': { + this._state.started.resolve({ok: true, value: undefined}); + this._logger.log({ + script: this._config, + type: 'info', + detail: 'service-started', + }); + this._state = { + id: 'started', + child: this._state.child, + }; + const allConsumersDone = Promise.all( + this._config.serviceConsumers.map( + (consumer) => + this._executor.getExecution(consumer).servicesNotNeeded + ) + ); + void allConsumersDone.then(() => { + this._allConsumersDone(); + }); + return; + } + case 'initial': + case 'fingerprinting': + case 'unstarted': + case 'started': + case 'stopping': + case 'stopped': { + throw unexpectedState(this._state); + } + default: { + throw unknownState(this._state); + } + } + } + + private _allConsumersDone() { + switch (this._state.id) { + case 'started': { + this._state.child.kill(); + this._state = {id: 'stopping'}; + return; + } + case 'initial': + case 'fingerprinting': + case 'unstarted': + case 'starting': + case 'stopping': + case 'stopped': { + throw unexpectedState(this._state); + } + default: { + throw unknownState(this._state); + } + } + } + + private _onChildExited() { + switch (this._state.id) { + case 'stopping': { + this._state = {id: 'stopped'}; + this._logger.log({ + script: this._config, + type: 'info', + detail: 'service-stopped', + }); + return; + } + case 'initial': + case 'fingerprinting': + case 'unstarted': + case 'starting': + case 'started': + case 'stopped': { + throw unexpectedState(this._state); + } + default: { + throw unknownState(this._state); + } + } } } diff --git a/src/test/service.test.ts b/src/test/service.test.ts index fb42dd613..2e9472337 100644 --- a/src/test/service.test.ts +++ b/src/test/service.test.ts @@ -5,6 +5,8 @@ */ import {suite} from 'uvu'; +import * as assert from 'uvu/assert'; +import {timeout} from './util/uvu-timeout.js'; import {WireitTestRig} from './util/test-rig.js'; const test = suite<{rig: WireitTestRig}>(); @@ -32,4 +34,56 @@ test.after.each(async (ctx) => { } }); +test( + 'simple consumer and service', + timeout(async ({rig}) => { + // consumer + // | + // v + // service + + const consumer = await rig.newCommand(); + const service = await rig.newCommand(); + await rig.writeAtomic({ + 'package.json': { + scripts: { + consumer: 'wireit', + service: 'wireit', + }, + wireit: { + consumer: { + command: consumer.command, + dependencies: ['service'], + }, + service: { + command: service.command, + service: true, + }, + }, + }, + }); + + const wireit = rig.exec('npm run consumer'); + + // The service starts because the consumer depends on it + const serviceInv = await service.nextInvocation(); + await wireit.waitForLog(/Service started/); + + // The consumer starts and finishes + const consumerInv = await consumer.nextInvocation(); + // Wait a moment to ensure the service stays running + await new Promise((resolve) => setTimeout(resolve, 100)); + assert.ok(serviceInv.running); + consumerInv.exit(0); + + // The service stops because the consumer is done + await serviceInv.closed; + await wireit.waitForLog(/Service stopped/); + + await wireit.exit; + assert.equal(service.numInvocations, 1); + assert.equal(consumer.numInvocations, 1); + }) +); + test.run(); From 76abfff461c71ea617cb70a69a6f810e17f40211 Mon Sep 17 00:00:00 2001 From: Alexander Marks Date: Mon, 24 Oct 2022 09:11:36 -0700 Subject: [PATCH 4/4] Address review feedback --- src/execution/service.ts | 123 ++++++++++++++++++++++++++---- src/test/service.test.ts | 2 +- src/test/util/test-rig-command.ts | 2 +- 3 files changed, 110 insertions(+), 17 deletions(-) diff --git a/src/execution/service.ts b/src/execution/service.ts index 6068e5252..1cf5924de 100644 --- a/src/execution/service.ts +++ b/src/execution/service.ts @@ -10,7 +10,7 @@ import {Deferred} from '../util/deferred.js'; import {ScriptChildProcess} from '../script-child-process.js'; import type {ExecutionResult} from './base.js'; -import type {ServiceScriptConfig} from '../config.js'; +import type {ScriptReference, ServiceScriptConfig} from '../config.js'; import type {Executor} from '../executor.js'; import type {Logger} from '../logging/logger.js'; import type {Failure} from '../event.js'; @@ -18,7 +18,14 @@ import type {Result} from '../error.js'; type ServiceState = | {id: 'initial'} - | {id: 'fingerprinting'} + | { + id: 'executingDeps'; + fingerprint: Deferred; + } + | { + id: 'fingerprinting'; + fingerprint: Deferred; + } | {id: 'unstarted'} | { id: 'starting'; @@ -60,7 +67,13 @@ function unexpectedState(state: ServiceState) { * ▼ execute * │ │ * │ ┌───────▼────────┐ - * ├─◄─ abort ─┤ FINGERPRINTING ├──── depExecErr ────►───╮ + * ├─◄─ abort ─┤ EXECUTING_DEPS ├──── depExecErr ────►───╮ + * │ └───────┬────────┘ │ + * │ │ │ + * ▼ depsExecuted │ + * │ │ │ + * │ ┌───────▼────────┐ │ + * ├─◄─ abort ─┤ FINGERPRINTING │ │ * │ └───────┬────────┘ │ * │ │ │ * ▼ fingerprinted │ @@ -136,21 +149,54 @@ export class ServiceScriptExecution extends BaseExecutionWithCommand { + protected override _execute(): Promise { switch (this._state.id) { case 'initial': { - this._state = {id: 'fingerprinting'}; - const dependencyFingerprints = await this._executeDependencies(); - if (!dependencyFingerprints.ok) { - return dependencyFingerprints; - } - const fingerprint = await Fingerprint.compute( - this._config, - dependencyFingerprints.value + this._state = { + id: 'executingDeps', + fingerprint: new Deferred(), + }; + void this._executeDependencies().then((result) => { + if (result.ok) { + this._onDepsExecuted(result.value); + } else { + this._onDepExecErr(result); + } + }); + return this._state.fingerprint.promise; + } + case 'executingDeps': + case 'fingerprinting': + case 'unstarted': + case 'starting': + case 'started': + case 'stopping': + case 'stopped': { + throw unexpectedState(this._state); + } + default: { + throw unknownState(this._state); + } + } + } + + private _onDepsExecuted( + depFingerprints: Array<[ScriptReference, Fingerprint]> + ): void { + switch (this._state.id) { + case 'executingDeps': { + this._state = { + id: 'fingerprinting', + fingerprint: this._state.fingerprint, + }; + void Fingerprint.compute(this._config, depFingerprints).then( + (result) => { + this._onFingerprinted(result); + } ); - this._state = {id: 'unstarted'}; - return {ok: true, value: fingerprint}; + return; } + case 'initial': case 'fingerprinting': case 'unstarted': case 'starting': @@ -165,10 +211,53 @@ export class ServiceScriptExecution extends BaseExecutionWithCommand> { + start(): Promise> { switch (this._state.id) { case 'unstarted': { this._state = { @@ -185,6 +274,7 @@ export class ServiceScriptExecution extends BaseExecutionWithCommand setTimeout(resolve, 100)); - assert.ok(serviceInv.running); + assert.ok(serviceInv.isRunning); consumerInv.exit(0); // The service stops because the consumer is done diff --git a/src/test/util/test-rig-command.ts b/src/test/util/test-rig-command.ts index 7b0d883d3..764c8bbd4 100644 --- a/src/test/util/test-rig-command.ts +++ b/src/test/util/test-rig-command.ts @@ -197,7 +197,7 @@ export class WireitTestRigCommandInvocation extends IpcClient< /** * Return whether this invocation is still running. */ - get running(): boolean { + get isRunning(): boolean { return this._state === 'connected'; }