Skip to content

Commit 4d5ba6c

Browse files
committed
ci(scripts): speed up verify-exercised-tests and close detection gaps (#9542)
* test(scripts): cover verify-exercised-tests detection The checker's failure branches are unpinned, so a rework can narrow what it detects while still exiting 0 on a healthy tree. This adds fixture checkouts that drive each branch. `main` takes the repository root as an optional argument so a fixture can stand in for the script's own checkout. * ci(scripts): answer test-file globs from one repository walk verify-exercised-tests expanded its script globs with 227 separate `globSync` traversals of the same tree. Walking once and matching the patterns in memory cuts the run from 312 ms to 178 ms, medians of ten interleaved runs on one checkout. Matching has to stay identical to the walk it replaces, so the index compiles patterns with the options `glob` derives internally. Its spec cross-checks every result against `globSync`, over hand-written pattern shapes and over the glob corpus this repository's own package.json produces. Answering with too few files would let an unexercised spec pass unseen, so the index refuses instead: an unreadable directory warns, and a `..` segment, which needs a real tree to resolve against, throws. Literal path segments are compared case-sensitively on every platform, the rule `glob` applies on Linux, so a result no longer depends on whether the developer's filesystem folds case. * ci(scripts): resolve workflow steps into one ordered event stream Two ways a suite could go unverified survived the checker, both because a job's steps were never modeled as one ordered, per-leg-resolved sequence. 1. A coverage upload placed before the suite it collects passed the check, because uploads were tracked as a per-job boolean and position never entered it. Five composite actions run a suite and upload its report, so the order that decides whether a report survives usually lives inside the action, not in the workflow. 2. `SPEC: ${{ matrix.spec }}` reached the glob unresolved, degrading `${SPEC:-*}` to `*` and making every spec in a directory look exercised by every matrix leg. Resolving it per leg surfaces packages/datadog-plugin-aws-sdk/test/base-inject-field.spec.js, which no leg selects; the serverless matrix gains the entry that runs it. One traversal now yields the `run` and `upload` events a step contributes. The second traversal that answered only "does this job upload?" is gone, and with it the missing `using: composite` guard that made it throw on a JavaScript or Docker action. Such an action is reported now, so an upload hidden inside one cannot read as "this job never uploads".
1 parent ef0ee0b commit 4d5ba6c

7 files changed

Lines changed: 1370 additions & 261 deletions

File tree

.github/CODEOWNERS

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,10 @@
279279

280280
/scripts/generate-config-types.js @DataDog/apm-sdk-capabilities-js
281281
/scripts/generate-supported-integrations.js @DataDog/apm-sdk-capabilities-js
282+
/scripts/helpers/test-file-index.js @DataDog/apm-sdk-capabilities-js
283+
/scripts/helpers/test-file-index.spec.js @DataDog/apm-sdk-capabilities-js
282284
/scripts/verify-exercised-tests.js @DataDog/apm-sdk-capabilities-js
285+
/scripts/verify-exercised-tests.spec.mjs @DataDog/apm-sdk-capabilities-js
283286
/supported_versions_output.json @DataDog/apm-sdk-capabilities-js
284287
/supported_versions_table.csv @DataDog/apm-sdk-capabilities-js
285288

.github/workflows/serverless.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ jobs:
5252
spec:
5353
- client
5454
- aws-sdk
55+
- base-inject-field
5556
- bedrockruntime
5657
- dynamodb
5758
- eventbridge

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,7 @@
222222
"istanbul-lib-report": "^3.0.0",
223223
"istanbul-reports": "^3.0.2",
224224
"jszip": "^3.10.1",
225+
"minimatch": "^10.2.5",
225226
"mocha": "^11.7.6",
226227
"mocha-junit-reporter": "^2.2.1",
227228
"mocha-multi-reporters": "^1.5.1",

scripts/helpers/test-file-index.js

Lines changed: 288 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,288 @@
1+
'use strict'
2+
3+
const fs = require('node:fs')
4+
const path = require('node:path')
5+
6+
const { Glob } = require('glob')
7+
const { Minimatch } = require('minimatch')
8+
9+
const TEST_FILE_GLOB = '**/*.@(spec|test).@(js|mjs|cjs)'
10+
11+
// Case-insensitive because `glob` matches case-insensitively on darwin and win32. Indexing a
12+
// `FOO.SPEC.JS` that a case-sensitive platform would reject is harmless: the pattern matcher
13+
// applies that platform's own rules and drops it again.
14+
const TEST_FILE_NAME = /\.(?:spec|test)\.(?:js|mjs|cjs)$/i
15+
16+
// Any character that can start a wildcard, brace, class, or extglob. Over-reporting only shortens
17+
// the literal prefix, which widens the candidate set and can never drop a match.
18+
const PATTERN_MAGIC = /[*?[\]{}()!+@\\]/
19+
20+
// `glob` walks `..` on the real filesystem before matching. The index has no tree to walk, so it
21+
// would answer such a pattern with too few files and report exercised specs as unexercised.
22+
const PARENT_SEGMENT = /(?:^|\/)\.\.(?:\/|$)/
23+
24+
/**
25+
* @param {string} repoRoot
26+
* @returns {string[]}
27+
*/
28+
function walkTestFiles (repoRoot) {
29+
/** @type {string[]} */
30+
const found = []
31+
/** @type {string[]} */
32+
const pending = ['']
33+
34+
while (pending.length) {
35+
const relativeDir = pending.pop()
36+
37+
/** @type {import('node:fs').Dirent[]} */
38+
let entries
39+
try {
40+
entries = fs.readdirSync(path.join(repoRoot, relativeDir), { withFileTypes: true })
41+
} catch (error) {
42+
// `glob` skips unreadable directories without a word. Staying silent here would shrink the
43+
// index instead, and an unexercised spec below that directory would pass the check unseen.
44+
process.stderr.write(`test-file-index: skipped unreadable ${relativeDir || '.'} (${error.code})\n`)
45+
continue
46+
}
47+
48+
for (const entry of entries) {
49+
const { name } = entry
50+
51+
// `glob` runs with `dot: false`, so a dot entry can never satisfy TEST_FILE_GLOB. Every
52+
// consumer of this index also ignores `node_modules`, so descending into it is dead work.
53+
if (name.startsWith('.') || name === 'node_modules') continue
54+
55+
const relativePath = relativeDir ? `${relativeDir}/${name}` : name
56+
57+
if (entry.isDirectory()) {
58+
pending.push(relativePath)
59+
} else if (TEST_FILE_NAME.test(name)) {
60+
// Symlinks are kept: `nodir: true` filters directories, not links to files.
61+
found.push(relativePath)
62+
}
63+
}
64+
}
65+
66+
return found
67+
}
68+
69+
/**
70+
* Drops `.` and empty path segments the way `glob` does while building its pattern. `minimatch`
71+
* on a bare string keeps them and would then compare one segment too many.
72+
*
73+
* @param {string} pattern
74+
* @returns {string}
75+
*/
76+
function normalizePattern (pattern) {
77+
if (!pattern.includes('./') && !pattern.includes('//')) return pattern
78+
79+
const segments = pattern.split('/')
80+
const kept = []
81+
82+
for (let i = 0; i < segments.length; i++) {
83+
const segment = segments[i]
84+
// A leading empty segment marks an absolute pattern and a trailing one a directory-only
85+
// pattern; both change what matches, so only interior blanks are collapsed.
86+
if (segment === '.' || (segment === '' && i !== 0 && i !== segments.length - 1)) continue
87+
kept.push(segment)
88+
}
89+
90+
return kept.join('/')
91+
}
92+
93+
/**
94+
* Longest leading directory path of `pattern` that contains no wildcard.
95+
*
96+
* @param {string} pattern
97+
* @returns {string}
98+
*/
99+
function literalPrefix (pattern) {
100+
const magic = pattern.search(PATTERN_MAGIC)
101+
const literal = magic === -1 ? pattern : pattern.slice(0, magic)
102+
const lastSlash = literal.lastIndexOf('/')
103+
return lastSlash === -1 ? '' : literal.slice(0, lastSlash + 1)
104+
}
105+
106+
/**
107+
* In-memory stand-in for repeated `globSync` walks over the repository's test files.
108+
*
109+
* The repository is traversed once; every later pattern is answered from the resulting list. Only
110+
* paths named like a test file are indexed, so a pattern that also selects non-test files reports
111+
* just the test-file subset.
112+
*
113+
* Results have to stay identical to the `globSync` walk they replace; `test-file-index.spec.js`
114+
* pins that against `glob` itself, including the repository's own pattern corpus.
115+
*/
116+
class TestFileIndex {
117+
/** @type {string[]} */
118+
files
119+
120+
/** @type {boolean} */
121+
#nocase
122+
123+
/** @type {NodeJS.Platform} */
124+
#platform
125+
126+
/** @type {Map<string, InstanceType<typeof Minimatch>>} */
127+
#matchers = new Map()
128+
129+
/** @type {Map<string, InstanceType<typeof Minimatch>>} */
130+
#ignoreMatchers = new Map()
131+
132+
/** @type {Map<string, { files: string[], folded: string[] }>} */
133+
#scopes = new Map()
134+
135+
/** @type {Map<string, string[]>} */
136+
#candidates = new Map()
137+
138+
/**
139+
* @param {string[]} files Sorted repository-relative POSIX paths.
140+
* @param {{ nocase: boolean, platform: NodeJS.Platform }} options
141+
*/
142+
constructor (files, { nocase, platform }) {
143+
this.files = files
144+
this.#nocase = nocase
145+
this.#platform = platform
146+
}
147+
148+
/**
149+
* A literal path segment is compared case-sensitively on every platform, which is what `glob`
150+
* does on Linux. On a case-insensitive filesystem `glob` instead resolves `foo/BAR.spec.js` to
151+
* the differently-spelled file on disk; reproducing that would make the result depend on the
152+
* developer's machine, so the index keeps the Linux rule everywhere.
153+
*
154+
* @param {string} pattern
155+
* @param {string[]} [ignoreGlobs]
156+
* @throws {Error} If `pattern` contains a `..` segment, which the index cannot resolve.
157+
* @returns {string[]} Matching paths, in the index's sort order.
158+
*/
159+
match (pattern, ignoreGlobs = []) {
160+
if (PARENT_SEGMENT.test(pattern)) {
161+
throw new Error(`test-file-index cannot resolve the '..' segment in pattern '${pattern}'`)
162+
}
163+
164+
const normalized = normalizePattern(pattern)
165+
const scope = this.#scope(ignoreGlobs)
166+
const matcher = this.#matcher(normalized)
167+
168+
/** @type {string[]} */
169+
const matched = []
170+
for (const file of this.#candidatesFor(scope, ignoreGlobs, normalized)) {
171+
// eslint-disable-next-line unicorn/prefer-regexp-test -- Minimatch#match, not String#match.
172+
if (matcher.match(file)) matched.push(file)
173+
}
174+
175+
return matched
176+
}
177+
178+
/**
179+
* @param {string[]} ignoreGlobs
180+
* @returns {{ files: string[], folded: string[] }}
181+
*/
182+
#scope (ignoreGlobs) {
183+
const key = ignoreGlobs.join('\0')
184+
185+
let scope = this.#scopes.get(key)
186+
if (scope === undefined) {
187+
const matchers = ignoreGlobs.map(glob => this.#ignoreMatcher(glob))
188+
const files = matchers.length === 0
189+
? this.files
190+
: this.files.filter(file => !matchers.some(matcher => matcher.match(file) || matcher.match(`${file}/`)))
191+
192+
scope = { files, folded: this.#nocase ? files.map(file => file.toLowerCase()) : files }
193+
this.#scopes.set(key, scope)
194+
}
195+
196+
return scope
197+
}
198+
199+
/**
200+
* @param {{ files: string[], folded: string[] }} scope
201+
* @param {string[]} ignoreGlobs
202+
* @param {string} pattern
203+
* @returns {string[]}
204+
*/
205+
#candidatesFor (scope, ignoreGlobs, pattern) {
206+
const prefix = literalPrefix(pattern)
207+
if (prefix === '') return scope.files
208+
209+
const key = `${ignoreGlobs.join('\0')}\0${prefix}`
210+
211+
let candidates = this.#candidates.get(key)
212+
if (candidates === undefined) {
213+
const wanted = this.#nocase ? prefix.toLowerCase() : prefix
214+
215+
candidates = []
216+
for (let i = 0; i < scope.files.length; i++) {
217+
if (scope.folded[i].startsWith(wanted)) candidates.push(scope.files[i])
218+
}
219+
220+
this.#candidates.set(key, candidates)
221+
}
222+
223+
return candidates
224+
}
225+
226+
/**
227+
* @param {string} pattern
228+
* @returns {InstanceType<typeof Minimatch>}
229+
*/
230+
#matcher (pattern) {
231+
let matcher = this.#matchers.get(pattern)
232+
if (matcher === undefined) {
233+
matcher = new Minimatch(pattern, {
234+
dot: false,
235+
nocase: this.#nocase,
236+
nocaseMagicOnly: this.#platform === 'darwin' || this.#platform === 'win32',
237+
nocomment: true,
238+
nonegate: true,
239+
optimizationLevel: 2,
240+
platform: this.#platform,
241+
windowsPathsNoEscape: true,
242+
})
243+
this.#matchers.set(pattern, matcher)
244+
}
245+
246+
return matcher
247+
}
248+
249+
/**
250+
* @param {string} glob
251+
* @returns {InstanceType<typeof Minimatch>}
252+
*/
253+
#ignoreMatcher (glob) {
254+
let matcher = this.#ignoreMatchers.get(glob)
255+
if (matcher === undefined) {
256+
// Mirrors the options `glob`'s Ignore class builds; notably `dot: true`, so `**/.git/**`
257+
// still matches paths that the main pattern would skip.
258+
matcher = new Minimatch(glob, {
259+
dot: true,
260+
nocase: this.#nocase,
261+
nocomment: true,
262+
nonegate: true,
263+
optimizationLevel: 2,
264+
platform: this.#platform,
265+
})
266+
this.#ignoreMatchers.set(glob, matcher)
267+
}
268+
269+
return matcher
270+
}
271+
}
272+
273+
/**
274+
* @param {string} repoRoot
275+
* @returns {TestFileIndex}
276+
*/
277+
function createTestFileIndex (repoRoot) {
278+
// `glob` decides case sensitivity from the platform's filesystem; read it back rather than
279+
// re-deriving it here, so the index cannot drift from the walker it replaces.
280+
const probe = new Glob(TEST_FILE_GLOB, { cwd: repoRoot, windowsPathsNoEscape: true })
281+
282+
const files = walkTestFiles(repoRoot)
283+
files.sort((a, b) => a.localeCompare(b, 'en'))
284+
285+
return new TestFileIndex(files, { nocase: probe.nocase, platform: probe.platform })
286+
}
287+
288+
module.exports = { TEST_FILE_GLOB, TestFileIndex, createTestFileIndex }

0 commit comments

Comments
 (0)