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.

Sign up for GitHub

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

Jump to bottom

authz: modify the tests to use stubserver instead of testservice implementations #7888

Open

janardhanvissa wants to merge 11 commits into grpc:master from janardhanvissa:stubserver-streamingcall

Contributor

janardhanvissa commented Dec 2, 2024

Partially Addresses: #7291

RELEASE NOTES: None


          Initializing StreamingInputCall and SteamingOutputCall in the stubserver

cb0a479

codecov bot commented Dec 2, 2024 •

edited

Loading

Codecov Report

Attention: Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 81.94%. Comparing base (e8055ea) to head (4298e86).
Report is 5 commits behind head on master.

Files with missing lines	Patch %	Lines
internal/stubserver/stubserver.go	50.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7888      +/-   ##
==========================================
- Coverage   82.08%   81.94%   -0.15%     
==========================================
  Files         379      379              
  Lines       38261    38265       +4     
==========================================
- Hits        31406    31355      -51     
- Misses       5551     5595      +44     
- Partials     1304     1315      +11

Files with missing lines	Coverage Δ
internal/stubserver/stubserver.go	`80.00% <50.00%> (-0.92%)`	⬇️

... and 23 files with indirect coverage changes

arjan-bal requested a review from purnesh42H

December 3, 2024 05:45

arjan-bal assigned purnesh42H

arjan-bal added this to the 1.69 Release milestone

arjan-bal added the Type: Testing label

janardhankrishna-sai added 2 commits

December 3, 2024 07:00


          switching to stubserver in tests instead of testserviceimpl

d364fdc


          switching to stubserver instead of testservice

309abd1

purnesh42H modified the milestones: 1.69 Release, 1.70 Release

purnesh42H reviewed

View reviewed changes

Contributor

purnesh42H left a comment

1 comment otherwise lgtm. Good work!

authz/audit/audit_logging_test.go Show resolved Hide resolved

purnesh42H assigned janardhanvissa and unassigned purnesh42H


          removing redundant server code

fa74a99

purnesh42H reviewed

View reviewed changes

internal/stubserver/stubserver.go Outdated

+              	EmptyCallF           func(ctx context.Context, in *testpb.Empty) (*testpb.Empty, error)
+              	UnaryCallF           func(ctx context.Context, in *testpb.SimpleRequest) (*testpb.SimpleResponse, error)
+              	FullDuplexCallF      func(stream testgrpc.TestService_FullDuplexCallServer) error
+              	StreamingInputCallF  func(stream testgrpc.TestService_StreamingInputCallServer) error

Contributor

purnesh42H Dec 6, 2024

@janardhanvissa i think we don't need to create extra functions for input and output. Can you just use the FullDuplexCallF for streaming calls

grpc-go/authz/audit/audit_logging_test.go

Line 250 in 66ba4b2

FullDuplexCallF: func(stream testgrpc.TestService_FullDuplexCallServer) error {

?

Contributor

purnesh42H Dec 18, 2024

Discussed offline. We need to use stream.CloseAndRecv() so we need a client streaming handler. We should rename it to ClientStreamingInputCall and ClientStreamingOutputCall though

purnesh42H changed the title ~~stubserver: Initializing StreamingInputCall and StreamingOutputCall in the stubserver~~ authz: Modify the tests to use stubserver instead of testservice implementations

purnesh42H changed the title ~~authz: Modify the tests to use stubserver instead of testservice implementations~~ authz: modify the tests to use stubserver instead of testservice implementations

janardhankrishna-sai added 2 commits

December 18, 2024 09:49


          renaming streamingcalls

1e7a099


          moving initialization of NewServer inside stub

924a9e7

purnesh42H reviewed

View reviewed changes

internal/stubserver/stubserver.go Show resolved Hide resolved

internal/stubserver/stubserver.go Show resolved Hide resolved

purnesh42H reviewed

View reviewed changes

internal/stubserver/stubserver.go Outdated

+              	UnaryCallF                func(ctx context.Context, in *testpb.SimpleRequest) (*testpb.SimpleResponse, error)
+              	FullDuplexCallF           func(stream testgrpc.TestService_FullDuplexCallServer) error
+              	ClientStreamingInputCall  func(stream testgrpc.TestService_StreamingInputCallServer) error
+              	ClientStreamingOutputCall func(req *testpb.StreamingOutputCallRequest, stream testgrpc.TestService_StreamingOutputCallServer) error

Contributor

purnesh42H Dec 20, 2024

@janardhanvissa let's change this StreamingInputCall and StreamingOutputCall and in the comment mention that that input is for ClientStreaming and output is for ServerStreaming


          renaming client and server handlers

ecdab23

janardhanvissa force-pushed the stubserver-streamingcall branch from cfba48c to ecdab23 Compare

December 20, 2024 09:56

janardhankrishna-sai added 2 commits

December 20, 2024 10:10


          updating grpc_authz_end2end_test.go


          Merge branch 'master' of https://github.com/janardhanvissa/grpc-go in…

5b41dbe

…to stubserver-streamingcall

purnesh42H reviewed

View reviewed changes

internal/stubserver/stubserver.go Outdated

+              	EmptyCallF           func(ctx context.Context, in *testpb.Empty) (*testpb.Empty, error)
+              	UnaryCallF           func(ctx context.Context, in *testpb.SimpleRequest) (*testpb.SimpleResponse, error)
+              	FullDuplexCallF      func(stream testgrpc.TestService_FullDuplexCallServer) error
+              	StreamingInputCallF  func(stream testgrpc.TestService_StreamingInputCallServer) error                                          // ClientStreaming

Contributor

purnesh42H Dec 21, 2024 •

edited

Loading

Next to StreamingInputCallF please mention Client-Streaming request

purnesh42H approved these changes

View reviewed changes

Contributor

purnesh42H left a comment

lgtm. Please address the comment suggestion for Streaming calls

internal/stubserver/stubserver.go Outdated

+              	UnaryCallF           func(ctx context.Context, in *testpb.SimpleRequest) (*testpb.SimpleResponse, error)
+              	FullDuplexCallF      func(stream testgrpc.TestService_FullDuplexCallServer) error
+              	StreamingInputCallF  func(stream testgrpc.TestService_StreamingInputCallServer) error                                          // ClientStreaming
+              	StreamingOutputCallF func(req *testpb.StreamingOutputCallRequest, stream testgrpc.TestService_StreamingOutputCallServer) error // ServerStreaming

Contributor

purnesh42H Dec 21, 2024

Next to StreamingOutputCallF please mention Server-streaming response

janardhankrishna-sai added 2 commits

December 22, 2024 06:23


          updating client and server streaming comments

4588fc1


          removing empty lines for re-trigger

4298e86

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels