Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ci-visibility] Remove usage of .asyncResource in mocha plugin #4348

Merged
merged 1 commit into from
May 27, 2024
+19 −12
Merged

[ci-visibility] Remove usage of .asyncResource in mocha plugin #4348

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/datadog-instrumentations/src/mocha/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ const {
getOnTestEndHandler,
getOnHookEndHandler,
getOnFailHandler,
getOnPendingHandler
getOnPendingHandler,
testFileToSuiteAr
} = require('./utils')
require('./common')

Expand All @@ -39,7 +40,6 @@ let isSuitesSkippingEnabled = false
let earlyFlakeDetectionNumRetries = 0
let knownTests = []
let itrCorrelationId = ''
const testFileToSuiteAr = new Map()
let isForcedToRun = false

// We'll preserve the original coverage here
Expand Down Expand Up @@ -469,7 +469,7 @@ addHook({
// Used to start and finish test session and test module
addHook({
name: 'mocha',
versions: ['>=8.0.0'],
versions: ['>=5.2.0'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a confirmation, you're making the version support lower than it was before (it's usually not a common thing so i'm just pointing it out)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh yeah, I should've mentioned this. This file is not there for <8 so this should be a noop. I changed it because the testing matrix became way more complicated. We should probably fix our testing setup but that's for another PR 😄

file: 'lib/nodejs/parallel-buffered-runner.js'
}, (ParallelBufferedRunner, frameworkVersion) => {
shimmer.wrap(ParallelBufferedRunner.prototype, 'run', run => function () {
Expand Down
15 changes: 7 additions & 8 deletions packages/datadog-instrumentations/src/mocha/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const testToAr = new WeakMap()
const originalFns = new WeakMap()
const testToStartLine = new WeakMap()
const testFileToSuiteAr = new Map()
const wrappedFunctions = new WeakSet()

function isNewTest (test, knownTests) {
const testSuite = getTestSuitePath(test.file, process.cwd())
Expand Down Expand Up @@ -87,7 +88,7 @@ function getTestAsyncResource (test) {
if (!test.fn) {
return testToAr.get(test)
}
if (!test.fn.asyncResource) {
if (!wrappedFunctions.has(test.fn)) {
return testToAr.get(test.fn)
}
const originalFn = originalFns.get(test.fn)
Expand All @@ -105,9 +106,10 @@ function runnableWrapper (RunnablePackage) {
const isTestHook = isBeforeEach || isAfterEach

// we restore the original user defined function
if (this.fn.asyncResource) {
if (wrappedFunctions.has(this.fn)) {
const originalFn = originalFns.get(this.fn)
this.fn = originalFn
wrappedFunctions.delete(this.fn)
}

if (isTestHook || this.type === 'test') {
Expand All @@ -122,11 +124,7 @@ function runnableWrapper (RunnablePackage) {
originalFns.set(newFn, this.fn)
this.fn = newFn

// Temporarily keep functionality when .asyncResource is removed from node
// in https://github.com/nodejs/node/pull/46432
if (!this.fn.asyncResource) {
this.fn.asyncResource = asyncResource
}
wrappedFunctions.add(this.fn)
}
}

Expand Down Expand Up @@ -303,5 +301,6 @@ module.exports = {
getOnTestEndHandler,
getOnHookEndHandler,
getOnFailHandler,
getOnPendingHandler
getOnPendingHandler,
testFileToSuiteAr
}
2 changes: 2 additions & 0 deletions packages/datadog-plugin-mocha/test/index.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ describe('Plugin', () => {
mocha.addFile(testFilePath)
mocha.run()
})

it('works with failing tests', (done) => {
const testFilePath = path.join(__dirname, 'mocha-test-fail.js')
const testSuite = testFilePath.replace(`${process.cwd()}/`, '')
Expand Down Expand Up @@ -178,6 +179,7 @@ describe('Plugin', () => {
mocha.addFile(testFilePath)
mocha.run()
})

it('works with skipping tests', (done) => {
const testFilePath = path.join(__dirname, 'mocha-test-skip.js')
const testNames = [
Expand Down
8 changes: 7 additions & 1 deletion packages/dd-trace/test/setup/mocha.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,11 @@ function loadInst (plugin) {
loadInstFile(`${plugin}/server.js`, instrumentations)
loadInstFile(`${plugin}/client.js`, instrumentations)
} catch (e) {
loadInstFile(`${plugin}.js`, instrumentations)
try {
loadInstFile(`${plugin}/main.js`, instrumentations)
} catch (e) {
loadInstFile(`${plugin}.js`, instrumentations)
}
}

return instrumentations
Expand Down Expand Up @@ -143,10 +147,12 @@ function withNamingSchema (
function withPeerService (tracer, pluginName, spanGenerationFn, service, serviceSource, opts = {}) {
describe('peer service computation' + (opts.desc ? ` ${opts.desc}` : ''), () => {
let computePeerServiceSpy

beforeEach(() => {
const plugin = tracer()._pluginManager._pluginsByName[pluginName]
computePeerServiceSpy = sinon.stub(plugin._tracerConfig, 'spanComputePeerService').value(true)
})

afterEach(() => {
computePeerServiceSpy.restore()
})
Expand Down
Loading