Fix truncated messages at the end of test runs

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,
 };

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<string, Array<string>> } */
   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<string> } */
         (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<string, Array<string>> }
  */
-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 {

lib/Supervisor.js

@@ -30,6 +30,17 @@ function run(elmTestVersion, pipeFilename, report, processes, dest, watch) {
     /** @type { Array<import('child_process').ChildProcess> } */
     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);
             }
           });
 

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,
 };

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);

tests/flags.js

@@ -483,7 +483,6 @@ describe('flags', () => {
         '1',
         path.join('tests', 'Passing', 'One.elm'),
       ]);
-      console.log(runResult);
       assert.strictEqual(runResult.status, 0);
     });
   });