-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Add grpc-gateway tests for all APIv3 methods #5051
Conversation
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
9703430
to
161e6a6
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5051 +/- ##
==========================================
- Coverage 95.53% 95.53% -0.01%
==========================================
Files 313 313
Lines 18161 18160 -1
==========================================
- Hits 17350 17349 -1
Misses 649 649
Partials 162 162
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
t.Run("FindTraces", func(t *testing.T) { | ||
runGatewayFindTraces(t, gw, setupRequest) | ||
}) | ||
} |
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.
@albertteoh this was the point of cleaning up tests - now they are testing all gateway APIs and we can try things like upgrading the gateway dep to v2.x or completely re-implementing it. My next step would be to try an upgrade, which requires building a new protoc image that I just kicked off (takes 4hrs to build), and maybe with v2 gateway we could find a way to integrate gogo marshaling.
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
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 to me.
## Which problem is this PR solving? - Part of #5052 ## Description of the changes - Pull in IDL change jaegertracing/jaeger-idl#102 - Re-implement APIv3 HTTP endpoints without the use of grpc-gateway - Share tests from grpc-gateway for manual implementation - Refactor tests to avoid duplication of snapshots - Fix inconsistency between http and grpc tenancy interceptors where HTTP was returning Unauthenticated in certain cases, but GRPC was always returning Forbidden. Make them consistent: missing tenant header results in Unauthenticated. ## Follow-ups - [x] http implementation needs more unit tests (mostly error handling and parameter variations) - [ ] the new implementation is not hooked up into production code yet, I first want to confirm it works with model-v2, and just in general minimize the scope of a single PR ## How was this change tested? - Using unit tests added in #5051, with additional enhancements --------- Signed-off-by: Yuri Shkuro <[email protected]>
Which problem is this PR solving?
Description of the changes
Gaps
How was this change tested?