Skip to content

Commit 5468eb6

Browse files
committed
ci(coverage): isolate patched Bun dependencies
Bun's Linux hardlink backend let in-place patches mutate its package cache, so vendor bundling embedded patched Istanbul while test sandboxes used a pristine copy. Replace only local files with fresh inodes and never resolve patch targets from the parent application.
1 parent b73eeaa commit 5468eb6

5 files changed

Lines changed: 208 additions & 10 deletions

File tree

scripts/patch-istanbul-lib-coverage.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@
3030
const fs = require('node:fs')
3131
const path = require('node:path')
3232

33+
const { replaceFile } = require('./replace-file')
34+
3335
// Inline marker so the script can detect a previous run without parsing the
3436
// whole replacement body. Bump the version suffix when the patch body changes.
3537
const SENTINEL = '// dd-trace-js patch v2: fold fnMap/branchMap into getLineCoverage'
@@ -142,10 +144,8 @@ for (const marker of requiredMarkers) {
142144
}
143145
}
144146

145-
let targetFile
146-
try {
147-
targetFile = require.resolve('istanbul-lib-coverage/lib/file-coverage.js', { paths: [repoRoot] })
148-
} catch {
147+
const targetFile = path.join(repoRoot, 'node_modules', 'istanbul-lib-coverage', 'lib', 'file-coverage.js')
148+
if (!fs.existsSync(targetFile)) {
149149
log('skipping: istanbul-lib-coverage is not installed yet')
150150
return
151151
}
@@ -179,5 +179,5 @@ if (current !== ORIGINAL && !current.includes(PATCH_MARKER)) {
179179
return
180180
}
181181

182-
fs.writeFileSync(targetFile, source.replace(current, REPLACEMENT))
182+
replaceFile(targetFile, source.replace(current, REPLACEMENT))
183183
log(`patched ${relativeTarget}`)

scripts/patch-v8-to-istanbul.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@
4242
const fs = require('node:fs')
4343
const path = require('node:path')
4444

45+
const { replaceFile } = require('./replace-file')
46+
4547
const LINE_SENTINEL = '// dd-trace-js patch v1: record firstColumn for the line-zeroing guard'
4648
const APPLY_SENTINEL = '// dd-trace-js patch v1: zero lines covered from first non-whitespace column'
4749
const PATCH_MARKER = '// dd-trace-js patch'
@@ -137,10 +139,8 @@ for (const marker of requiredMarkers) {
137139
* @returns {boolean} whether the file is now patched
138140
*/
139141
function applyPatch (relTarget, sentinel, re, original, replacement) {
140-
let targetFile
141-
try {
142-
targetFile = require.resolve(relTarget, { paths: [repoRoot] })
143-
} catch {
142+
const targetFile = path.join(repoRoot, 'node_modules', relTarget)
143+
if (!fs.existsSync(targetFile)) {
144144
log(`skipping: ${relTarget} is not installed yet`)
145145
return false
146146
}
@@ -171,7 +171,7 @@ function applyPatch (relTarget, sentinel, re, original, replacement) {
171171
return false
172172
}
173173

174-
fs.writeFileSync(targetFile, source.replace(current, replacement))
174+
replaceFile(targetFile, source.replace(current, replacement))
175175
log(`patched ${relativeTarget}`)
176176
return true
177177
}

scripts/replace-file.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
'use strict'
2+
3+
const fs = require('node:fs')
4+
const path = require('node:path')
5+
6+
/**
7+
* @param {string} filename
8+
* @param {string|Buffer} content
9+
*/
10+
function replaceFile (filename, content) {
11+
const temporaryFile = path.join(
12+
path.dirname(filename),
13+
`.${path.basename(filename)}.${process.pid}.tmp`
14+
)
15+
16+
try {
17+
const { mode } = fs.statSync(filename)
18+
// A fresh inode keeps Bun's Linux hardlink cache immutable.
19+
fs.writeFileSync(temporaryFile, content, { mode })
20+
fs.renameSync(temporaryFile, filename)
21+
} finally {
22+
fs.rmSync(temporaryFile, { force: true })
23+
}
24+
}
25+
26+
module.exports = { replaceFile }
Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
'use strict'
2+
3+
const assert = require('node:assert/strict')
4+
const { execFileSync } = require('node:child_process')
5+
const fs = require('node:fs')
6+
const os = require('node:os')
7+
const path = require('node:path')
8+
9+
const repoRoot = path.resolve(__dirname, '..', '..')
10+
const relativeTarget = path.join('lib', 'file-coverage.js')
11+
const rootPatchScript = path.join(repoRoot, 'scripts', 'patch-istanbul-lib-coverage.js')
12+
const rootInstalledTarget = path.join(repoRoot, 'node_modules', 'istanbul-lib-coverage', relativeTarget)
13+
14+
/**
15+
* @param {string} fixtureRoot
16+
*/
17+
function createSourceFixture (fixtureRoot) {
18+
fs.mkdirSync(path.join(fixtureRoot, 'scripts'), { recursive: true })
19+
fs.mkdirSync(path.join(fixtureRoot, 'packages', 'datadog-instrumentations'), { recursive: true })
20+
fs.mkdirSync(path.join(fixtureRoot, 'integration-tests', 'coverage'), { recursive: true })
21+
fs.writeFileSync(path.join(fixtureRoot, 'eslint.config.mjs'), '')
22+
fs.writeFileSync(path.join(fixtureRoot, 'integration-tests', 'coverage', 'merge-lcov.js'), '')
23+
fs.copyFileSync(
24+
rootPatchScript,
25+
path.join(fixtureRoot, 'scripts', 'patch-istanbul-lib-coverage.js')
26+
)
27+
fs.copyFileSync(
28+
path.join(repoRoot, 'scripts', 'replace-file.js'),
29+
path.join(fixtureRoot, 'scripts', 'replace-file.js')
30+
)
31+
}
32+
33+
describe('patch-istanbul-lib-coverage', function () {
34+
this.timeout(60_000)
35+
36+
let fixtureDirectory
37+
38+
beforeEach(() => {
39+
fixtureDirectory = fs.mkdtempSync(path.join(os.tmpdir(), 'dd-istanbul-patch-'))
40+
createSourceFixture(fixtureDirectory)
41+
fs.writeFileSync(path.join(fixtureDirectory, 'package.json'), JSON.stringify({
42+
dependencies: {
43+
'istanbul-lib-coverage': '3.2.2',
44+
},
45+
}))
46+
})
47+
48+
afterEach(() => {
49+
fs.rmSync(fixtureDirectory, { recursive: true, force: true })
50+
})
51+
52+
it('does not mutate Bun hardlink cache entries', async () => {
53+
const cachedTarget = path.join(fixtureDirectory, 'istanbul-lib-coverage-cache.js')
54+
const backupTarget = `${rootInstalledTarget}.backup-${process.pid}`
55+
fs.mkdirSync(path.dirname(cachedTarget), { recursive: true })
56+
const sourceTarget = path.join(
57+
repoRoot, 'vendor', 'node_modules', 'istanbul-lib-coverage', relativeTarget
58+
)
59+
fs.copyFileSync(sourceTarget, cachedTarget)
60+
fs.renameSync(rootInstalledTarget, backupTarget)
61+
fs.linkSync(cachedTarget, rootInstalledTarget)
62+
const originalCacheSource = fs.readFileSync(cachedTarget, 'utf8')
63+
64+
try {
65+
delete require.cache[rootPatchScript]
66+
require(rootPatchScript)
67+
68+
assert.match(fs.readFileSync(rootInstalledTarget, 'utf8'), /dd-trace-js patch v2/)
69+
assert.strictEqual(await Promise.resolve(fs.readFileSync(cachedTarget, 'utf8')), originalCacheSource)
70+
} finally {
71+
fs.rmSync(rootInstalledTarget, { force: true })
72+
fs.renameSync(backupTarget, rootInstalledTarget)
73+
delete require.cache[rootPatchScript]
74+
}
75+
})
76+
77+
it('skips when the package is not installed locally', () => {
78+
const backupTarget = `${rootInstalledTarget}.backup-${process.pid}`
79+
fs.renameSync(rootInstalledTarget, backupTarget)
80+
81+
try {
82+
delete require.cache[rootPatchScript]
83+
require(rootPatchScript)
84+
assert.strictEqual(fs.existsSync(rootInstalledTarget), false)
85+
} finally {
86+
fs.renameSync(backupTarget, rootInstalledTarget)
87+
delete require.cache[rootPatchScript]
88+
}
89+
})
90+
91+
it('does not patch dependencies from the parent application', async () => {
92+
const packageRoot = path.join(fixtureDirectory, 'node_modules', 'dd-trace')
93+
createSourceFixture(packageRoot)
94+
95+
const parentTarget = path.join(fixtureDirectory, 'node_modules', 'istanbul-lib-coverage', relativeTarget)
96+
fs.mkdirSync(path.dirname(parentTarget), { recursive: true })
97+
const sourceTarget = path.join(
98+
repoRoot, 'vendor', 'node_modules', 'istanbul-lib-coverage', relativeTarget
99+
)
100+
fs.copyFileSync(sourceTarget, parentTarget)
101+
const originalParentSource = fs.readFileSync(parentTarget, 'utf8')
102+
103+
execFileSync(process.execPath, ['scripts/patch-istanbul-lib-coverage.js'], {
104+
cwd: packageRoot,
105+
})
106+
107+
assert.strictEqual(
108+
await Promise.resolve(fs.readFileSync(parentTarget, 'utf8')),
109+
originalParentSource
110+
)
111+
})
112+
})
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
'use strict'
2+
3+
const assert = require('node:assert/strict')
4+
const fs = require('node:fs')
5+
const path = require('node:path')
6+
7+
const { replaceFile } = require('../replace-file')
8+
9+
const repoRoot = path.resolve(__dirname, '..', '..')
10+
const patchScript = path.join(repoRoot, 'scripts', 'patch-v8-to-istanbul.js')
11+
const lineTarget = path.join(repoRoot, 'node_modules', 'v8-to-istanbul', 'lib', 'line.js')
12+
const applyTarget = path.join(repoRoot, 'node_modules', 'v8-to-istanbul', 'lib', 'v8-to-istanbul.js')
13+
const lineOriginal = ` // we start with all lines having been executed, and work
14+
// backwards zeroing out lines based on V8 output.
15+
this.count = 1`
16+
const applyOriginal = ` if (startCol <= line.startCol && endCol >= line.endCol && !line.ignore) {
17+
line.count = range.count
18+
}`
19+
20+
describe('patch-v8-to-istanbul', () => {
21+
let originalLineSource
22+
let originalApplySource
23+
24+
beforeEach(() => {
25+
originalLineSource = fs.readFileSync(lineTarget)
26+
originalApplySource = fs.readFileSync(applyTarget)
27+
})
28+
29+
afterEach(() => {
30+
if (fs.existsSync(lineTarget)) {
31+
replaceFile(lineTarget, originalLineSource)
32+
} else {
33+
fs.writeFileSync(lineTarget, originalLineSource)
34+
}
35+
replaceFile(applyTarget, originalApplySource)
36+
delete require.cache[patchScript]
37+
})
38+
39+
it('patches both local coverage files', () => {
40+
replaceFile(lineTarget, lineOriginal)
41+
replaceFile(applyTarget, applyOriginal)
42+
43+
delete require.cache[patchScript]
44+
require(patchScript)
45+
46+
assert.match(fs.readFileSync(lineTarget, 'utf8'), /record firstColumn/)
47+
assert.match(fs.readFileSync(applyTarget, 'utf8'), /zero lines covered from first non-whitespace column/)
48+
})
49+
50+
it('skips a missing local coverage file', () => {
51+
fs.rmSync(lineTarget)
52+
replaceFile(applyTarget, applyOriginal)
53+
54+
delete require.cache[patchScript]
55+
require(patchScript)
56+
57+
assert.strictEqual(fs.existsSync(lineTarget), false)
58+
assert.match(fs.readFileSync(applyTarget, 'utf8'), /zero lines covered from first non-whitespace column/)
59+
})
60+
})

0 commit comments

Comments
 (0)