-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
test(node): Add tests for current DSC transaction name updating behavior #13961
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
Conversation
75af20f
to
8a67452
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fought multiple hours with our test runner to get these tests working. Very frustrating.
Turns out, you can't assert on headers in the test server as well as on events in the same test. It will appear as if it's working but it won't fail if an assertion actually fails :/
moreover, there#s also super weird inter operation between expect
and ecpectHeaders
when you want to check on an event and its headers. After trying to make everything work in one test, I eventually gave up and made one for baggage
and one for the trace
envelope header.
I think part of the problem is that there is some race condition with the test server and the test runners both calling done
which completely fucks up the assertion count.
size-limit report 📦
|
❌ 6 Tests Failed:
View the top 3 failed tests by shortest run time
To view individual test run time comparison to the main branch, go to the Test Analytics Dashboard |
8a67452
to
08fdf0d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, I think it's okay to just add tests for the stuff that didn't fall into a race condition. But what a bummer that this didn't work :/
This PR adds tests for DSC updating behaviour in the Node SDK, analogously to #13953
Currently, also in Node, we mutate the DSC and always include the most up to date transaction name