Skip to content

Commit 837bc52

Browse files
authored
chore(debugger): add JSDoc types to debugger tests (#6962)
1 parent 21673ec commit 837bc52

File tree

12 files changed

+235
-45
lines changed

12 files changed

+235
-45
lines changed

benchmark/sirun/debugger/start-devtools-client.js

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,17 @@
11
'use strict'
22

3+
const assert = require('node:assert')
4+
35
const getConfig = require('../../../packages/dd-trace/src/config')
46
const { start } = require('../../../packages/dd-trace/src/debugger')
57
const { generateProbeConfig } = require('../../../packages/dd-trace/test/debugger/devtools_client/utils')
68

7-
const breakpoint = {
8-
file: process.env.BREAKPOINT_FILE,
9-
line: process.env.BREAKPOINT_LINE
10-
}
9+
const sourceFile = process.env.BREAKPOINT_FILE
10+
const line = Number(process.env.BREAKPOINT_LINE)
11+
assert(sourceFile, 'BREAKPOINT_FILE environment variable must be set')
12+
assert(!Number.isNaN(line), 'BREAKPOINT_LINE environment variable must be a number')
13+
14+
const breakpoint = { sourceFile, line }
1115
const config = getConfig()
1216
const rc = {
1317
setProductHandler (product, cb) {

integration-tests/debugger/re-evaluation.spec.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,9 +114,10 @@ describe('Dynamic Instrumentation Probe Re-Evaluation', function () {
114114
DD_DYNAMIC_INSTRUMENTATION_UPLOAD_INTERVAL_SECONDS: '0',
115115
DD_TRACE_AGENT_PORT: agent.port,
116116
DD_TRACE_DEBUG: process.env.DD_TRACE_DEBUG, // inherit to make debugging the sandbox easier
117-
DD_REMOTE_CONFIG_POLL_INTERVAL_SECONDS: 0.1
117+
DD_REMOTE_CONFIG_POLL_INTERVAL_SECONDS: '0.1'
118118
}
119119
}).then(_proc => {
120+
assert(_proc, 'proc must be spawned successfully')
120121
proc = _proc
121122
// Possible race condition, in case axios.get() is called in the test before it's created here. But we have
122123
// to start the test quickly in order to test the re-evaluation of the probe.

integration-tests/debugger/utils.js

Lines changed: 121 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
'use strict'
22

3+
const assert = require('assert')
34
const { basename, join } = require('path')
45
const { readFileSync } = require('fs')
56
const { randomUUID } = require('crypto')
@@ -12,27 +13,111 @@ const { generateProbeConfig } = require('../../packages/dd-trace/test/debugger/d
1213
const BREAKPOINT_TOKEN = '// BREAKPOINT'
1314
const pollInterval = 0.1
1415

16+
/**
17+
* @typedef {import('../../packages/dd-trace/test/debugger/devtools_client/utils').ProbeConfig} ProbeConfig
18+
*/
19+
20+
/**
21+
* @typedef {typeof import('../../packages/dd-trace/test/debugger/devtools_client/utils').generateProbeConfig}
22+
* GenerateProbeConfigFn
23+
*/
24+
25+
/**
26+
* @typedef {Object} BreakpointInfo
27+
* @property {string} sourceFile
28+
* @property {string} deployedFile
29+
* @property {number} line
30+
* @property {string} url
31+
*/
32+
33+
/**
34+
* A breakpoint with helpers bound for convenient testing.
35+
*
36+
* @typedef {BreakpointInfo & {
37+
* rcConfig: object|null,
38+
* triggerBreakpoint: (url: string) => Promise<import('axios').AxiosResponse<unknown>>,
39+
* generateRemoteConfig: (overrides?: object) => object,
40+
* generateProbeConfig: GenerateProbeConfigFn
41+
* }} EnrichedBreakpoint
42+
*/
43+
44+
/**
45+
* The live‑debugger integration test harness returned by {@link setup}. Provides the spawned app process, fake agent,
46+
* axios client, and helpers to generate remote config and trigger breakpoints.
47+
*
48+
* @typedef {Object} DebuggerTestEnvironment
49+
* @property {BreakpointInfo} breakpoint - Primary breakpoint metadata.
50+
* @property {EnrichedBreakpoint[]} breakpoints - All discovered breakpoints with helpers.
51+
* @property {import('axios').AxiosInstance} axios - HTTP client bound to the test app. Throws if accessed before
52+
* `beforeEach` hook runs.
53+
* @property {string} appFile - Absolute path to the test app entry file. Throws if accessed before `before` hook runs.
54+
* @property {import('../helpers').FakeAgent} agent - Started fake agent instance. Throws if accessed before
55+
* `beforeEach` hook runs.
56+
* @property {import('../helpers').SpawnedProcess} proc - Spawned app process. Throws if accessed before `beforeEach`
57+
* hook runs.
58+
* @property {object|null} rcConfig - Default remote config for the primary breakpoint.
59+
* @property {() => Promise<import('axios').AxiosResponse<unknown>>} triggerBreakpoint - Triggers the primary breakpoint
60+
* once installed.
61+
* @property {(overrides?: object) => object} generateRemoteConfig - Generates RC for the primary breakpoint.
62+
* @property {GenerateProbeConfigFn} generateProbeConfig - Generates probe config for the primary breakpoint.
63+
*/
64+
1565
module.exports = {
1666
pollInterval,
1767
setup
1868
}
1969

70+
/**
71+
* Setup the integration test harness for live‑debugger scenarios.
72+
*
73+
* @param {object} [options] The options for the test environment.
74+
* @param {object} [options.env] The environment variables to set in the test environment.
75+
* @param {string} [options.testApp] The path to the test application file.
76+
* @param {string} [options.testAppSource] The path to the test application source file.
77+
* @param {string[]} [options.dependencies] The dependencies to install in the test environment.
78+
* @param {boolean} [options.silent] Whether to silence the output of the test environment.
79+
* @param {(data: Buffer) => void} [options.stdioHandler] The function to handle the standard output of the test
80+
* environment.
81+
* @param {(data: Buffer) => void} [options.stderrHandler] The function to handle the standard error output of the test
82+
* environment.
83+
* @returns {DebuggerTestEnvironment} Test harness with agent, app process, axios client and breakpoint helpers.
84+
*/
2085
function setup ({ env, testApp, testAppSource, dependencies, silent, stdioHandler, stderrHandler } = {}) {
21-
let cwd
86+
let cwd, axios, appFile, agent, proc
2287

2388
const breakpoints = getBreakpointInfo({
2489
deployedFile: testApp,
2590
sourceFile: testAppSource,
2691
stackIndex: 1 // `1` to disregard the `setup` function
27-
})
92+
}).map((breakpoint) => /** @type {EnrichedBreakpoint} */ ({
93+
rcConfig: null,
94+
triggerBreakpoint: triggerBreakpoint.bind(null, breakpoint.url),
95+
generateRemoteConfig: generateRemoteConfig.bind(null, breakpoint),
96+
generateProbeConfig: generateProbeConfig.bind(null, breakpoint),
97+
...breakpoint
98+
}))
2899

100+
/** @type {DebuggerTestEnvironment} */
29101
const t = {
30102
breakpoint: breakpoints[0],
31103
breakpoints,
32104

33-
axios: null,
34-
appFile: null,
35-
agent: null,
105+
get axios () {
106+
assert(axios, 'axios must be initialized in beforeEach hook')
107+
return axios
108+
},
109+
get appFile () {
110+
assert(appFile, 'appFile must be initialized in before hook')
111+
return appFile
112+
},
113+
get agent () {
114+
assert(agent, 'agent must be initialized in beforeEach hook')
115+
return agent
116+
},
117+
get proc () {
118+
assert(proc, 'proc must be initialized in beforeEach hook')
119+
return proc
120+
},
36121

37122
// Default to the first breakpoint in the file (normally there's only one)
38123
rcConfig: null,
@@ -41,18 +126,13 @@ function setup ({ env, testApp, testAppSource, dependencies, silent, stdioHandle
41126
generateProbeConfig: generateProbeConfig.bind(null, breakpoints[0])
42127
}
43128

44-
// Allow specific access to each breakpoint
45-
for (let i = 0; i < breakpoints.length; i++) {
46-
t.breakpoints[i] = {
47-
rcConfig: null,
48-
triggerBreakpoint: triggerBreakpoint.bind(null, breakpoints[i].url),
49-
generateRemoteConfig: generateRemoteConfig.bind(null, breakpoints[i]),
50-
generateProbeConfig: generateProbeConfig.bind(null, breakpoints[i]),
51-
...breakpoints[i]
52-
}
53-
}
54-
55-
// Trigger the breakpoint once probe is successfully installed
129+
/**
130+
* Trigger the breakpoint once probe is successfully installed
131+
*
132+
* @param {string} url The URL of the HTTP route containing the breakpoint to trigger.
133+
* @returns {Promise<import('axios').AxiosResponse<unknown>>} A promise that resolves with the response from the HTTP
134+
* request after the breakpoint is triggered.
135+
*/
56136
async function triggerBreakpoint (url) {
57137
let triggered = false
58138
return new Promise((resolve, reject) => {
@@ -67,6 +147,13 @@ function setup ({ env, testApp, testAppSource, dependencies, silent, stdioHandle
67147
})
68148
}
69149

150+
/**
151+
* Generate a remote config for a breakpoint
152+
*
153+
* @param {BreakpointInfo} breakpoint - The breakpoint to generate a remote config for.
154+
* @param {object} [overrides] - The overrides to apply to the remote config.
155+
* @returns {object} - The remote config.
156+
*/
70157
function generateRemoteConfig (breakpoint, overrides = {}) {
71158
overrides.id = overrides.id || randomUUID()
72159
return {
@@ -81,7 +168,7 @@ function setup ({ env, testApp, testAppSource, dependencies, silent, stdioHandle
81168
before(function () {
82169
cwd = sandboxCwd()
83170
// The sandbox uses the `integration-tests` folder as its root
84-
t.appFile = join(cwd, 'debugger', breakpoints[0].deployedFile)
171+
appFile = join(cwd, 'debugger', breakpoints[0].deployedFile)
85172
})
86173

87174
beforeEach(async function () {
@@ -90,8 +177,8 @@ function setup ({ env, testApp, testAppSource, dependencies, silent, stdioHandle
90177
// Allow specific access to each breakpoint
91178
t.breakpoints.forEach((breakpoint) => { breakpoint.rcConfig = generateRemoteConfig(breakpoint) })
92179

93-
t.agent = await new FakeAgent().start()
94-
t.proc = await spawnProc(t.appFile, {
180+
agent = await new FakeAgent().start()
181+
proc = await spawnProc(/** @type {string} */ (t.appFile), {
95182
cwd,
96183
env: {
97184
DD_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
@@ -103,7 +190,7 @@ function setup ({ env, testApp, testAppSource, dependencies, silent, stdioHandle
103190
},
104191
silent: silent ?? false
105192
}, stdioHandler, stderrHandler)
106-
t.axios = Axios.create({ baseURL: t.proc.url })
193+
axios = Axios.create({ baseURL: t.proc.url })
107194
})
108195

109196
afterEach(async function () {
@@ -114,10 +201,20 @@ function setup ({ env, testApp, testAppSource, dependencies, silent, stdioHandle
114201
return t
115202
}
116203

204+
/**
205+
* Get breakpoint information from a test file by scanning for BREAKPOINT_TOKEN markers.
206+
*
207+
* @param {Object} [options] - Options for finding breakpoints.
208+
* @param {string} [options.deployedFile] - The deployed file path. If not provided, will be inferred from the stack
209+
* trace.
210+
* @param {string} [options.sourceFile] - The source file path. Defaults to `deployedFile` if not provided.
211+
* @param {number} [options.stackIndex=0] - The stack index to use when inferring the file from the stack trace.
212+
* @returns {BreakpointInfo[]} An array of breakpoint information objects found in the file.
213+
*/
117214
function getBreakpointInfo ({ deployedFile, sourceFile = deployedFile, stackIndex = 0 } = {}) {
118215
if (!deployedFile) {
119216
// First, get the filename of file that called this function
120-
const testFile = new Error().stack
217+
const testFile = /** @type {string} */ (new Error().stack)
121218
.split('\n')[stackIndex + 2] // +2 to skip this function + the first line, which is the error message
122219
.split(' (')[1]
123220
.slice(0, -1)
@@ -127,6 +224,8 @@ function getBreakpointInfo ({ deployedFile, sourceFile = deployedFile, stackInde
127224
deployedFile = sourceFile = join('target-app', basename(testFile).replace('.spec', ''))
128225
}
129226

227+
assert(sourceFile, 'sourceFile must be provided or inferred from stack trace')
228+
130229
// Finally, find the line number(s) of the breakpoint(s)
131230
const lines = readFileSync(join(__dirname, sourceFile), 'utf8').split('\n')
132231
const result = []

integration-tests/helpers/index.js

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,10 @@ function assertTelemetryPoints (pid, msgs, expectedTelemetryPoints) {
164164
}
165165
}
166166

167+
/**
168+
* @typedef {childProcess.ChildProcess & { url: string }} SpawnedProcess
169+
*/
170+
167171
/**
168172
* Spawns a Node.js script in a child process and returns a promise that resolves when the process is ready.
169173
*
@@ -173,14 +177,14 @@ function assertTelemetryPoints (pid, msgs, expectedTelemetryPoints) {
173177
* standard output of the child process. If not provided, the output will be logged to the console.
174178
* @param {(data: Buffer) => void} [stderrHandler] - A function that's called with one data argument to handle the
175179
* standard error of the child process. If not provided, the error will be logged to the console.
176-
* @returns {Promise<childProcess.ChildProcess & { url?: string }|void>} A promise that resolves when the process
177-
* is either ready or terminated without an error. If the process is terminated without an error, the promise will
178-
* resolve with `undefined`.The returned process will have a `url` property if the process didn't terminate.
180+
* @returns {Promise<SpawnedProcess|void>} A promise that resolves when the process is either ready or terminated
181+
* without an error. If the process is terminated without an error, the promise will resolve with `undefined`. The
182+
* returned process will have a `url` property if the process didn't terminate.
179183
*/
180184
function spawnProc (filename, options = {}, stdioHandler, stderrHandler) {
181185
const proc = fork(filename, { ...options, stdio: 'pipe' })
182186

183-
return /** @type {Promise<childProcess.ChildProcess & { url?: string }|void>} */ (new Promise((resolve, reject) => {
187+
return /** @type {Promise<SpawnedProcess|void>} */ (new Promise((resolve, reject) => {
184188
proc
185189
.on('message', ({ port }) => {
186190
if (typeof port !== 'number' && typeof port !== 'string') {

packages/dd-trace/test/debugger/devtools_client/index.spec.js

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
'use strict'
22

3+
const assert = require('node:assert')
4+
35
const { expect } = require('chai')
46
const { describe, it, beforeEach } = require('mocha')
57
const sinon = require('sinon')
@@ -23,7 +25,24 @@ const event = {
2325
}
2426

2527
describe('onPause', function () {
26-
let session, send, onPaused, ackReceived, log
28+
/**
29+
* @typedef {{
30+
* on: sinon.SinonSpy & { args: Array<[string, Function]> },
31+
* post: sinon.SinonSpy,
32+
* emit: sinon.SinonSpy,
33+
* '@noCallThru'?: boolean
34+
* }} MockSession
35+
*/
36+
/** @type {MockSession} */
37+
let session
38+
/** @type {Function} */
39+
let send
40+
/** @type {Function} */
41+
let onPaused
42+
/** @type {sinon.SinonSpy} */
43+
let ackReceived
44+
/** @type {unknown} */
45+
let log
2746

2847
beforeEach(async function () {
2948
ackReceived = sinon.spy()
@@ -82,6 +101,7 @@ describe('onPause', function () {
82101
})
83102

84103
const onPausedCall = session.on.args.find(([event]) => event === 'Debugger.paused')
104+
assert(onPausedCall, 'onPaused call should be found')
85105
onPaused = onPausedCall[1]
86106
})
87107

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ describe('worker thread logger', function () {
2323
{ level: 'debug', args: ['test4'] }
2424
]
2525

26+
// @ts-expect-error - MessagePort has 'on' method at runtime but @types/node doesn't include it
2627
logChannel.port2.on('message', (message) => {
2728
assert.deepStrictEqual(message, expected.shift())
2829
if (expected.length === 0) done()
@@ -42,6 +43,7 @@ describe('worker thread logger', function () {
4243
}
4344
})
4445

46+
// @ts-expect-error - MessagePort has 'on' method at runtime but @types/node doesn't include it
4547
logChannel.port2.on('message', () => {
4648
throw new Error('should not have logged')
4749
})
@@ -62,6 +64,7 @@ describe('worker thread logger', function () {
6264
}
6365
})
6466

67+
// @ts-expect-error - MessagePort has 'on' method at runtime but @types/node doesn't include it
6568
logChannel.port2.on('message', (message) => {
6669
assert.deepStrictEqual(message, { level: 'debug', args: ['logged'] })
6770
done()
@@ -93,6 +96,7 @@ function checkLogLevel (level, expectedLevels) {
9396

9497
const expected = expectedLevels.map((level) => ({ level, args: ['logged'] }))
9598

99+
// @ts-expect-error - MessagePort has 'on' method at runtime but @types/node doesn't include it
96100
logChannel.port2.on('message', (message) => {
97101
assert.deepStrictEqual(message, expected.shift())
98102
if (expected.length === 0) done()

packages/dd-trace/test/debugger/devtools_client/send.spec.js

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,14 @@ const dd = { dd: true }
2727
const snapshot = { snapshot: true }
2828

2929
describe('input message http requests', function () {
30-
let clock, send, request, jsonBuffer
30+
/** @type {sinon.SinonFakeTimers} */
31+
let clock
32+
/** @type {Function} */
33+
let send
34+
/** @type {sinon.SinonSpy} */
35+
let request
36+
/** @type {import('../../../src/debugger/devtools_client/json-buffer')} */
37+
let jsonBuffer
3138

3239
beforeEach(function () {
3340
clock = sinon.useFakeTimers({
@@ -108,6 +115,10 @@ describe('input message http requests', function () {
108115
})
109116
})
110117

118+
/**
119+
* @param {object} [_message] - The message to get the payload for. Defaults to the {@link message} object.
120+
* @returns {object} - The payload.
121+
*/
111122
function getPayload (_message = message) {
112123
return {
113124
ddsource,
Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
'use strict'
22

33
const inspector = require('../../../../src/debugger/devtools_client/inspector_promises_polyfill')
4-
const session = module.exports = new inspector.Session()
4+
const session = module.exports = /** @type {import('../../../../src/debugger/devtools_client/session').CDPSession} */
5+
(new inspector.Session())
56
session.connect()
67

78
session['@noCallThru'] = true

0 commit comments

Comments
 (0)