diff --git a/lib/DependencyProvider.js b/lib/DependencyProvider.js index d432beb6..afd1fbcd 100644 --- a/lib/DependencyProvider.js +++ b/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 */ { + list( + pkg /*: string */, + pinnedVersion /*: void | string */ + ) /*: Array */ { 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,7 +127,7 @@ class OnlineAvailableVersionLister { } const allVersions = [...allVersionsSet].sort(flippedSemverCompare); this.memoCache.set(pkg, allVersions); - return allVersions; + return prioritizePinnedIndirectVersion(allVersions, pinnedVersion); } } @@ -125,16 +135,19 @@ class OfflineAvailableVersionLister { // Memoization cache to avoid doing the same work twice in list. cache /*: Map> */ = new Map(); - list(pkg /*: string */) /*: Array */ { + list( + pkg /*: string */, + pinnedVersion /*: void | string */ + ) /*: Array */ { 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> */ { - 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 */, + pinnedVersion /*: void | string */ +) /*: Array */ { + 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, +}; diff --git a/lib/Generate.js b/lib/Generate.js index 6d87e068..e6aec134 100644 --- a/lib/Generate.js +++ b/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'); diff --git a/lib/RunTests.js b/lib/RunTests.js index e66a1aab..42e34320 100644 --- a/lib/RunTests.js +++ b/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'); diff --git a/lib/Solve.js b/lib/Solve.js index 5801f801..21a7e397 100644 --- a/lib/Solve.js +++ b/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. diff --git a/lib/SyncGet.js b/lib/SyncGet.js index c8108800..32f5f427 100644 --- a/lib/SyncGet.js +++ b/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, }; diff --git a/lib/elm-test.js b/lib/elm-test.js index 854ef1e4..1127b77c 100644 --- a/lib/elm-test.js +++ b/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'); diff --git a/tests/DependencyProvider.js b/tests/DependencyProvider.js new file mode 100644 index 00000000..3ce7f395 --- /dev/null +++ b/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()); + }); + }); +});