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.

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

test: Updated tests that relied on tspl by awating the plan.completed instead of calling end to avoid flaky tests #2610

Merged
merged 1 commit into from
Sep 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
28 changes: 10 additions & 18 deletions test/unit/serverless/api-gateway-v2.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ test('should pick up the arn', async (t) => {
assert.equal(agent.collector.metadata.arn, functionContext.invokedFunctionArn)
})

test('should create a web transaction', (t, end) => {
test('should create a web transaction', async (t) => {
const plan = tspl(t, { plan: 8 })
const { agent, lambda, event, functionContext, responseBody } = t.nr
agent.on('transactionFinished', verifyAttributes)
Expand All @@ -90,21 +90,16 @@ test('should create a web transaction', (t, end) => {
plan.equal(agentAttributes['request.uri'], '/my/path')
plan.equal(spanAttributes['request.method'], 'POST')
plan.equal(spanAttributes['request.uri'], '/my/path')

end()
}
await plan.completed
})

test('should set w3c tracecontext on transaction if present on request header', (t, end) => {
test('should set w3c tracecontext on transaction if present on request header', async (t) => {
const plan = tspl(t, { plan: 2 })

const expectedTraceId = '4bf92f3577b34da6a3ce929d0e0e4736'
const traceparent = `00-${expectedTraceId}-00f067aa0ba902b7-00`
const { agent, lambda, event, functionContext, responseBody } = t.nr
agent.on('transactionFinished', () => {
end()
})

agent.config.distributed_tracing.enabled = true
event.headers.traceparent = traceparent

Expand All @@ -124,15 +119,13 @@ test('should set w3c tracecontext on transaction if present on request header',
})

wrappedHandler(event, functionContext, () => {})
await plan.completed
})

test('should add w3c tracecontext to transaction if not present on request header', (t, end) => {
test('should add w3c tracecontext to transaction if not present on request header', async (t) => {
const plan = tspl(t, { plan: 2 })

const { agent, lambda, event, functionContext, responseBody } = t.nr
agent.on('transactionFinished', () => {
end()
})

agent.config.account_id = 'AccountId1'
agent.config.primary_application_id = 'AppId1'
Expand All @@ -152,9 +145,10 @@ test('should add w3c tracecontext to transaction if not present on request heade
})

wrappedHandler(event, functionContext, () => {})
await plan.completed
})

test('should capture request parameters', (t, end) => {
test('should capture request parameters', async (t) => {
const plan = tspl(t, { plan: 5 })

const { agent, lambda, event, functionContext, responseBody } = t.nr
Expand Down Expand Up @@ -182,12 +176,11 @@ test('should capture request parameters', (t, end) => {
plan.equal(spanAttributes['request.parameters.team'], 'node agent')

plan.equal(agentAttributes['request.parameters.parameter1'], 'value1,value2')

end()
}
await plan.completed
})

test('should capture request headers', (t, end) => {
test('should capture request headers', async (t) => {
const plan = tspl(t, { plan: 2 })

const { agent, lambda, event, functionContext, responseBody } = t.nr
Expand All @@ -203,9 +196,8 @@ test('should capture request headers', (t, end) => {

plan.equal(attrs['request.headers.accept'], 'application/json')
plan.equal(attrs['request.headers.header2'], 'value1,value2')

end()
}
await plan.completed
})

test('should not crash when headers are non-existent', (t) => {
Expand Down
4 changes: 2 additions & 2 deletions test/unit/serverless/aws-lambda.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1397,7 +1397,7 @@ test('AwsLambda.patchLambdaHandler', async (t) => {
}
)

await t.test('should record error event when error is thrown', (t, end) => {
await t.test('should record error event when error is thrown', async (t) => {
const plan = tspl(t, { plan: 8 })
const { agent, awsLambda, error, stubEvent, stubContext, stubCallback } = t.nr

Expand Down Expand Up @@ -1427,8 +1427,8 @@ test('AwsLambda.patchLambdaHandler', async (t) => {
if (error.name !== 'SyntaxError') {
throw error
}
end()
}
await plan.completed
})

await t.test('should not end transactions twice', (t, end) => {
Expand Down
8 changes: 4 additions & 4 deletions test/unit/shim/message-shim.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ test('MessageShim', async function (t) {
})
})

await t.test('should insert distributed trace headers in all messages', function (t, end) {
await t.test('should insert distributed trace headers in all messages', async function (t) {
const plan = tspl(t, { plan: 1 })
const { agent, shim, wrappable } = t.nr
const messages = [{}, { headers: { foo: 'foo' } }, {}]
Expand Down Expand Up @@ -418,13 +418,13 @@ test('MessageShim', async function (t) {
}
])
plan.equal(called, 1)
end()
})

helper.runInTransaction(agent, (tx) => {
wrappable.sendMessages()
tx.end()
})
await plan.completed
})

await t.test('should create message broker metrics', function (t, end) {
Expand Down Expand Up @@ -1241,7 +1241,7 @@ test('MessageShim', async function (t) {
})
})

await t.test('should wrap object key of consumer', function (t, end) {
await t.test('should wrap object key of consumer', async function (t) {
const plan = tspl(t, { plan: 4 })
const { shim } = t.nr
const message = { foo: 'bar' }
Expand All @@ -1267,10 +1267,10 @@ test('MessageShim', async function (t) {
const segment = shim.getSegment()
plan.equal(segment.name, 'OtherTransaction/Message/RabbitMQ/Exchange/Named/exchange.foo')
plan.equal(msg, message)
end()
}
}
wrapped(handler)
await plan.completed
})
})
})
68 changes: 28 additions & 40 deletions test/unit/shim/shim.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1184,7 +1184,7 @@ test('Shim', async function (t) {

t.afterEach(afterEach)

await t.test('should make the segment translucent when promise resolves', function (t, end) {
await t.test('should make the segment translucent when promise resolves', async function (t) {
const plan = tspl(t, { plan: 4 })
const { agent, promise, resolve, shim, toWrap } = t.nr
const wrapped = shim.record(toWrap, function () {
Expand All @@ -1196,22 +1196,20 @@ test('Shim', async function (t) {
const ret = wrapped()
plan.ok(ret instanceof Object.getPrototypeOf(promise).constructor)

ret
.then(function (val) {
plan.equal(result, val)
plan.equal(promise.segment.opaque, false)
end()
})
.catch(end)
ret.then(function (val) {
plan.equal(result, val)
plan.equal(promise.segment.opaque, false)
})
})

plan.equal(promise.segment.opaque, true)
setTimeout(function () {
resolve(result)
}, 5)
await plan.completed
})

await t.test('should touch the segment when promise resolves', function (t, end) {
await t.test('should touch the segment when promise resolves', async function (t) {
const plan = tspl(t, { plan: 3 })
const { agent, promise, resolve, shim, toWrap } = t.nr
const wrapped = shim.record(toWrap, function () {
Expand All @@ -1224,21 +1222,19 @@ test('Shim', async function (t) {
const oldDur = promise.segment.timer.getDurationInMillis()
plan.ok(ret instanceof Object.getPrototypeOf(promise).constructor)

ret
.then(function (val) {
plan.equal(result, val)
plan.ok(promise.segment.timer.getDurationInMillis() > oldDur)
end()
})
.catch(end)
ret.then(function (val) {
plan.equal(result, val)
plan.ok(promise.segment.timer.getDurationInMillis() > oldDur)
})
})

setTimeout(function () {
resolve(result)
}, 5)
await plan.completed
})

await t.test('should make the segment translucent when promise rejects', function (t, end) {
await t.test('should make the segment translucent when promise rejects', async function (t) {
const plan = tspl(t, { plan: 4 })
const { agent, promise, reject, shim, toWrap } = t.nr
const wrapped = shim.record(toWrap, function () {
Expand All @@ -1250,27 +1246,25 @@ test('Shim', async function (t) {
const ret = wrapped()
plan.ok(ret instanceof Object.getPrototypeOf(promise).constructor)

ret
.then(
function () {
end(new Error('Should not have resolved!'))
},
function (err) {
plan.equal(err, result)
plan.equal(promise.segment.opaque, false)
end()
}
)
.catch(end)
ret.then(
function () {
throw new Error('Should not have resolved!')
},
function (err) {
plan.equal(err, result)
plan.equal(promise.segment.opaque, false)
}
)
})

plan.equal(promise.segment.opaque, true)
setTimeout(function () {
reject(result)
}, 5)
await plan.completed
})

await t.test('should touch the segment when promise rejects', function (t, end) {
await t.test('should touch the segment when promise rejects', async function (t) {
const plan = tspl(t, { plan: 3 })
const { agent, promise, reject, shim, toWrap } = t.nr
const wrapped = shim.record(toWrap, function () {
Expand All @@ -1288,28 +1282,26 @@ test('Shim', async function (t) {
function (err) {
plan.equal(err, result)
plan.ok(promise.segment.timer.getDurationInMillis() > oldDur)
end()
}
)
})

setTimeout(function () {
reject(result)
}, 5)
await plan.completed
})

await t.test('should not affect unhandledRejection event', async (t) => {
const plan = tspl(t, { plan: 2 })
const { agent, promise, reject, shim, toWrap } = t.nr
const { promise: testPromise, resolve: testResolve } = promiseResolvers()
const result = new Error('unhandled rejection test')

tempOverrideUncaught({
t,
type: tempOverrideUncaught.REJECTION,
handler(err) {
plan.deepEqual(err, result)
testResolve()
}
})

Expand All @@ -1328,13 +1320,12 @@ test('Shim', async function (t) {
reject(result)
}, 5)

await testPromise
await plan.completed
})

await t.test('should call after hook when promise resolves', async (t) => {
const plan = tspl(t, { plan: 7 })
const { agent, promise, resolve, shim, toWrap } = t.nr
const { promise: testPromise, resolve: testResolve } = promiseResolvers()
const segmentName = 'test segment'
const expectedResult = { returned: true }
const wrapped = shim.record(toWrap, function () {
Expand All @@ -1349,7 +1340,6 @@ test('Shim', async function (t) {
plan.equal(error, null)
plan.deepEqual(result, expectedResult)
plan.equal(segment.name, segmentName)
testResolve()
}
})
})
Expand All @@ -1363,13 +1353,12 @@ test('Shim', async function (t) {
resolve(expectedResult)
}, 5)

await testPromise
await plan.completed
})

await t.test('should call after hook when promise reject', async (t) => {
const plan = tspl(t, { plan: 6 })
const { agent, promise, reject, shim, toWrap } = t.nr
const { promise: testPromise, resolve: testResolve } = promiseResolvers()
const segmentName = 'test segment'
const expectedResult = new Error('should call after hook when promise rejects')
const wrapped = shim.record(toWrap, function () {
Expand All @@ -1383,7 +1372,6 @@ test('Shim', async function (t) {
plan.equal(name, toWrap.name)
plan.deepEqual(error, expectedResult)
plan.equal(segment.name, segmentName)
testResolve()
}
})
})
Expand All @@ -1396,7 +1384,7 @@ test('Shim', async function (t) {
setTimeout(function () {
reject(expectedResult)
}, 5)
await testPromise
await plan.completed
})
})

Expand Down
Loading
Loading