tests: refactor away need for test/instrumentation-specific mocks#2319
tests: refactor away need for test/instrumentation-specific mocks#2319
Conversation
Drop the need for the mocks _agent.js and _instrumentation.js under test/instrumentation that reach into Agent internal implementation details. This will help coming re-work of some Instrumentation internal implementation details.
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
Trends 🧪 |
astorm
left a comment
There was a problem hiding this comment.
Overall this look good -- as in it does the thing and moves us away from the mock agent. Also, no noticeable perf. regressions in the time these particular new tests take to run. Approving.
One thing to note -- the npx tape test/instrumentation/transaction.test.js has a different number of assertions on this branch when compared to master (140 on master, 136 on this branch) I did not dig deeply into it but my presumption is these are assertions lost due to how mockInstrumentation worked. Not a deal break, just an observation in case this wasn't expected.
We add two assertions near the top, e.g. here is one added assertion: https://github.com/elastic/apm-agent-nodejs/pull/2319/files#diff-115ec49fff221477451b17e1fca2a5208448489ccbb84c4eb7b53c6c4016f3caR189-R203 Then for six replacements of mockInstrumentation, we drop a Thanks for the review. |
…astic#2319) Drop the need for the mocks _agent.js and _instrumentation.js under test/instrumentation that reach into Agent internal implementation details. This will help coming re-work of some Instrumentation internal implementation details.
Drop the need for the mocks _agent.js and _instrumentation.js under
test/instrumentation that reach into Agent internal implementation
details. This will help coming re-work of some Instrumentation
internal implementation details.
This depends on #2316 going in first (for the added
"test/_capturing_transport.js").