Fix mismatched app and test versions for indirect dependencies

lib/DependencyProvider.js

@@ -9,8 +9,15 @@ const collator = new Intl.Collator('en', { numeric: true }); // for sorting SemV
 
 // Initialization work done only once.
 wasm.init();
-const syncGetWorker /*: {| get: (string) => string, shutDown: () => void |} */ =
-  SyncGet.startWorker();
+// Lazily start the worker until needed.
+// This is important for the tests, which never exit otherwise.
+let syncGetWorker_ /*: void | typeof SyncGet.SyncGetWorker */ = undefined;
+function syncGetWorker() /*: typeof SyncGet.SyncGetWorker */ {
+  if (syncGetWorker_ === undefined) {
+    syncGetWorker_ = SyncGet.startWorker();
+  }
+  return syncGetWorker_;
+}
 
 // Cache of existing versions according to the package website.
 class OnlineVersionsCache {
@@ -55,7 +62,7 @@ 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(syncGetWorker().get(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);
@@ -104,10 +111,13 @@ class OnlineAvailableVersionLister {
     this.onlineCache = onlineCache;
   }
 
-  list(pkg /*: string */) /*: Array<string> */ {
+  list(
+    pkg /*: string */,
+    pinnedVersion /*: void | string */
+  ) /*: Array<string> */ {
     const memoVersions = this.memoCache.get(pkg);
     if (memoVersions !== undefined) {
-      return memoVersions;
+      return prioritizePinnedIndirectVersion(memoVersions, pinnedVersion);
     }
     const offlineVersions = readVersionsInElmHomeAndSort(pkg);
     const allVersionsSet = new Set(this.onlineCache.getVersions(pkg));
@@ -117,24 +127,27 @@ class OnlineAvailableVersionLister {
     }
     const allVersions = [...allVersionsSet].sort(flippedSemverCompare);
     this.memoCache.set(pkg, allVersions);
-    return allVersions;
+    return prioritizePinnedIndirectVersion(allVersions, pinnedVersion);
   }
 }
 
 class OfflineAvailableVersionLister {
   // Memoization cache to avoid doing the same work twice in list.
   cache /*: Map<string, Array<string>> */ = new Map();
 
-  list(pkg /*: string */) /*: Array<string> */ {
+  list(
+    pkg /*: string */,
+    pinnedVersion /*: void | string */
+  ) /*: Array<string> */ {
     const memoVersions = this.cache.get(pkg);
     if (memoVersions !== undefined) {
-      return memoVersions;
+      return prioritizePinnedIndirectVersion(memoVersions, pinnedVersion);
     }
 
     const offlineVersions = readVersionsInElmHomeAndSort(pkg);
 
     this.cache.set(pkg, offlineVersions);
-    return offlineVersions;
+    return prioritizePinnedIndirectVersion(offlineVersions, pinnedVersion);
   }
 }
 
@@ -162,13 +175,21 @@ class DependencyProvider {
     extra /*: { [string]: string } */
   ) /*: string */ {
     const lister = new OfflineAvailableVersionLister();
+    const dependencies = JSON.parse(elmJson).dependencies;
+    const indirectDeps =
+      dependencies === undefined ? undefined : dependencies.indirect;
+
     try {
       return wasm.solve_deps(
         elmJson,
         useTest,
         extra,
         fetchElmJsonOffline,
-        (pkg) => lister.list(pkg)
+        (pkg) =>
+          lister.list(
+            pkg,
+            indirectDeps === undefined ? undefined : indirectDeps[pkg]
+          )
       );
     } catch (errorMessage) {
       throw new Error(errorMessage);
@@ -182,13 +203,21 @@ class DependencyProvider {
     extra /*: { [string]: string } */
   ) /*: string */ {
     const lister = new OnlineAvailableVersionLister(this.cache);
+    const dependencies = JSON.parse(elmJson).dependencies;
+    const indirectDeps =
+      dependencies === undefined ? undefined : dependencies.indirect;
+
     try {
       return wasm.solve_deps(
         elmJson,
         useTest,
         extra,
         fetchElmJsonOnline,
-        (pkg) => lister.list(pkg)
+        (pkg) =>
+          lister.list(
+            pkg,
+            indirectDeps === undefined ? undefined : indirectDeps[pkg]
+          )
       );
     } catch (errorMessage) {
       throw new Error(errorMessage);
@@ -208,7 +237,7 @@ function fetchElmJsonOnline(
     // 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 = syncGetWorker().get(remoteUrl);
     const cachePath = cacheElmJsonPath(pkg, version);
     const parentDir = path.dirname(cachePath);
     fs.mkdirSync(parentDir, { recursive: true });
@@ -239,7 +268,7 @@ function onlineVersionsFromScratch(
   cachePath /*: string */,
   remotePackagesUrl /*: string */
 ) /*: Map<string, Array<string>> */ {
-  const onlineVersionsJson = syncGetWorker.get(remotePackagesUrl);
+  const onlineVersionsJson = syncGetWorker().get(remotePackagesUrl);
   fs.writeFileSync(cachePath, onlineVersionsJson);
   const onlineVersions = JSON.parse(onlineVersionsJson);
   try {
@@ -253,6 +282,42 @@ function onlineVersionsFromScratch(
 
 // Helper functions ##################################################
 
+/**
+ * Enforces respecting pinned indirect dependencies.
+ *
+ * When Elm apps have pinned indirect versions, e.g.:
+ *
+ * "indirect": {
+ *   "elm/virtual-dom": "1.0.3"
+ * }
+ *
+ * We must prioritize these versions for the wasm dependency solver.
+ *
+ * Otherwise the wasm solver will take liberties that will result in
+ * tests running with dependency versions distinct from those used by
+ * the real live application.
+ *
+ * Assumes versions is sorted descending (newest -> oldest).
+ */
+function prioritizePinnedIndirectVersion(
+  versions /*: Array<string> */,
+  pinnedVersion /*: void | string */
+) /*: Array<string> */ {
+  if (pinnedVersion === undefined || !versions.includes(pinnedVersion)) {
+    return versions;
+  }
+
+  // the pinned version and any newer version, in ascending order
+  const desirableVersions = versions
+    .filter((v) => v >= pinnedVersion)
+    .reverse();
+
+  // older versions, in descending order
+  const olderVersions = versions.filter((v) => v < pinnedVersion);
+
+  return desirableVersions.concat(olderVersions);
+}
+
 /* Compares two versions so that newer versions appear first when sorting with this function. */
 function flippedSemverCompare(a /*: string */, b /*: string */) /*: number */ {
   return collator.compare(b, a);
@@ -340,4 +405,7 @@ function splitPkgVersion(str /*: string */) /*: {
   return { pkg: parts[0], version: parts[1] };
 }
 
-module.exports = DependencyProvider;
+module.exports = {
+  DependencyProvider,
+  prioritizePinnedIndirectVersion,
+};

lib/Generate.js

@@ -3,7 +3,7 @@
 const { supportsColor } = require('chalk');
 const fs = require('fs');
 const path = require('path');
-const DependencyProvider = require('./DependencyProvider.js');
+const { DependencyProvider } = require('./DependencyProvider.js');
 const ElmJson = require('./ElmJson');
 const Project = require('./Project');
 const Report = require('./Report');

lib/RunTests.js

@@ -6,7 +6,7 @@ const path = require('path');
 const readline = require('readline');
 const packageInfo = require('../package.json');
 const Compile = require('./Compile');
-const DependencyProvider = require('./DependencyProvider.js');
+const { DependencyProvider } = require('./DependencyProvider.js');
 const ElmJson = require('./ElmJson');
 const FindTests = require('./FindTests');
 const Generate = require('./Generate');

lib/Solve.js

@@ -5,7 +5,7 @@ const fs = require('fs');
 const path = require('path');
 const ElmJson = require('./ElmJson');
 const Project = require('./Project');
-const DependencyProvider = require('./DependencyProvider.js');
+const { DependencyProvider } = require('./DependencyProvider.js');
 
 // These value are used _only_ in flow types. 'use' them with the javascript
 // void operator to keep eslint happy.

lib/SyncGet.js

@@ -8,12 +8,19 @@ const {
   // $FlowFixMe[cannot-resolve-module]: Flow doesn’t seem to know about the `worker_threads` module yet.
 } = require('worker_threads');
 
-// Start a worker thread and return a `syncGetWorker`
-// capable of making sync requests until shut down.
-function startWorker() /*: {
+// Poor man’s type alias. We can’t use /*:: type SyncGetWorker = ... */ because of:
+// https://github.com/prettier/prettier/issues/2597
+const SyncGetWorker /*: {
   get: (string) => string,
   shutDown: () => void,
-} */ {
+} */ = {
+  get: (string) => string,
+  shutDown: () => {},
+};
+
+// Start a worker thread and return a `syncGetWorker`
+// capable of making sync requests until shut down.
+function startWorker() /*: typeof SyncGetWorker */ {
   const { port1: localPort, port2: workerPort } = new MessageChannel();
   const sharedLock = new SharedArrayBuffer(4);
   // $FlowFixMe[incompatible-call]: Flow is wrong and says `sharedLock` is not an accepted parameter here.
@@ -41,5 +48,6 @@ function startWorker() /*: {
 }
 
 module.exports = {
+  SyncGetWorker,
   startWorker,
 };

lib/elm-test.js

@@ -7,7 +7,7 @@ const path = require('path');
 const which = require('which');
 const packageInfo = require('../package.json');
 const Compile = require('./Compile');
-const DependencyProvider = require('./DependencyProvider.js');
+const { DependencyProvider } = require('./DependencyProvider.js');
 const ElmJson = require('./ElmJson');
 const FindTests = require('./FindTests');
 const Generate = require('./Generate');

tests/DependencyProvider.js

@@ -0,0 +1,50 @@
+'use strict';
+
+const assert = require('assert');
+const {
+  prioritizePinnedIndirectVersion,
+} = require('../lib/DependencyProvider');
+
+describe('DependencyProvider', () => {
+  describe('prioritizePinnedIndirectVersion', () => {
+    const versions = ['1.0.5', '1.0.4', '1.0.3', '1.0.2', '1.0.1', '1.0.0'];
+
+    const testPinning = (pinnedVersion, expectedResult) => {
+      assert.deepStrictEqual(
+        prioritizePinnedIndirectVersion(versions, pinnedVersion),
+        expectedResult
+      );
+    };
+
+    it('retains order when no pinned indirect dependency', () => {
+      testPinning(undefined, versions);
+    });
+
+    it("retains order when pinned version doesn't exist", () => {
+      testPinning('1.0.6', versions);
+    });
+
+    it('retains order if already at latest', () => {
+      testPinning('1.0.5', versions);
+    });
+
+    it("prioritizes a version in the middle, if we're pinned to it", () => {
+      const expected = [
+        // first, try the pinned version
+        '1.0.3',
+        // then, try upgrading
+        '1.0.4',
+        '1.0.5',
+        // then, try downgrading
+        '1.0.2',
+        '1.0.1',
+        '1.0.0',
+      ];
+      testPinning('1.0.3', expected);
+    });
+
+    it("prioritizes first version, if we're pinned to it", () => {
+      testPinning('1.0.0', [...versions].sort());
+    });
+  });
+});