fixup! address review comments

Author: BridgeAR

AGENTS.md

@@ -10,7 +10,7 @@
 
 - Install dependencies: `yarn install`
 
-**Note:** This project uses yarn, not npm. Always use `yarn` commands instead of `npm` commands.
+**This project uses yarn, not npm. Always use `yarn` commands instead of `npm` commands.**
 
 ## Project Overview
 
@@ -36,18 +36,13 @@ When developing a feature or fixing a bug:
 
 ### Running Individual Tests
 
-**IMPORTANT**: Never run `yarn test` directly. Use `mocha` or `tap` directly on test files.
+**IMPORTANT**: Never run `yarn test` directly. Use `mocha` directly on test files.
 
 **Mocha unit tests:**
 ```bash
 ./node_modules/.bin/mocha -r "packages/dd-trace/test/setup/mocha.js" path/to/test.spec.js
 ```
 
-**Tap unit tests:**
-```bash
-./node_modules/.bin/tap path/to/test.spec.js
-```
-
 **Integration tests:**
 ```bash
 ./node_modules/.bin/mocha --timeout 60000 -r "packages/dd-trace/test/setup/core.js" path/to/test.spec.js
@@ -59,8 +54,6 @@ When developing a feature or fixing a bug:
 **Enable debug logging:**
 - Prefix with `DD_TRACE_DEBUG=true`
 
-**Note**: New tests should be written using mocha, not tap. Existing tap tests use mocha-style `describe` and `it` blocks.
-
 ### Plugin Tests
 
 **Use `PLUGINS` env var:**

CONTRIBUTING.md

@@ -288,14 +288,14 @@ Please refer to [the "Install" section](https://github.com/brianc/node-postgres/
 
 When developing, it's often faster to run individual test files rather than entire test suites. **Never run `yarn test` directly** as it requires too much setup and takes too long.
 
-To target specific tests, use the `--grep` flag with mocha or tap to match test names:
+To target specific tests, use the `--grep` flag with mocha to match test names:
 
 ```sh
 yarn test:debugger --grep "test name pattern"
 yarn test:appsec --grep "specific test"
 ```
 
-**Note:** This project uses a mix of tap and mocha for testing. However, new tests should be written using mocha, not tap.
+**This project uses mocha for testing.**
 
 ### Test Assertions
 

benchmark/sirun/startup/startup-test.js

@@ -68,7 +68,6 @@ if (Number(process.env.EVERYTHING)) {
     'shell-quote',
     'sinon',
     'source-map',
-    'tap',
     'tiktoken',
     'tlhunter-sorted-set',
     'ttl-set',

packages/dd-trace/test/azure_metadata.spec.js

@@ -24,7 +24,7 @@ describe('Azure metadata', () => {
     'WEBSITE_SKU'
   ]
 
-  const initialAzureEnv = Object.fromEntries(AZURE_ENV_KEYS.map(k => [k, process.env[k]]))
+  const initialAzureEnv = Object.fromEntries(AZURE_ENV_KEYS.map(key => [key, process.env[key]]))
 
   afterEach(() => {
     for (const key of AZURE_ENV_KEYS) {
@@ -53,11 +53,6 @@ describe('Azure metadata', () => {
   })
 
   it('provided completely with minimum vars', () => {
-    delete process.env.WEBSITE_RESOURCE_GROUP
-    delete process.env.WEBSITE_OS
-    delete process.env.FUNCTIONS_EXTENSION_VERSION
-    delete process.env.FUNCTIONS_WORKER_RUNTIME
-    delete process.env.FUNCTIONS_WORKER_RUNTIME_VERSION
     process.env.COMPUTERNAME = 'boaty_mcboatface'
     process.env.WEBSITE_SITE_NAME = 'website_name'
     process.env.WEBSITE_OWNER_NAME = 'subscription_id+resource_group-regionwebspace'
@@ -110,11 +105,6 @@ describe('Azure metadata', () => {
   })
 
   it('tags are correctly generated from vars', () => {
-    delete process.env.WEBSITE_RESOURCE_GROUP
-    delete process.env.WEBSITE_OS
-    delete process.env.FUNCTIONS_EXTENSION_VERSION
-    delete process.env.FUNCTIONS_WORKER_RUNTIME
-    delete process.env.FUNCTIONS_WORKER_RUNTIME_VERSION
     process.env.COMPUTERNAME = 'boaty_mcboatface'
     process.env.WEBSITE_SITE_NAME = 'website_name'
     process.env.WEBSITE_OWNER_NAME = 'subscription_id+resource_group-regionwebspace'
@@ -137,9 +127,6 @@ describe('Azure metadata', () => {
   })
 
   it('uses DD_AZURE_RESOURCE_GROUP for Flex Consumption Azure Functions', () => {
-    delete process.env.WEBSITE_RESOURCE_GROUP
-    delete process.env.WEBSITE_OS
-    delete process.env.DD_AAS_DOTNET_EXTENSION_VERSION
     process.env.COMPUTERNAME = 'flex_function'
     process.env.WEBSITE_SITE_NAME = 'flex_function_app'
     process.env.WEBSITE_OWNER_NAME = 'subscription_id+flex-regionwebspace'

packages/dd-trace/test/custom-metrics.spec.js

@@ -6,7 +6,6 @@ const path = require('node:path')
 const os = require('node:os')
 const { exec } = require('node:child_process')
 
-/* eslint-disable no-console */
 const { describe, it, beforeEach, afterEach } = require('mocha')
 
 require('./setup/core')
@@ -52,7 +51,9 @@ describe('Custom Metrics', () => {
       }
     }, (err, stdout, stderr) => {
       if (err) return done(err)
+      // eslint-disable-next-line no-console
       if (stdout) console.log(stdout)
+      // eslint-disable-next-line no-console
       if (stderr) console.error(stderr)
 
       assert.strictEqual(metricsData.split('#')[0], 'page.views.data:1|c|')

packages/dd-trace/test/encode/coverage-ci-visibility.spec.js

@@ -1,12 +1,6 @@
 'use strict'
 
 const assert = require('node:assert/strict')
-/**
- * @typedef {{
- *   version: number,
- *   coverages: { test_session_id: number, test_suite_id: number, files: { filename: string }[] }[] }
- * } CoverageObject
- */
 
 const { describe, it, beforeEach } = require('mocha')
 const msgpack = require('@msgpack/msgpack')
@@ -17,6 +11,13 @@ const { assertObjectContains } = require('../../../../integration-tests/helpers'
 require('../setup/core')
 const id = require('../../src/id')
 
+/**
+ * @typedef {{
+ *   version: number,
+ *   coverages: { test_session_id: number, test_suite_id: number, files: { filename: string }[] }[] }
+ * } CoverageObject
+ */
+
 describe('coverage-ci-visibility', () => {
   let encoder
   let logger

packages/dd-trace/test/log.spec.js

@@ -1,7 +1,6 @@
 'use strict'
 
 const assert = require('node:assert/strict')
-/* eslint-disable no-console */
 
 const { describe, it, beforeEach, afterEach } = require('mocha')
 const sinon = require('sinon')
@@ -10,6 +9,8 @@ const proxyquire = require('proxyquire')
 require('./setup/core')
 const { storage } = require('../../datadog-core')
 
+/* eslint-disable no-console */
+
 describe('log', () => {
   describe('config', () => {
     let env

packages/dd-trace/test/plugins/util/inferred_proxy.spec.js

@@ -2,7 +2,6 @@
 
 const assert = require('node:assert/strict')
 const { Agent } = require('node:http')
-// Create axios instance with no connection pooling
 
 const { describe, it, afterEach } = require('mocha')
 const axios = require('axios')
@@ -11,6 +10,7 @@ require('../../setup/core')
 const agent = require('../agent')
 const { assertObjectContains } = require('../../../../../integration-tests/helpers')
 
+// Create axios instance with no connection pooling
 const httpClient = axios.create({
   httpAgent: new Agent({ keepAlive: false }),
   timeout: 5000
@@ -22,7 +22,7 @@ describe('Inferred Proxy Spans', function () {
   let controller
   let port
 
-  // tap was throwing timeout errors when trying to use hooks like `before`, so instead we just use this function
+  // Timeout errors occurred when trying to use hooks like `before`, so instead we just use this function
   // and call before the test starts
   const loadTest = async function ({ inferredProxyServicesEnabled = true } = {}) {
     const options = {
@@ -62,7 +62,7 @@ describe('Inferred Proxy Spans', function () {
       })
     })
 
-    return new Promise((resolve, reject) => {
+    return new Promise(/** @type {() => void} */ (resolve, reject) => {
       appListener = server.listen(0, '127.0.0.1', () => {
         port = (/** @type {import('net').AddressInfo} */ (server.address())).port
         appListener._connections = connections
@@ -82,7 +82,7 @@ describe('Inferred Proxy Spans', function () {
         }
       }
 
-      await new Promise((resolve, reject) => {
+      await new Promise(/** @type {() => void} */ (resolve, reject) => {
         appListener.close((err) => {
           if (err) {
             reject(err)

packages/dd-trace/test/runtime_metrics.spec.js

@@ -2,17 +2,15 @@
 
 const assert = require('node:assert')
 const os = require('node:os')
+const { performance } = require('node:perf_hooks')
+const { setImmediate, setTimeout } = require('node:timers/promises')
+const util = require('node:util')
 
 const { describe, it, beforeEach, afterEach } = require('mocha')
 const proxyquire = require('proxyquire')
 const sinon = require('sinon')
 
-const performance = require('node:perf_hooks').performance
-const { setImmediate, setTimeout } = require('node:timers/promises')
-const util = require('node:util')
-
 require('./setup/core')
-
 const { DogStatsDClient } = require('../src/dogstatsd')
 
 function createGarbage (count = 50) {

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

@@ -148,12 +148,12 @@ function withPeerService (tracer, pluginName, spanGenerationFn, service, service
     it('should compute peer service', async () => {
       const useCallback = spanGenerationFn.length === 1
       const spanGenerationPromise = useCallback
-        ? new Promise((resolve, reject) => {
-          const result = spanGenerationFn((err) => err ? reject(err) : resolve(undefined))
+        ? new Promise(/** @type {() => void} */ (resolve, reject) => {
+          const result = spanGenerationFn((err) => err ? reject(err) : resolve())
           // Some callback based methods are a mixture of callback and promise,
           // depending on the module version. Await the promises as well.
           if (util.types.isPromise(result)) {
-            result.then?.(() => resolve(undefined), reject)
+            result.then?.(resolve, reject)
           }
         })
         : spanGenerationFn()
@@ -238,9 +238,7 @@ function withVersions (plugin, modules, range, cb) {
       for (const version of versions) {
         if (process.env.RANGE && !semver.subset(version, process.env.RANGE)) continue
         if (version !== '*') {
-          const result = semver.coerce(version)
-          if (!result) throw new Error(`Invalid version: ${version}`)
-          const min = result.version
+          const min = semver.coerce(version)?.version
           if (!min) throw new Error(`Invalid version: ${version}`)
           testVersions.set(min, { versionRange: version, versionKey: min, resolvedVersion: min })
         }