test(plugins): select latest unversioned dependency range (#9352)

Runtime-gated declarations could leave the first active range as the unversioned target even when a newer compatible range was also active.

Author: BridgeAR

packages/dd-trace/test/plugins/externals.js

@@ -163,13 +163,12 @@ module.exports = {
   'express-mongo-sanitize': [
     {
       name: 'mongodb',
-      versions: ['>=3.3 <5', '5', '6', '>=7'],
-      node: '>=20.19.0',
+      versions: ['>=3.3 <5', '5', '6'],
     },
     {
       name: 'mongodb',
-      versions: ['>=3.3 <5', '5', '6'],
-      node: '<20.19.0',
+      versions: ['>=7'],
+      node: '>=20.19.0',
     },
     {
       name: 'mongodb-core',
@@ -201,13 +200,12 @@ module.exports = {
     },
     {
       name: 'mongodb',
-      versions: ['5', '6', '>=7'],
-      node: '>=20.19.0',
+      versions: ['5', '6'],
     },
     {
       name: 'mongodb',
-      versions: ['5', '6'],
-      node: '<20.19.0',
+      versions: ['>=7'],
+      node: '>=20.19.0',
     },
   ],
   mysql2: [

packages/dd-trace/test/plugins/versions.spec.js

@@ -3,9 +3,13 @@
 const assert = require('node:assert/strict')
 
 const { describe, it } = require('mocha')
-const { coerce, major } = require('semver')
+const { coerce, major, maxSatisfying } = require('semver')
 
-const { getVersionList, resolvePluginVersions, brokenVersionReason } = require('./versions')
+const {
+  brokenVersionReason,
+  getVersionList,
+  resolvePluginVersions,
+} = require('./versions')
 
 const latests = require('./versions/package.json').dependencies
 
@@ -80,32 +84,43 @@ describe('getVersionList', () => {
 
 describe('resolvePluginVersions', () => {
   const versionKeys = result => result.versionList.map(({ versionKey }) => versionKey)
-
-  it('expands the declared versions and points the unversioned folder at the newest in-scope key', () => {
-    const result = resolvePluginVersions({ name: 'mongodb', declaredVersions: ['>=2 <5'], env: {} })
+  const declarations = [
+    {
+      versions: ['>=7'],
+      node: '>=20.19.0',
+    },
+    {
+      versions: ['5', '6'],
+    },
+  ]
+
+  it('expands the declared versions and uses every active range for the unversioned folder', () => {
+    const result = resolvePluginVersions({
+      name: 'mongodb',
+      declarations: [{ versions: ['>=2 <5'] }],
+      env: {},
+    })
 
     assert.deepEqual(versionKeys(result), ['2.0.0', '2', '3', '4'])
-    assert.equal(result.unversioned, '4')
+    assert.equal(result.unversioned, '>=2 <5')
   })
 
   it('includes declarations at the Node.js range boundary', () => {
     const result = resolvePluginVersions({
       name: 'mongodb',
-      declaredVersions: ['>=2 <5'],
-      nodeRange: '>=22',
+      declarations: [{ versions: ['>=2 <5'], node: '>=22' }],
       nodeVersion: '22.0.0',
       env: {},
     })
 
     assert.deepEqual(versionKeys(result), ['2.0.0', '2', '3', '4'])
-    assert.equal(result.unversioned, '4')
+    assert.equal(result.unversioned, '>=2 <5')
   })
 
   it('excludes declarations outside the Node.js range before applying package range overrides', () => {
     const result = resolvePluginVersions({
       name: 'mongodb',
-      declaredVersions: ['>=2 <5'],
-      nodeRange: '>=22',
+      declarations: [{ versions: ['>=2 <5'], node: '>=22' }],
       nodeVersion: '21.999.999',
       env: { PACKAGE_VERSION_RANGE: '>=3 <4' },
     })
@@ -118,8 +133,7 @@ describe('resolvePluginVersions', () => {
     assert.throws(
       () => resolvePluginVersions({
         name: 'mongodb',
-        declaredVersions: ['>=2 <5'],
-        nodeRange: 'not-a-version',
+        declarations: [{ versions: ['>=2 <5'], node: 'not-a-version' }],
         nodeVersion: '22.0.0',
         env: {},
       }),
@@ -130,18 +144,19 @@ describe('resolvePluginVersions', () => {
   it('filters the installed keys by RANGE and follows the filtered tail', () => {
     const result = resolvePluginVersions({
       name: 'mongodb',
-      declaredVersions: ['>=1 <6'],
+      declarations: [{ versions: ['>=1 <6'] }],
       env: { RANGE: '>=2.0.0 <4.0.0' },
     })
 
     assert.deepEqual(versionKeys(result), ['2', '3'])
-    assert.equal(result.unversioned, '3')
+    assert.equal(result.unversioned, '2 || 3')
+    assert.equal(maxSatisfying(['2.12.0', '3.9.0'], result.unversioned), '3.9.0')
   })
 
   it('replaces the declared versions with PACKAGE_VERSION_RANGE when the module is honoured', () => {
     const result = resolvePluginVersions({
       name: 'mongodb',
-      declaredVersions: ['>=2 <5'],
+      declarations: [{ versions: ['>=2 <5'] }],
       env: { PACKAGE_VERSION_RANGE: '>=3 <4' },
     })
 
@@ -152,19 +167,19 @@ describe('resolvePluginVersions', () => {
   it('ignores PACKAGE_VERSION_RANGE for a sibling external that must not be sharded', () => {
     const result = resolvePluginVersions({
       name: 'mongodb',
-      declaredVersions: ['>=2 <5'],
+      declarations: [{ versions: ['>=2 <5'] }],
       honourEnvRange: false,
       env: { PACKAGE_VERSION_RANGE: '>=3 <4' },
     })
 
     assert.deepEqual(versionKeys(result), ['2.0.0', '2', '3', '4'])
-    assert.equal(result.unversioned, '4')
+    assert.equal(result.unversioned, '>=2 <5')
   })
 
   it('keeps the unversioned folder on the raw shard while RANGE narrows the installed keys', () => {
     const result = resolvePluginVersions({
       name: 'mongodb',
-      declaredVersions: ['>=1 <6'],
+      declarations: [{ versions: ['>=1 <6'] }],
       env: { PACKAGE_VERSION_RANGE: '>=2 <5', RANGE: '>=3.0.0 <4.0.0' },
     })
 
@@ -173,7 +188,7 @@ describe('resolvePluginVersions', () => {
   })
 
   it('reports nothing in scope when no version is declared', () => {
-    const result = resolvePluginVersions({ name: 'mongodb', declaredVersions: [], env: {} })
+    const result = resolvePluginVersions({ name: 'mongodb', declarations: [{}], env: {} })
 
     assert.deepEqual(result.versionList, [])
     assert.equal(result.unversioned, undefined)
@@ -182,13 +197,58 @@ describe('resolvePluginVersions', () => {
   it('reports nothing in scope when RANGE excludes every declared key', () => {
     const result = resolvePluginVersions({
       name: 'mongodb',
-      declaredVersions: ['>=2 <3'],
+      declarations: [{ versions: ['>=2 <3'] }],
       env: { RANGE: '>=9.0.0 <10.0.0' },
     })
 
     assert.deepEqual(result.versionList, [])
     assert.equal(result.unversioned, undefined)
   })
+
+  it('points the unversioned folder at the latest active declaration regardless of order', () => {
+    const result = resolvePluginVersions({
+      name: 'mongodb',
+      declarations,
+      nodeVersion: '20.19.0',
+      env: {},
+    })
+
+    assert.deepEqual(
+      versionKeys(result),
+      ['7.0.0', '7', '5.0.0', '5', '6.0.0', '6']
+    )
+    assert.equal(maxSatisfying(['6.21.0', '7.2.0'], result.unversioned), '7.2.0')
+  })
+
+  it('excludes declarations unsupported by the current Node.js version', () => {
+    const result = resolvePluginVersions({
+      name: 'mongodb',
+      declarations,
+      nodeVersion: '20.18.0',
+      env: {},
+    })
+
+    assert.deepEqual(
+      versionKeys(result),
+      ['5.0.0', '5', '6.0.0', '6']
+    )
+    assert.equal(result.unversioned, '5 || 6')
+  })
+
+  it('applies a package version override once across active declarations', () => {
+    const result = resolvePluginVersions({
+      name: 'mongodb',
+      declarations,
+      nodeVersion: '20.19.0',
+      env: { PACKAGE_VERSION_RANGE: '>=6 <7' },
+    })
+
+    assert.deepEqual(
+      versionKeys(result),
+      ['6.0.0', '6']
+    )
+    assert.equal(result.unversioned, '>=6 <7')
+  })
 })
 
 describe('brokenVersionReason', () => {

packages/dd-trace/test/plugins/versions/index.js

@@ -218,40 +218,55 @@ function highestMajor (name, range, floorMajor) {
  *
  * @param {object} options
  * @param {string} options.name The module name, e.g. `fastify`.
- * @param {string[]} options.declaredVersions The declared version entries to expand.
- * @param {string} [options.nodeRange] The Node.js versions where these declarations apply.
+ * @param {Array<{ versions?: string[], node?: string }>} options.declarations
  * @param {string} [options.nodeVersion] The current Node.js version; injectable for testing.
  * @param {boolean} [options.honourEnvRange] Whether `PACKAGE_VERSION_RANGE` applies to this module. False for sibling
  *   externals that must stay on their declared versions while the matrix shards a different package.
  * @param {NodeJS.ProcessEnv} [options.env] Injectable for testing.
  * @returns {{ versionList: Array<{ versionKey: string, range: string }>, unversioned: string|undefined }} The ordered,
- *   `RANGE`-filtered key set, and the key the default `versions/<name>` folder resolves to (the newest in-scope entry,
- *   or `undefined` when nothing is in scope).
+ *   `RANGE`-filtered key set, and the range the default `versions/<name>` folder resolves from.
  */
 function resolvePluginVersions ({
   name,
-  declaredVersions,
-  nodeRange,
+  declarations,
   nodeVersion = process.versions.node,
   honourEnvRange = true,
   env = process.env,
 }) {
-  if (nodeRange !== undefined) {
-    if (!validRange(nodeRange)) throw new Error(`Invalid Node.js version range for '${name}': ${nodeRange}`)
-    if (!satisfies(nodeVersion, nodeRange)) return { versionList: [], unversioned: undefined }
+  const useEnvRange = Boolean(env.PACKAGE_VERSION_RANGE) && honourEnvRange
+  const versions = []
+  let hasActiveDeclaration = false
+
+  for (const declaration of declarations) {
+    if (declaration.node !== undefined) {
+      if (!validRange(declaration.node)) {
+        throw new Error(`Invalid Node.js version range for '${name}': ${declaration.node}`)
+      }
+      if (!satisfies(nodeVersion, declaration.node)) continue
+    }
+
+    hasActiveDeclaration = true
+    if (!useEnvRange) versions.push(...(declaration.versions ?? []))
   }
 
-  const useEnvRange = Boolean(env.PACKAGE_VERSION_RANGE) && honourEnvRange
-  const versions = useEnvRange ? [env.PACKAGE_VERSION_RANGE] : declaredVersions
+  if (!hasActiveDeclaration) return { versionList: [], unversioned: undefined }
+  if (useEnvRange) versions.push(env.PACKAGE_VERSION_RANGE)
 
   let versionList = getVersionList(name, versions)
   if (env.RANGE) {
     versionList = versionList.filter(({ versionKey }) => subset(versionKey, env.RANGE))
+    if (!useEnvRange) {
+      versions.length = 0
+      for (const { versionKey } of versionList) versions.push(versionKey)
+    }
   }
 
   // With `PACKAGE_VERSION_RANGE` the shard itself is the target, so the unversioned folder keeps the raw range even
-  // when `RANGE` narrows the installed keys; otherwise it follows the newest in-scope key.
-  const unversioned = useEnvRange ? env.PACKAGE_VERSION_RANGE : versionList.at(-1)?.versionKey
+  // when `RANGE` narrows the installed keys. Otherwise the package manager selects the newest version from every
+  // active range, independent of declaration order.
+  const unversioned = useEnvRange
+    ? env.PACKAGE_VERSION_RANGE
+    : versions.length > 0 ? versions.join(' || ') : undefined
 
   return { versionList, unversioned }
 }

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

@@ -291,34 +291,29 @@ function withVersions (plugin, modules, range, cb) {
     /** @type {Map<string, {versionRange: string, versionKey: string, resolvedVersion: string}>} */
     const testVersions = new Map()
 
-    let moduleMatched = false
+    const declarations = []
     for (const instrumentation of instrumentations) {
-      if (instrumentation.name !== moduleName) continue
-      moduleMatched = true
-
-      // Some entries coming from `externals.js` are dependency-only (e.g. `dep: true`) and don't have `versions`.
-      // Treat those as "not a test target" instead of crashing.
-      // Share the install script's resolution so the tested folders exactly match the installed ones (lowest supported
-      // version, the latest of every major in between, and the newest supported version), de-duplicated by version.
-      const { versionList } = resolvePluginVersions({
-        name: moduleName,
-        declaredVersions: normalizeVersions(instrumentation.versions),
-        nodeRange: instrumentation.node,
-      })
-
-      for (const { versionKey, range: declaredRange } of versionList) {
-        // Exact keys resolve to themselves; range keys (`*`, `>=2`, `>=3.0.0 <4.0.0`) resolve to what was installed.
-        const resolvedVersion = semver.valid(versionKey) ?? require(getModulePath(moduleName, versionKey)).version()
-        testVersions.set(resolvedVersion, { versionRange: declaredRange, versionKey, resolvedVersion })
-      }
+      if (instrumentation.name === moduleName) declarations.push(instrumentation)
     }
 
     // A module no instrumentation declares would silently run zero tests instead of failing.
-    if (!moduleMatched) {
+    if (declarations.length === 0) {
       throw new Error(`withVersions: no instrumentation declares the module "${moduleName}". Pass the integration ` +
         `name as the first argument (e.g. 'express'), or register "${moduleName}" in test/plugins/externals.js.`)
     }
 
+    // Some entries coming from `externals.js` are dependency-only (e.g. `dep: true`) and don't have `versions`.
+    // Treat those as "not a test target" instead of crashing.
+    // Share the install script's resolution so the tested folders exactly match the installed ones (lowest supported
+    // version, the latest of every major in between, and the newest supported version), de-duplicated by version.
+    const { versionList } = resolvePluginVersions({ name: moduleName, declarations })
+
+    for (const { versionKey, range: declaredRange } of versionList) {
+      // Exact keys resolve to themselves; range keys (`*`, `>=2`, `>=3.0.0 <4.0.0`) resolve to what was installed.
+      const resolvedVersion = semver.valid(versionKey) ?? require(getModulePath(moduleName, versionKey)).version()
+      testVersions.set(resolvedVersion, { versionRange: declaredRange, versionKey, resolvedVersion })
+    }
+
     const testCases = Array.from(testVersions.values())
       .filter(({ resolvedVersion }) => !range || semver.satisfies(resolvedVersion, range))
       .sort(({ resolvedVersion }) => resolvedVersion.localeCompare(resolvedVersion))
@@ -370,11 +365,6 @@ function withVersions (plugin, modules, range, cb) {
   }
 }
 
-function normalizeVersions (versions) {
-  if (!versions) return []
-  return Array.isArray(versions) ? versions : [versions]
-}
-
 /**
  * @callback withExportsCallback
  * @param {() => import('module').Module} getExport - A function that returns the module export to test