-
Notifications
You must be signed in to change notification settings - Fork 534
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
fix: early close of span with graphql execute #752
Conversation
|
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. Would be nice if you could add a test for this
Codecov Report
@@ Coverage Diff @@
## main #752 +/- ##
==========================================
- Coverage 96.87% 94.96% -1.92%
==========================================
Files 11 22 +11
Lines 641 1629 +988
Branches 127 221 +94
==========================================
+ Hits 621 1547 +926
- Misses 20 82 +62
|
Cheers, I'll take a look tomorrow at getting a unit test written and also rebasing it with the latest main branch |
I've fixed the failing tests - it was due to a change in the execution order changing. I've added a test for the span timings, let me know if there are any changes you'd like - eg duplicating the test in to the different cases eg when depth is set to 0. |
@samriley could you rebase your PR please ? it should be fix the lint |
Which problem is this PR solving?
Fixes #750
Short description of the changes
Await promise resolution before calling endSpan