http2_client is missing tests

[jeanbza]

The following cases are untested, and should be tested:

• https://github.com/grpc/grpc-go/blob/fb6867e42b4743663f07f4c7ce6c29c8d01247e1/internal/transport/http2_client.go#L275-L278

• https://github.com/grpc/grpc-go/blob/fb6867e42b4743663f07f4c7ce6c29c8d01247e1/internal/transport/http2_client.go#L279-L282

• https://github.com/grpc/grpc-go/blob/fb6867e42b4743663f07f4c7ce6c29c8d01247e1/internal/transport/http2_client.go#L298-L301

• https://github.com/grpc/grpc-go/blob/fb6867e42b4743663f07f4c7ce6c29c8d01247e1/internal/transport/http2_client.go#L304-L307

e.g. changes in https://github.com/grpc/grpc-go/pull/2219 were almost merged that should not have because of lack of the aforementioned test coverage.

EDIT: Getting complete coverage for all network-related edge cases using only unit tests is difficult and can lead to fragile test suites.

We could instead write test cases for the http2_client in gRPC-Go that focus on edge cases involving unstable network connections. Specifically, we want to simulate connections that frequently disconnect during the connection phase and while making RPCs. These tests could help uncover various error paths that are currently uncovered, improving the robustness of gRPC-Go in real-world scenarios.

[v-sreejith]

@ginayeh I would like to contribute here

[arvindbr8]

Sure! Assigning it to you.

The original issue description might not be relevant anymore. I would re-evaluate the missed coverage of the file here.

[arvindbr8]

@v-sreejith -- ping.. Are you still actively tracking this?

[hanut19]

@purnesh42H assign this issue to me

[hanut19]

@purnesh42H, for reference which file contains other test cases for this function "func newHTTP2Client(connectCtx, ctx context.Context, addr resolver.Address, opts ConnectOptions, onClose func(GoAwayReason)) (_ *http2Client, err error) " eg. name of these other test cases are. plz provide

[hanut19]

@purnesh42H In order to cover unit test for below condition (ref. screenshot). I need to mock the “Write” function which is defined “net.go” of net package.

I have written below unit test for cover above condition. I tried to mock “Write” function with below approach.

Using that approach unable to mock the “Write”. Get below error FAIL: Write([]uint8) at: [C:/go-work/src/grpc-go/internal/transport/http2_client_test.go:214] http2_client_test.go:223: FAIL: 0 out of 1 expectation(s) were met.

[purnesh42H]

@purnesh42H, for reference which file contains other test cases for this function "func newHTTP2Client(connectCtx, ctx context.Context, addr resolver.Address, opts ConnectOptions, onClose func(GoAwayReason)) (_ *http2Client, err error) " eg. name of these other test cases are. plz provide

@hanut19 you have to write test cases for uncovered blocks here https://app.codecov.io/gh/grpc/grpc-go/blob/master/internal%2Ftransport%2Fhttp2_client.go

[purnesh42H]

@purnesh42H In order to cover unit test for below condition (ref. screenshot). I need to mock the “Write” function which is defined “net.go” of net package.

I have written below unit test for cover above condition. I tried to mock “Write” function with below approach.

Using that approach unable to mock the “Write”. Get below error FAIL: Write([]uint8) at: [C:/go-work/src/grpc-go/internal/transport/http2_client_test.go:214] http2_client_test.go:223: FAIL: 0 out of 1 expectation(s) were met.

@hanut19 we don't use mocking libraries for unit tests. Instead we write custom implementations of the interfaces that we want to mock. For example, in this case you want to provide an override for net.conn so you can write your custom dialer similar to this PR https://github.com/purnesh42H/grpc-go/pull/1/files#diff-c90a44d2d36c8cc9c90b97b290cce3bdf6a5086bdac1e51e42d0e7b8f415b8b8 and provide that dialer in WithContextDialer while creating new client

[hanut19]

@purnesh42H I have write unit test case for first condition under this issue. My code is under this url https://github.com/hanut19/grpc-go/tree/issue-2248. My code cover first condition but while i am run this unit test showing "exit status 1" on terminal. Any suggestion on please let me know.

[purnesh42H]

https://github.com/hanut19/grpc-go/tree/issue-2248

@hanut19 could you point to the code block you have the problem? I tried to look https://github.com/hanut19/grpc-go/blob/issue-2248/internal/transport/http2_client_test.go but it has so many tests.

Also, please provide more context on what condition your are referring. Just saying first condition is hard to figure.

[hanut19]

@purnesh42H 1) for condition https://github.com/grpc/grpc-go/blob/fb6867e42b4743663f07f4c7ce6c29c8d01247e1/internal/transport/http2_client.go#L275-L278 Unit test case is here https://github.com/hanut19/grpc-go/blob/issue-2248/internal/transport/http2_client_test.go#L13-L79

2) for condition https://github.com/grpc/grpc-go/blob/fb6867e42b4743663f07f4c7ce6c29c8d01247e1/internal/transport/http2_client.go#L279-L282 Unit test case is here https://github.com/hanut19/grpc-go/blob/issue-2248/internal/transport/http2_client_test.go#L81-L149

[purnesh42H]

@hanut19 as discussed since you are writing test cases for failure conditions, your happy case is error being returned and bad case is error not being returned so raise error only in case of error being nil for your unit tests

[hanut19]

@purnesh42H

1. For condition: https://github.com/grpc/grpc-go/blob/fb6867e42b4743663f07f4c7ce6c29c8d01247e1/internal/transport/http2_client.go#L275-L278 unit test case is here: https://github.com/hanut19/grpc-go/blob/issue-2248/internal/transport/http2_client_test.go#L13-L57

2. For condition: https://github.com/grpc/grpc-go/blob/fb6867e42b4743663f07f4c7ce6c29c8d01247e1/internal/transport/http2_client.go#L279-L282 unit test case is here: https://github.com/hanut19/grpc-go/blob/issue-2248/internal/transport/http2_client_test.go#L59-L102

3. For condition: https://github.com/grpc/grpc-go/blob/fb6867e42b4743663f07f4c7ce6c29c8d01247e1/internal/transport/http2_client.go#L298-L301 unit test case is here: https://github.com/hanut19/grpc-go/blob/issue-2248/internal/transport/http2_client_test.go#L104-L148

   4. For condition: https://github.com/grpc/grpc-go/blob/fb6867e42b4743663f07f4c7ce6c29c8d01247e1/internal/transport/http2_client.go#L304-L307 unit test case is here: https://github.com/hanut19/grpc-go/blob/issue-2248/internal/transport/http2_client_test.go#L150-L194

[hanut19]

@purnesh42H I have raised PR: https://github.com/grpc/grpc-go/pull/7442

But one of pipeline is failed Run vet.sh getting error:

• cleanup
• git reset --hard HEAD HEAD is now at 10420a1 Merge 6e72ed96b4b3365cc7c72fd22912e7356df93289 into 3eb01450ff91253bfc67a6b5b3c258ea2fc245c8 Error: Process completed with exit code 1.

refer url: https://github.com/grpc/grpc-go/actions/runs/10138936080/job/28031400686?pr=7442

Please help here to understand this issue.

[purnesh42H]

@purnesh42H I have raised PR: #7442

But one of pipeline is failed Run vet.sh getting error:

• cleanup
• git reset --hard HEAD HEAD is now at 10420a1 Merge 6e72ed956df93289 into 3eb0145 Error: Process completed with exit code 1.

refer url: https://github.com/grpc/grpc-go/actions/runs/10138936080/job/28031400686?pr=7442

Please help here to understand this issue.

Can you rebase with latest main branch and retry? You can also run ./scripts/vet.sh locally to verify if there is a real issue. See https://github.com/grpc/grpc-go/blob/master/CONTRIBUTING.md

[hanut19]

@purnesh42H Please review this PR: https://github.com/grpc/grpc-go/pull/7462 I have resolved comments.