From 1ab6a3b5f3e291ed0a21a8c0432b3b32597dec03 Mon Sep 17 00:00:00 2001 From: Simon Lydell Date: Sat, 1 Aug 2026 11:39:01 +0200 Subject: [PATCH 1/4] Fix truncated messages at the end of test runs --- lib/Supervisor.js | 17 ++++++++++++++--- lib/elm-test.js | 36 ++++++++++++++++++++++++++++++++---- tests/flags.js | 1 - 3 files changed, 46 insertions(+), 8 deletions(-) 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/elm-test.js b/lib/elm-test.js index 6df25de2..751cc61f 100644 --- a/lib/elm-test.js +++ b/lib/elm-test.js @@ -14,6 +14,32 @@ const Project = require('./Project'); const Report = require('./Report'); const RunTests = require('./RunTests'); +/** + * 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 wrote 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 had 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) => { const number = Number(string); @@ -261,9 +287,11 @@ function main() { ); }; make().then( - () => process.exit(0), - // `elm-test make` has never logged errors it seems. - () => process.exit(1) + () => setExitCode(0), + // `Compile.compileSources` rejects with an error like this on compile errors: + // `elm make` failed with exit code 1. + // We have never printed that part in `make` mode, so we ignore the error parameter here. + () => setExitCode(1) ); }); @@ -287,7 +315,7 @@ function main() { // The flag validations are supposed to make this type assertion safe here. /** @type { import('./RunTests').Options } */ (options) ).then( - (code) => process.exit(code), + (code) => setExitCode(code), (error) => { console.error(error.message); process.exit(1); 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); }); }); From a2626c326e8530fdf449782d4345fee8b4c3fd7c Mon Sep 17 00:00:00 2001 From: Simon Lydell Date: Sat, 1 Aug 2026 13:50:28 +0200 Subject: [PATCH 2/4] Fix comment --- lib/elm-test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/elm-test.js b/lib/elm-test.js index 751cc61f..2ba7cad3 100644 --- a/lib/elm-test.js +++ b/lib/elm-test.js @@ -24,13 +24,13 @@ const RunTests = require('./RunTests'); * 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 wrote a large amount of output (at the end of + * 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 had buffered. If they are never printed, the consumer + * 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. * From efd8fb92a87a55b1e95ca77ebb61c7f0a5ba67ef Mon Sep 17 00:00:00 2001 From: Simon Lydell Date: Sat, 1 Aug 2026 18:34:12 +0200 Subject: [PATCH 3/4] Properly shut down the SyncGetWorker --- lib/DependencyProvider.js | 63 +++++++++++++++++++++++---------------- lib/SyncGet.js | 26 ++++++++++++++-- lib/elm-test.js | 24 ++++++++++----- 3 files changed, 78 insertions(+), 35 deletions(-) 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/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 2ba7cad3..9944dfd2 100644 --- a/lib/elm-test.js +++ b/lib/elm-test.js @@ -13,6 +13,7 @@ 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, @@ -143,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'; @@ -276,6 +281,7 @@ function main() { const project = getProject('make'); const make = async () => { Generate.generateElmJson(dependencyProvider, project); + lazySyncGetWorker.shutDown(); await Compile.compileSources( FindTests.resolveGlobs( testFileGlobs.length === 0 ? [project.testsDir] : testFileGlobs, @@ -314,13 +320,15 @@ function main() { processes, // The flag validations are supposed to make this type assertion safe here. /** @type { import('./RunTests').Options } */ (options) - ).then( - (code) => setExitCode(code), - (error) => { - console.error(error.message); - process.exit(1); - } - ); + ) + .finally(() => lazySyncGetWorker.shutDown()) + .then( + (code) => setExitCode(code), + (error) => { + console.error(error.message); + process.exit(1); + } + ); }); program.parse(process.argv); From eb7cdfdee3aafc797109c77520a53c25b4eeb132 Mon Sep 17 00:00:00 2001 From: Simon Lydell Date: Sat, 1 Aug 2026 19:02:39 +0200 Subject: [PATCH 4/4] Use a hard process.exit for unknown errors --- lib/Compile.js | 14 ++++++++++++-- lib/elm-test.js | 29 ++++++++++++++++++++--------- 2 files changed, 32 insertions(+), 11 deletions(-) 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/elm-test.js b/lib/elm-test.js index 9944dfd2..9a879602 100644 --- a/lib/elm-test.js +++ b/lib/elm-test.js @@ -281,7 +281,6 @@ function main() { const project = getProject('make'); const make = async () => { Generate.generateElmJson(dependencyProvider, project); - lazySyncGetWorker.shutDown(); await Compile.compileSources( FindTests.resolveGlobs( testFileGlobs.length === 0 ? [project.testsDir] : testFileGlobs, @@ -292,13 +291,21 @@ function main() { options['report'] ); }; - make().then( - () => setExitCode(0), - // `Compile.compileSources` rejects with an error like this on compile errors: - // `elm make` failed with exit code 1. - // We have never printed that part in `make` mode, so we ignore the error parameter here. - () => setExitCode(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 @@ -326,7 +333,11 @@ function main() { (code) => setExitCode(code), (error) => { console.error(error.message); - process.exit(1); + if (error instanceof Compile.ElmMakeError) { + setExitCode(1); + } else { + process.exit(1); + } } ); });