make metrics.test.js less flaky #1715
Conversation
test/plugins/metrics.test.js
Outdated
| @@ -203,11 +203,11 @@ describe('request metrics plugin', function() { | |||
| // However the timeout value is 200, | |||
| // it's calculated by the client, | |||
| // but setTimeout is happening on the server, tolerate 10ms | |||
| assert.isAtLeast(metrics.preLatency, 50); | |||
| assert.closeTo(metrics.preLatency, 50, 10); | |||
There was a problem hiding this comment.
Looking a bit further into this issue, it seems it could be due to nodejs/node#10154. If so, it means that the value of metrics.preLatency should always be at least 49ms in this test.
Thus I would suggest changing this to:
assert.isAtLeast(metrics.preLatency, 49);
And adding a comment why we use the value 49 instead of 50 (and adding a link to that GH issue in that comment).
Moreover, it seems that this specific Node.js core issue has been fixed in Node.js 11.x (but it hasn't been back ported to previous versions), so it would be interesting to check whether we're indeed not seeing this behavior for those tests when running with node 11.x.
|
Well done, looks like your first intuition was correct, locally with Node.js v11.1.0 the tests pass unmodified. I’ve adjusted the test and added a comment. |
Seeing values between 49 and 56, relaxing the test to resolve #1691 seems reasonable.