diff --git a/lib/Compile.js b/lib/Compile.js index 5ff7ccbd..3f78956b 100644 --- a/lib/Compile.js +++ b/lib/Compile.js @@ -3,6 +3,15 @@ const ElmCompiler = require('./ElmCompiler'); const Report = require('./Report'); const Spawn = require('./Spawn'); +class ElmMakeError extends Error { + /** + * @param { number | null } exitCode + */ + constructor(exitCode) { + super(`\`elm make\` failed with exit code ${exitCode}.`); + } +} + /** * @param { string } cwd * @param { string } testFile @@ -22,7 +31,7 @@ function compile(cwd, testFile, dest, pathToElmBinary, report) { compileProcess.on('close', function (exitCode) { if (exitCode !== 0) { - reject(new Error(`\`elm make\` failed with exit code ${exitCode}.`)); + reject(new ElmMakeError(exitCode)); } else { resolve(); } @@ -57,7 +66,7 @@ function compileSources( if (exitCode === 0) { resolve(); } else { - reject(new Error(`\`elm make\` failed with exit code ${exitCode}.`)); + reject(new ElmMakeError(exitCode)); } }); }); @@ -119,4 +128,5 @@ function processOptsForReporter(report) { module.exports = { compile, compileSources, + ElmMakeError, }; diff --git a/lib/DependencyProvider.js b/lib/DependencyProvider.js index a0954e63..de35ea76 100644 --- a/lib/DependencyProvider.js +++ b/lib/DependencyProvider.js @@ -2,30 +2,23 @@ const fs = require('fs'); const path = require('path'); const wasm = require('elm-solve-deps-wasm'); const ElmHome = require('./ElmHome.js'); -const SyncGet = require('./SyncGet.js'); const collator = new Intl.Collator('en', { numeric: true }); // for sorting SemVer strings // Initialization work done only once. wasm.init(); -// Lazily start the worker until needed. -// This is important for the tests, which never exit otherwise. -/** @type { undefined | import('./SyncGet').SyncGetWorker } */ -let syncGetWorker_ = undefined; -/** - * @returns { import('./SyncGet').SyncGetWorker } - */ -function syncGetWorker() { - if (syncGetWorker_ === undefined) { - syncGetWorker_ = SyncGet.startWorker(); - } - return syncGetWorker_; -} // Cache of existing versions according to the package website. class OnlineVersionsCache { /** @type { Map> } */ map = new Map(); + /** + * @param { import('./SyncGet').SyncHttpGet } syncHttpGet + */ + constructor(syncHttpGet) { + this.syncHttpGet = syncHttpGet; + } + /** * @returns { void } */ @@ -41,7 +34,11 @@ class OnlineVersionsCache { cacheFile = fs.readFileSync(cachePath, 'utf8'); } catch (_) { // The cache file does not exist so let's reset it. - this.map = onlineVersionsFromScratch(cachePath, remotePackagesUrl); + this.map = onlineVersionsFromScratch( + this.syncHttpGet, + cachePath, + remotePackagesUrl + ); return; } try { @@ -71,10 +68,14 @@ class OnlineVersionsCache { // Complete cache with a remote call to the package server. const remoteUrl = remotePackagesUrl + '/since/' + (versionsCount - 1); // -1 to check if no package was deleted. - const newVersions = JSON.parse(syncGetWorker().get(remoteUrl)); + const newVersions = JSON.parse(this.syncHttpGet(remoteUrl)); if (newVersions.length === 0) { // Reload from scratch since it means at least one package was deleted from the registry. - this.map = onlineVersionsFromScratch(cachePath, remotePackagesUrl); + this.map = onlineVersionsFromScratch( + this.syncHttpGet, + cachePath, + remotePackagesUrl + ); return; } // Check that the last package in the list was already in cache @@ -100,7 +101,11 @@ class OnlineVersionsCache { fs.writeFileSync(cachePath, JSON.stringify(onlineVersions)); } else { // There was a problem and a package got deleted from the server. - this.map = onlineVersionsFromScratch(cachePath, remotePackagesUrl); + this.map = onlineVersionsFromScratch( + this.syncHttpGet, + cachePath, + remotePackagesUrl + ); } } @@ -205,8 +210,13 @@ function readVersionsInElmHomeAndSort(pkg) { */ class DependencyProvider { - /** @type { OnlineVersionsCache } */ - cache = new OnlineVersionsCache(); + /** + * @param { import('./SyncGet').SyncHttpGet } syncHttpGet + */ + constructor(syncHttpGet) { + this.syncHttpGet = syncHttpGet; + this.cache = new OnlineVersionsCache(syncHttpGet); + } /** * Solve dependencies completely offline, without any http request. @@ -259,7 +269,8 @@ class DependencyProvider { elmJson, useTest, extra, - fetchElmJsonOnline, + /** @type { (pkg: string, version: string) => string } */ + (pkg, version) => fetchElmJsonOnline(this.syncHttpGet, pkg, version), /** @type { (pkg: string) => Array } */ (pkg) => lister.list( @@ -274,11 +285,12 @@ class DependencyProvider { } /** + * @param { import('./SyncGet').SyncHttpGet } syncHttpGet * @param { string } pkg * @param { string } version * @returns { string } */ -function fetchElmJsonOnline(pkg, version) { +function fetchElmJsonOnline(syncHttpGet, pkg, version) { try { return fetchElmJsonOffline(pkg, version); } catch (_) { @@ -287,7 +299,7 @@ function fetchElmJsonOnline(pkg, version) { // or because there was an error parsing `pkg` and `version`. // In such case, this will throw again with `cacheElmJsonPath()` so it's fine. const remoteUrl = remoteElmJsonUrl(pkg, version); - const elmJson = syncGetWorker().get(remoteUrl); + const elmJson = syncHttpGet(remoteUrl); const cachePath = cacheElmJsonPath(pkg, version); const parentDir = path.dirname(cachePath); fs.mkdirSync(parentDir, { recursive: true }); @@ -318,12 +330,13 @@ function fetchElmJsonOffline(pkg, version) { * Reset the cache of existing versions from scratch * with a request to the package server. * + * @param { import('./SyncGet').SyncHttpGet } syncHttpGet * @param { string } cachePath * @param { string } remotePackagesUrl * @returns { Map> } */ -function onlineVersionsFromScratch(cachePath, remotePackagesUrl) { - const onlineVersionsJson = syncGetWorker().get(remotePackagesUrl); +function onlineVersionsFromScratch(syncHttpGet, cachePath, remotePackagesUrl) { + const onlineVersionsJson = syncHttpGet(remotePackagesUrl); fs.writeFileSync(cachePath, onlineVersionsJson); const onlineVersions = JSON.parse(onlineVersionsJson); try { diff --git a/lib/Supervisor.js b/lib/Supervisor.js index fa8a53a9..43fcbd38 100644 --- a/lib/Supervisor.js +++ b/lib/Supervisor.js @@ -30,6 +30,17 @@ function run(elmTestVersion, pipeFilename, report, processes, dest, watch) { /** @type { Array } */ var workers = []; + /** + * @param { number } exitCode + * @returns { void } + */ + function end(exitCode) { + if (server) { + server.close(); + } + resolve(exitCode); + } + /** * @param { any } result This `any` became explicit instead of implicit when migrating from Flow to TypeScript. * @returns { void } @@ -227,7 +238,7 @@ function run(elmTestVersion, pipeFilename, report, processes, dest, watch) { workers.forEach(function (worker) { worker.kill(); }); - resolve(response.exitCode); + end(response.exitCode); break; case 'BEGIN': testsToRun = response.testCount; @@ -304,11 +315,11 @@ function run(elmTestVersion, pipeFilename, report, processes, dest, watch) { reportRuntimeException(); pendingException = false; } - resolve(1); + end(1); } } else if (hasNonZeroExitCode) { reportRuntimeException(); - resolve(1); + end(1); } }); diff --git a/lib/SyncGet.js b/lib/SyncGet.js index 04106d12..af523eae 100644 --- a/lib/SyncGet.js +++ b/lib/SyncGet.js @@ -6,12 +6,34 @@ const { } = require('worker_threads'); /** + * @typedef { (key: string) => string } SyncHttpGet + * * @typedef { { - get: (key: string) => string, + get: SyncHttpGet, shutDown: () => void, } } SyncGetWorker */ +class LazySyncGetWorker { + /** @type { SyncGetWorker | undefined } */ + worker = undefined; + + /** @type { SyncGetWorker["get"] } */ + get(key) { + if (this.worker === undefined) { + this.worker = startWorker(); + } + return this.worker.get(key); + } + + /** @type { SyncGetWorker["shutDown"] } */ + shutDown() { + if (this.worker !== undefined) { + this.worker.shutDown(); + } + } +} + /** * Start a worker thread and return a `syncGetWorker` * capable of making sync requests until shut down. @@ -51,5 +73,5 @@ function startWorker() { } module.exports = { - startWorker, + LazySyncGetWorker, }; diff --git a/lib/elm-test.js b/lib/elm-test.js index 6df25de2..9a879602 100644 --- a/lib/elm-test.js +++ b/lib/elm-test.js @@ -13,6 +13,33 @@ const Install = require('./Install'); const Project = require('./Project'); const Report = require('./Report'); const RunTests = require('./RunTests'); +const SyncGet = require('./SyncGet'); + +/** + * Printing to the terminal is a synchronous function in Node.js, + * but behind the scenes it might take a short while for the print + * to finish when printing a larger amount of text. If we call + * `process.exit` right after a large print, there is a risk that + * the output is cut off midway. The way to handle this is to set + * `process.exitCode` instead, and let the process exit when the + * event loop runs out. The downside of this approach is that if + * we forget to shut some listener down, elm-test never exits. + * We use this function instead of `process.exit` in places where + * we sometimes write a large amount of output (at the end of + * `elm-test make` and at the end of test runs) and where we have + * taken care to shut everything down. + * + * An important case is `elm-test --report=json`. Right at the end + * workers can send multiple messages at once, one for each passing + * test that was buffered. If they are never printed, the consumer + * (such as intellij-elm) might think the tests never finished and + * were terminated instead of successful. + * + * @param { number } exitCode + */ +function setExitCode(exitCode) { + process.exitCode = exitCode; +} /** @type { (minimum: number) => (string: string) => number } */ const parsePositiveInteger = (minimum) => (string) => { @@ -117,7 +144,11 @@ elm-test "src/**/*Tests.elm" const numberOfLogicalCPUCores = os.cpus().length; function main() { - const dependencyProvider = new DependencyProvider(); + // Note: If you use `dependencyProvider`, you must call `lazySyncGetWorker.shutDown()` afterwards! + const lazySyncGetWorker = new SyncGet.LazySyncGetWorker(); + const dependencyProvider = new DependencyProvider( + lazySyncGetWorker.get.bind(lazySyncGetWorker) + ); process.title = 'elm-test'; @@ -260,11 +291,21 @@ function main() { options['report'] ); }; - make().then( - () => process.exit(0), - // `elm-test make` has never logged errors it seems. - () => process.exit(1) - ); + make() + .finally(() => lazySyncGetWorker.shutDown()) + .then( + () => setExitCode(0), + (error) => { + if (error instanceof Compile.ElmMakeError) { + // `elm-test make` has never printed the “`elm make` failed with exit code 1.” + // part, which is why we don’t print `error` here. + setExitCode(1); + } else { + console.error(error.message); + process.exit(1); + } + } + ); }); program @@ -286,13 +327,19 @@ function main() { processes, // The flag validations are supposed to make this type assertion safe here. /** @type { import('./RunTests').Options } */ (options) - ).then( - (code) => process.exit(code), - (error) => { - console.error(error.message); - process.exit(1); - } - ); + ) + .finally(() => lazySyncGetWorker.shutDown()) + .then( + (code) => setExitCode(code), + (error) => { + console.error(error.message); + if (error instanceof Compile.ElmMakeError) { + setExitCode(1); + } else { + process.exit(1); + } + } + ); }); program.parse(process.argv); diff --git a/tests/flags.js b/tests/flags.js index cab52ddd..4a01c48f 100644 --- a/tests/flags.js +++ b/tests/flags.js @@ -483,7 +483,6 @@ describe('flags', () => { '1', path.join('tests', 'Passing', 'One.elm'), ]); - console.log(runResult); assert.strictEqual(runResult.status, 0); }); });