Skip to content

Commit 92cb702

Browse files
authored
tests: fix race in apm tracing tests (#47)
If we lost the race we would end up waiting on app.on('close') that had already fired. Fixes: #46
1 parent 2c30aba commit 92cb702

File tree

3 files changed

+38
-12
lines changed

3 files changed

+38
-12
lines changed

loggers/morgan/test/apm.test.js

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ const validate = ajv.compile(require('../../../utils/schema.json'))
4141
test('tracing integration works', t => {
4242
let apmServer
4343
let app
44+
let appIsClosed = false
4445
const traceObjs = []
4546
const logObjs = []
4647
let stderr = ''
@@ -97,6 +98,7 @@ test('tracing integration works', t => {
9798
app.on('close', function (code) {
9899
t.equal(stderr, '', 'empty stderr from app')
99100
t.equal(code, 0, 'app exited 0')
101+
appIsClosed = true
100102
})
101103
}
102104

@@ -139,11 +141,17 @@ test('tracing integration works', t => {
139141
}
140142

141143
function finish () {
142-
app.on('close', function () {
144+
if (appIsClosed) {
143145
apmServer.close(function () {
144146
t.end()
145147
})
146-
})
148+
} else {
149+
app.on('close', function () {
150+
apmServer.close(function () {
151+
t.end()
152+
})
153+
})
154+
}
147155
}
148156

149157
step1StartMockApmServer(function onListening (apmServerErr, apmServerUrl) {

loggers/pino/test/apm.test.js

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ const validate = ajv.compile(require('../../../utils/schema.json'))
4141
test('tracing integration works', t => {
4242
let apmServer
4343
let app
44+
let appIsClosed = false
4445
const traceObjs = []
4546
const logObjs = []
4647
let stderr = ''
@@ -97,6 +98,7 @@ test('tracing integration works', t => {
9798
app.on('close', function (code) {
9899
t.equal(stderr, '', 'empty stderr from app')
99100
t.equal(code, 0, 'app exited 0')
101+
appIsClosed = true
100102
})
101103
}
102104

@@ -137,11 +139,17 @@ test('tracing integration works', t => {
137139
}
138140

139141
function finish () {
140-
app.on('close', function () {
142+
if (appIsClosed) {
141143
apmServer.close(function () {
142144
t.end()
143145
})
144-
})
146+
} else {
147+
app.on('close', function () {
148+
apmServer.close(function () {
149+
t.end()
150+
})
151+
})
152+
}
145153
}
146154

147155
step1StartMockApmServer(function onListening (apmServerErr, apmServerUrl) {

loggers/winston/test/apm.test.js

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ const validate = ajv.compile(require('../../../utils/schema.json'))
4141
test('tracing integration works', t => {
4242
let apmServer
4343
let app
44+
let appIsClosed = false
4445
const traceObjs = []
4546
const logObjs = []
4647
let stderr = ''
@@ -97,6 +98,7 @@ test('tracing integration works', t => {
9798
app.on('close', function (code) {
9899
t.equal(stderr, '', 'empty stderr from app')
99100
t.equal(code, 0, 'app exited 0')
101+
appIsClosed = true
100102
})
101103
}
102104

@@ -116,11 +118,13 @@ test('tracing integration works', t => {
116118
function collectTracesLogsAndCheck (traceObj, logObj) {
117119
if (traceObj) {
118120
traceObjs.push(traceObj)
121+
t.comment(`received traceObjs ${traceObjs.length}`)
119122
}
120123
if (logObj) {
121124
t.ok(validate(logObj), 'logObj is ECS valid')
122-
t.equal(ecsLoggingValidate(logObj), null)
125+
t.equal(ecsLoggingValidate(logObj), null, 'logObj is ecs-logging valid')
123126
logObjs.push(logObj)
127+
t.comment(`received logObjs ${logObjs.length}`)
124128
}
125129
if (traceObjs.length >= 3 && logObjs.length >= 1) {
126130
t.ok(traceObjs[0].metadata, 'traceObjs[0] is metadata')
@@ -130,38 +134,44 @@ test('tracing integration works', t => {
130134
t.equal(logObjs[0].trace.id, span.trace_id, 'trace.id matches')
131135
t.equal(logObjs[0].transaction.id, span.transaction_id, 'transaction.id matches')
132136
t.equal(logObjs[0].span.id, span.id, 'span.id matches')
133-
t.equal(logObjs[0].service.name, 'test-apm')
134-
t.equal(logObjs[0].event.dataset, 'test-apm.log')
137+
t.equal(logObjs[0].service.name, 'test-apm', 'service.name matches')
138+
t.equal(logObjs[0].event.dataset, 'test-apm.log', 'event.dataset matches')
135139
finish()
136140
}
137141
}
138142

139143
function finish () {
140-
app.on('close', function () {
144+
if (appIsClosed) {
141145
apmServer.close(function () {
142146
t.end()
143147
})
144-
})
148+
} else {
149+
app.on('close', function () {
150+
apmServer.close(function () {
151+
t.end()
152+
})
153+
})
154+
}
145155
}
146156

147157
step1StartMockApmServer(function onListening (apmServerErr, apmServerUrl) {
148-
t.ifErr(apmServerErr)
158+
t.ifErr(apmServerErr, 'no error from starting the mock APM server')
149159
if (apmServerErr) {
150160
finish()
151161
return
152162
}
153163
t.ok(apmServerUrl, 'apmServerUrl: ' + apmServerUrl)
154164

155165
step2StartApp(apmServerUrl, function onReady (appErr, appUrl) {
156-
t.ifErr(appErr)
166+
t.ifErr(appErr, 'no error from starting the app')
157167
if (appErr) {
158168
finish()
159169
return
160170
}
161171
t.ok(appUrl, 'appUrl: ' + appUrl)
162172

163173
step3CallApp(appUrl, function (clientErr) {
164-
t.ifErr(clientErr)
174+
t.ifErr(clientErr, 'no error from calling the app')
165175

166176
// The thread of control now is expected to be in
167177
// `collectTracesLogsAndCheck()`.

0 commit comments

Comments
 (0)