Skip to content

Commit

Permalink
test(gRPC): add test case prompt and remove redundant unit tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@DMwangnima
DMwangnima committed Dec 3, 2024
1 parent e98fd88 commit 2c729a1
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 88 deletions.
2 changes: 1 addition & 1 deletion pkg/remote/trans/nphttp2/grpc/http2_server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -958,7 +958,7 @@ func (t *http2Server) keepalive() {
continue
}
if outstandingPing && kpTimeoutLeft <= 0 {
klog.Infof("transport: closing server transport due to idleness.")
klog.CtxInfof(t.ctx, "transport: closing server transport due to idleness.")
t.closeWithErr(errIdleClosing)
return
}
Expand Down
51 changes: 23 additions & 28 deletions pkg/remote/trans/nphttp2/grpc/keepalive_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ func TestKeepaliveServerClosesUnresponsiveClient(t *testing.T) {
// and then waits for KeepaliveParams.Timeout for a ping ack.
timeout := time.NewTimer(1 * time.Second)
select {
// Ideally, this case will trigger in 50ms
case err := <-errCh:
if err != io.EOF {
t.Fatalf("client.Read(_) = _,%v, want io.EOF", err)
Expand All @@ -213,18 +214,11 @@ func TestKeepaliveServerClosesUnresponsiveClient(t *testing.T) {
func TestKeepaliveServerWithResponsiveClient(t *testing.T) {
serverConfig := &ServerConfig{
KeepaliveParams: ServerKeepalive{
Time: 50 * time.Millisecond,
Timeout: 50 * time.Millisecond,
Time: 25 * time.Millisecond,
Timeout: 25 * time.Millisecond,
},
}
server, client := setUpWithOptions(t, 0, serverConfig, suspended, ConnectOptions{
// FIXME the original ut don't contain KeepaliveParams
KeepaliveParams: ClientKeepalive{
Time: 50 * time.Millisecond,
Timeout: 50 * time.Millisecond,
PermitWithoutStream: true,
},
})
server, client := setUpWithOptions(t, 0, serverConfig, suspended, ConnectOptions{})
defer func() {
client.Close(errSelfCloseForTest)
server.stop()
Expand Down Expand Up @@ -450,6 +444,7 @@ func TestKeepaliveServerEnforcementWithAbusiveClientNoRPC(t *testing.T) {

timeout := time.NewTimer(4 * time.Second)
select {
// Ideally, this case will trigger in 100ms
case <-client.Error():
if !timeout.Stop() {
<-timeout.C
Expand Down Expand Up @@ -499,6 +494,7 @@ func TestKeepaliveServerEnforcementWithAbusiveClientWithRPC(t *testing.T) {

timeout := time.NewTimer(4 * time.Second)
select {
// Ideally, this case will trigger in 100ms
case <-client.Error():
if !timeout.Stop() {
<-timeout.C
Expand All @@ -523,14 +519,14 @@ func TestKeepaliveServerEnforcementWithAbusiveClientWithRPC(t *testing.T) {
func TestKeepaliveServerEnforcementWithObeyingClientNoRPC(t *testing.T) {
serverConfig := &ServerConfig{
KeepaliveEnforcementPolicy: EnforcementPolicy{
MinTime: 50 * time.Millisecond,
MinTime: 25 * time.Millisecond,
PermitWithoutStream: true,
},
}
clientOptions := ConnectOptions{
KeepaliveParams: ClientKeepalive{
Time: 51 * time.Millisecond,
Timeout: 500 * time.Millisecond,
Time: 26 * time.Millisecond,
Timeout: 52 * time.Millisecond,
PermitWithoutStream: true,
},
}
Expand All @@ -540,9 +536,8 @@ func TestKeepaliveServerEnforcementWithObeyingClientNoRPC(t *testing.T) {
server.stop()
}()

// Give keepalive enough time.
t.Parallel() // slow test due to sleep, remove me after optimization.
time.Sleep(500 * time.Millisecond)
// 3 Ping/Ping-Ack round trips
time.Sleep(100 * time.Millisecond)

ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
defer cancel()
Expand All @@ -559,13 +554,13 @@ func TestKeepaliveServerEnforcementWithObeyingClientNoRPC(t *testing.T) {
func TestKeepaliveServerEnforcementWithObeyingClientWithRPC(t *testing.T) {
serverConfig := &ServerConfig{
KeepaliveEnforcementPolicy: EnforcementPolicy{
MinTime: 50 * time.Millisecond,
MinTime: 25 * time.Millisecond,
},
}
clientOptions := ConnectOptions{
KeepaliveParams: ClientKeepalive{
Time: 51 * time.Millisecond,
Timeout: 500 * time.Millisecond,
Time: 26 * time.Millisecond,
Timeout: 52 * time.Millisecond,
},
}
server, client := setUpWithOptions(t, 0, serverConfig, suspended, clientOptions)
Expand All @@ -580,9 +575,8 @@ func TestKeepaliveServerEnforcementWithObeyingClientWithRPC(t *testing.T) {
t.Fatalf("client.NewStream() failed: %v", err)
}

// Give keepalive enough time.
t.Parallel() // slow test due to sleep, remove me after optimization.
time.Sleep(1 * time.Second)
// 3 Ping/Ping-Ack round trips
time.Sleep(100 * time.Millisecond)

// Make sure the client transport is healthy.
if _, err := client.NewStream(ctx, &CallHdr{}); err != nil {
Expand All @@ -599,13 +593,13 @@ func TestKeepaliveServerEnforcementWithObeyingClientWithRPC(t *testing.T) {
func TestKeepaliveServerEnforcementWithDormantKeepaliveOnClient(t *testing.T) {
serverConfig := &ServerConfig{
KeepaliveEnforcementPolicy: EnforcementPolicy{
MinTime: 400 * time.Millisecond,
MinTime: 60 * time.Millisecond,
},
}
clientOptions := ConnectOptions{
KeepaliveParams: ClientKeepalive{
Time: 25 * time.Millisecond,
Timeout: 250 * time.Millisecond,
Time: 20 * time.Millisecond,
Timeout: 40 * time.Millisecond,
},
}
server, client := setUpWithOptions(t, 0, serverConfig, normal, clientOptions)
Expand All @@ -614,9 +608,10 @@ func TestKeepaliveServerEnforcementWithDormantKeepaliveOnClient(t *testing.T) {
server.stop()
}()

// No active streams on the client. Give keepalive enough time.
t.Parallel() // slow test due to sleep, remove me after optimization.
time.Sleep(500 * time.Millisecond)
// No active streams on the client
// Verify that client has entered a dormant state
// after the client's keepalive(20+40) time has elapsed.
time.Sleep(80 * time.Millisecond)

ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
defer cancel()
Expand Down
107 changes: 52 additions & 55 deletions pkg/remote/trans/nphttp2/grpc/testutils/leakcheck/leakcheck_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,59 +20,56 @@

package leakcheck

import (
"fmt"
"strings"
"testing"
"time"
)
// since leakcheck is not used in Kitex grpc unit tests for now, we just skip the unit tests here.
// it can save 4s.
// when we need to make use of leakcheck, we can uncomment the following code and do some optimizations.

type testErrorfer struct {
errorCount int
errors []string
}

func (e *testErrorfer) Errorf(format string, args ...interface{}) {
e.errors = append(e.errors, fmt.Sprintf(format, args...))
e.errorCount++
}

func TestCheck(t *testing.T) {
const leakCount = 3
for i := 0; i < leakCount; i++ {
go func() { time.Sleep(2 * time.Second) }()
}
if ig := interestingGoroutines(); len(ig) == 0 {
t.Error("blah")
}
e := &testErrorfer{}
check(e, time.Second)
if e.errorCount != leakCount {
t.Errorf("check found %v leaks, want %v leaks", e.errorCount, leakCount)
t.Logf("leaked goroutines:\n%v", strings.Join(e.errors, "\n"))
}
check(t, 3*time.Second)
}

func ignoredTestingLeak(d time.Duration) {
time.Sleep(d)
}

func TestCheckRegisterIgnore(t *testing.T) {
RegisterIgnoreGoroutine("ignoredTestingLeak")
const leakCount = 3
for i := 0; i < leakCount; i++ {
go func() { time.Sleep(2 * time.Second) }()
}
go func() { ignoredTestingLeak(3 * time.Second) }()
if ig := interestingGoroutines(); len(ig) == 0 {
t.Error("blah")
}
e := &testErrorfer{}
check(e, time.Second)
if e.errorCount != leakCount {
t.Errorf("check found %v leaks, want %v leaks", e.errorCount, leakCount)
t.Logf("leaked goroutines:\n%v", strings.Join(e.errors, "\n"))
}
check(t, 3*time.Second)
}
//type testErrorfer struct {
// errorCount int
// errors []string
//}
//
//func (e *testErrorfer) Errorf(format string, args ...interface{}) {
// e.errors = append(e.errors, fmt.Sprintf(format, args...))
// e.errorCount++
//}
//
//func TestCheck(t *testing.T) {
// const leakCount = 3
// for i := 0; i < leakCount; i++ {
// go func() { time.Sleep(2 * time.Second) }()
// }
// if ig := interestingGoroutines(); len(ig) == 0 {
// t.Error("blah")
// }
// e := &testErrorfer{}
// check(e, time.Second)
// if e.errorCount != leakCount {
// t.Errorf("check found %v leaks, want %v leaks", e.errorCount, leakCount)
// t.Logf("leaked goroutines:\n%v", strings.Join(e.errors, "\n"))
// }
// check(t, 3*time.Second)
//}
//
//func ignoredTestingLeak(d time.Duration) {
// time.Sleep(d)
//}
//
//func TestCheckRegisterIgnore(t *testing.T) {
// RegisterIgnoreGoroutine("ignoredTestingLeak")
// const leakCount = 3
// for i := 0; i < leakCount; i++ {
// go func() { time.Sleep(2 * time.Second) }()
// }
// go func() { ignoredTestingLeak(3 * time.Second) }()
// if ig := interestingGoroutines(); len(ig) == 0 {
// t.Error("blah")
// }
// e := &testErrorfer{}
// check(e, time.Second)
// if e.errorCount != leakCount {
// t.Errorf("check found %v leaks, want %v leaks", e.errorCount, leakCount)
// t.Logf("leaked goroutines:\n%v", strings.Join(e.errors, "\n"))
// }
// check(t, 3*time.Second)
//}
13 changes: 9 additions & 4 deletions pkg/remote/trans/nphttp2/grpc/transport_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import (
"golang.org/x/net/http2/hpack"

"github.com/cloudwego/kitex/internal/test"
"github.com/cloudwego/kitex/pkg/klog"
"github.com/cloudwego/kitex/pkg/remote/trans/nphttp2/codes"
"github.com/cloudwego/kitex/pkg/remote/trans/nphttp2/grpc/grpcframe"
"github.com/cloudwego/kitex/pkg/remote/trans/nphttp2/grpc/testutils"
Expand Down Expand Up @@ -70,6 +71,7 @@ var (
expectedResponseLarge = make([]byte, initialWindowSize*2)
expectedInvalidHeaderField = "invalid/content-type"
errSelfCloseForTest = errors.New("self-close in test")
originalDefLogger = klog.DefaultLogger()

Check failure on line 74 in pkg/remote/trans/nphttp2/grpc/transport_test.go

View workflow job for this annotation

GitHub Actions / golangci-lint 

var `originalDefLogger` is unused (unused)
)

func init() {
Expand Down Expand Up @@ -466,6 +468,9 @@ func (s *server) stop() {
s.mu.Lock()
for c := range s.conns {
c.Close()
rawSrv := c.(*http2Server)
// wait for reader goroutine exited
<-rawSrv.readerDone
}
s.conns = nil
s.mu.Unlock()
Expand Down Expand Up @@ -602,7 +607,7 @@ func TestInflightStreamClosing(t *testing.T) {
defer server.stop()
defer client.Close(fmt.Errorf("self-close in test"))

ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
ctx, cancel := context.WithTimeout(client.ctx, defaultTestTimeout)
defer cancel()
stream, err := client.NewStream(ctx, &CallHdr{})
if err != nil {
Expand Down Expand Up @@ -926,7 +931,7 @@ func TestLargeMessageSuspension(t *testing.T) {
Method: "foo.Large",
}
// Set a long enough timeout for writing a large message out.
ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
ctx, cancel := context.WithTimeout(ct.ctx, 100*time.Millisecond)
defer cancel()
s, err := ct.NewStream(ctx, callHdr)
if err != nil {
Expand Down Expand Up @@ -1547,7 +1552,7 @@ func TestInvalidHeaderField(t *testing.T) {
Host: "localhost",
Method: "foo",
}
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
ctx, cancel := context.WithTimeout(ct.ctx, defaultTestTimeout)
defer cancel()
s, err := ct.NewStream(ctx, callHdr)
if err != nil {
Expand All @@ -1566,7 +1571,7 @@ func TestHeaderChanClosedAfterReceivingAnInvalidHeader(t *testing.T) {
server, ct := setUp(t, 0, math.MaxUint32, invalidHeaderField)
defer server.stop()
defer ct.Close(errSelfCloseForTest)
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
ctx, cancel := context.WithTimeout(ct.ctx, defaultTestTimeout)
defer cancel()
s, err := ct.NewStream(ctx, &CallHdr{Host: "localhost", Method: "foo"})
if err != nil {
Expand Down

0 comments on commit 2c729a1

Please sign in to comment.