consolidate implementaitons of getting version ranges

packages/dd-trace/test/setup/helpers/version-utils.js

@@ -0,0 +1,77 @@
+'use strict'
+
+const { execSync } = require('child_process')
+const semver = require('semver')
+
+const versions = {}
+
+function getAllVersions (name) {
+  // TODO caching this per process is probably sufficient for now, but to handle
+  // the case where a package is published between process that run one after the
+  // other, we should probably cache this in a file or something.
+  if (!versions[name]) {
+    versions[name] = JSON.parse(execSync('npm show ' + name + ' versions --json').toString())
+  }
+  return versions[name]
+}
+
+function getAllVersionsMatching (name, ranges) {
+  return getAllVersions(name).filter(version => ranges.some(range => semver.satisfies(version, range)))
+}
+
+function rangesTestVersion (name, ranges, version) {
+  for (const range of ranges) {
+    const matches = getAllVersionsMatching(name, [range])
+    if (matches[0] === version) {
+      return true
+    }
+    if (matches[matches.length - 1] === version) {
+      return true
+    }
+  }
+  return false
+}
+
+// In addition to version ranges inputted, we also want to ensure that
+// versions at the beggining and end of any major version covered by
+// existing ranges are individually tested.
+function getIdeallyTestedVersionRanges (name, ranges) {
+  const result = ranges.slice()
+  const allVersionsMatching = getAllVersionsMatching(name, ranges)
+  for (let i = 0; i < allVersionsMatching.length; i++) {
+    const version = allVersionsMatching[i]
+    if (i !== 0 && version.endsWith('.0.0')) {
+      if (!rangesTestVersion(name, ranges, version)) {
+        result.push(version)
+      }
+      if (allVersionsMatching[i - 1] && !rangesTestVersion(name, ranges, allVersionsMatching[i - 1])) {
+        result.push(allVersionsMatching[i - 1])
+      }
+    }
+  }
+  return result
+}
+
+function getIdeallyTestedVersions (name, ranges) {
+  ranges = (process.env.PACKAGE_VERSION_RANGE
+    ? [process.env.PACKAGE_VERSION_RANGE]
+    : ranges || [])
+    .filter(range => !process.env.RANGE || semver.subset(range, process.env.RANGE))
+  // TODO sub `ranges` for `getIdeallyTestedVersionRanges(name, ranges)` below
+  // once we're ready to use it
+  return ranges.reduce((acc, range) => {
+    const matches = getAllVersionsMatching(name, [range])
+    if (range !== '*') {
+      acc.push({ range, version: matches[0] })
+    }
+    acc.push({ range, version: matches[matches.length - 1] })
+    return acc
+  }, [])
+}
+
+module.exports = {
+  getAllVersions,
+  getAllVersionsMatching,
+  getIdeallyTestedVersionRanges,
+  getIdeallyTestedVersions
+}

packages/dd-trace/test/setup/mocha.js

@@ -12,6 +12,8 @@ const Nomenclature = require('../../src/service-naming')
 const { storage } = require('../../../datadog-core')
 const { schemaDefinitions } = require('../../src/service-naming/schemas')
 const { getInstrumentation } = require('./helpers/load-inst')
+const { getIdeallyTestedVersions } = require('./helpers/version-utils')
+const fs = require('fs')
 
 global.withVersions = withVersions
 global.withExports = withExports
@@ -168,38 +170,31 @@ function withVersions (plugin, modules, range, cb) {
       if (!packages.includes(moduleName)) return
     }
 
-    const testVersions = new Map()
+    const testVersions = []
 
     instrumentations
       .filter(instrumentation => instrumentation.name === moduleName)
       .forEach(instrumentation => {
-        const versions = process.env.PACKAGE_VERSION_RANGE
-          ? [process.env.PACKAGE_VERSION_RANGE]
-          : instrumentation.versions
-        versions
-          .filter(version => !process.env.RANGE || semver.subset(version, process.env.RANGE))
-          .forEach(version => {
-            if (version !== '*') {
-              const min = semver.coerce(version).version
-
-              testVersions.set(min, { range: version, test: min })
-            }
-
-            const max = require(`../../../../versions/${moduleName}@${version}`).version()
-
-            testVersions.set(max, { range: version, test: version })
-          })
+        const versionRanges = instrumentation.versions
+        const ideallyTestedVersions = getIdeallyTestedVersions(moduleName, versionRanges)
+        testVersions.push(...ideallyTestedVersions)
       })
 
-    Array.from(testVersions)
-      .filter(v => !range || semver.satisfies(v[0], range))
-      .sort(v => v[0].localeCompare(v[0]))
-      .map(v => Object.assign({}, v[1], { version: v[0] }))
+    // TODO this isn't the best way to dedupe
+    Array.from(new Set(testVersions.map(x => JSON.stringify(x))))
+      .map(x => JSON.parse(x))
+      // TODO range is nonsense since it can only work if there's only one module
+      .filter(v => !range || semver.satisfies(v.version, range))
+      .sort(v => v.version.localeCompare(v.version)) // What??? comparing with itself???
       .forEach(v => {
-        const versionPath = path.resolve(
+        let versionPath = path.resolve(
           __dirname, '../../../../versions/',
-          `${moduleName}@${v.test}/node_modules`
+          `${moduleName}@${v.version}`
         )
+        if (!fs.existsSync(versionPath)) {
+          throw new Error(`Version path does not exist "${versionPath}". Try running \`yarn services\``)
+        }
+        versionPath = `${versionPath}/node_modules`
 
         describe(`with ${moduleName} ${v.range} (${v.version})`, () => {
           let nodePath
@@ -220,7 +215,7 @@ function withVersions (plugin, modules, range, cb) {
             require('module').Module._initPaths()
           })
 
-          cb(v.test, moduleName, v.version)
+          cb(v.range, moduleName, v.version) // TODO get rid of 3rd param here
 
           after(() => {
             process.env.NODE_PATH = nodePath

scripts/install_plugin_modules.js

@@ -9,6 +9,7 @@ const exec = require('./helpers/exec')
 const childProcess = require('child_process')
 const externals = require('../packages/dd-trace/test/plugins/externals')
 const { getInstrumentation } = require('../packages/dd-trace/test/setup/helpers/load-inst')
+const { getIdeallyTestedVersions } = require('../packages/dd-trace/test/setup/helpers/version-utils')
 
 const requirePackageJsonPath = require.resolve('../packages/dd-trace/src/require-package-json')
 
@@ -63,24 +64,25 @@ async function assertVersions () {
 }
 
 async function assertInstrumentation (instrumentation, external) {
-  const versions = process.env.PACKAGE_VERSION_RANGE && !external
-    ? [process.env.PACKAGE_VERSION_RANGE]
-    : [].concat(instrumentation.versions || [])
-
-  for (const version of versions) {
-    if (version) {
-      if (version !== '*') {
-        await assertModules(instrumentation.name, semver.coerce(version).version, external)
-      }
+  const versions = getIdeallyTestedVersions(instrumentation.name, instrumentation.versions)
 
-      await assertModules(instrumentation.name, version, external)
-    }
+  let last
+  for (const entry of versions) {
+    last = entry
+    await assertModules(instrumentation.name, entry.version, external)
+  }
+
+  // Some tests require this symlink, since they were written expecting looser
+  // versions. These are tests that fail when _specific_ versions are passed
+  // as the first argument to the `withVersions` callback.
+  // TODO fix those tests.
+  let dest = folder(instrumentation.name, last.range)
+  if (!fs.existsSync(dest)) {
+    fs.symlinkSync(folder(instrumentation.name, last.version), dest)
   }
 }
 
 async function assertModules (name, version, external) {
-  const range = process.env.RANGE
-  if (range && !semver.subset(version, range)) return
   addFolder(name)
   addFolder(name, version)
   assertFolder(name)