diff --git a/docs/contributing/coding-style.md b/docs/contributing/coding-style.md index 055e0820d0..2591662eb9 100644 --- a/docs/contributing/coding-style.md +++ b/docs/contributing/coding-style.md @@ -509,3 +509,97 @@ for name, fn := range tests { } ``` + +### Generate test code + +We implement our own [gotests](https://github.com/cweill/gotests) template to generate test code. +If you want to install `gotest` tools, please execute the following command under the project root directory. + +```bash +make gotests/install +``` + +If you use the following command to generate the missing test code. + +```bash +make make gotests/gen +``` + +After the command above executed, the file `*target*_test.go` will be generated for each Go source file. +The test code generated follows the table-driven test format. +You can implement your test code under the `tests` variable generated following the table-driven test format. + +### Customize test case + +We do not suggest to modify the generated code other than the `tests` variable, but in some cases, you may need to modify the generated code to meet your requirement, for example: + +1. init() function + + init() function is executed automatically before the test is started. + You may need to initialize some singleton before your test cases are executed. + For example, Vald uses [glg](https://github.com/kpango/glg) library for logging by default, if the logger is not initialized before the test, the nil pointer error may be thrown during the test is running. + You may need to implement `init()` function like: + + ```golang + func init() { + log.Init() + } + ``` + + And place it on the header of the test file. + +1. goleak option + + By default, the generated test code will use [goleak](https://github.com/uber-go/goleak) library to test if there is any Goroutine leak. + Sometimes you may want to skip the detection, for example, Vald uses [fastime](https://github.com/kpango/fastime) library but the internal Goroutine is not closed due to the needs of the library. + To skip the goleak detection we need to create the following variable to store the ignore function. + + ```golang + var ( + // Goroutine leak is detected by `fastime`, but it should be ignored in the test because it is an external package. + goleakIgnoreOptions = []goleak.Option{ + goleak.IgnoreTopFunction("github.com/kpango/fastime.(*Fastime).StartTimerD.func1"), + } + ) + ``` + + And modify the generated test code. + + ```golang + // before + for _, test := range tests { + t.Run(test.name, func(tt *testing.T) { + defer goleak.VerifyNone(tt) + + // after + for _, test := range tests { + t.Run(test.name, func(tt *testing.T) { + // modify the following line + defer goleak.VerifyNone(tt, goleakIgnoreOptions...) + ``` + +1. Defer function + + By default the template provides `beforeFunc()` and `afterFunc()` to initialize and finalize the test case, but in some case, it may not support your use case. + For example `recover()` function only works in `defer()` function, if you need to use `recover()` function to handle the panic in your test code, you may need to implement your custom `defer()` function and change the generated test code. + + For example: + + ```golang + for _, test := range tests { + t.Run(test.name, func(tt *testing.T) { + defer goleak.VerifyNone(tt, goleakIgnoreOptions...) + + // insert your defer function here + defer func(w want, tt *testing.T) { + // implement your defer func logic + if err:= recover(); err != nil { + // check the panic + } + }(test.want, tt) + + if test.beforeFunc != nil { + test.beforeFunc(test.args) + } + // generated test code + ``` diff --git a/internal/safety/safety_test.go b/internal/safety/safety_test.go index 0815b310c0..d51fb4ea07 100644 --- a/internal/safety/safety_test.go +++ b/internal/safety/safety_test.go @@ -24,65 +24,138 @@ import ( "go.uber.org/goleak" ) +var ( + // Goroutine leak is detected by `fastime`, but it should be ignored in the test because it is an external package. + goleakIgnoreOptions = []goleak.Option{ + goleak.IgnoreTopFunction("github.com/kpango/fastime.(*Fastime).StartTimerD.func1"), + } +) + +func init() { + log.Init() +} + func TestRecoverFunc(t *testing.T) { + type args struct { + fn func() error + } + type want struct { + want func() error + wantPanic func() error + } type test struct { name string - fn func() error - runtimeErr bool - want error + args args + want want + checkFunc func(want, func() error) error + beforeFunc func(args) + afterFunc func(args) } + defaultCheckFunc := func(w want, got func() error) error { + gotErr := got() + + // if wantPanic is not nil then the panic should be recovered and this line should not be executed + if w.wantPanic != nil { + return errors.Errorf("wantPanic is not nil, but got return error: %v", gotErr) + } + if (w.want == nil && got != nil) || (w.want != nil && got == nil) { + return errors.Errorf("got = %v, want %v", got, w) + } + wantErr := w.want() + if !errors.Is(gotErr, wantErr) { + return errors.Errorf("got error= %v, want error= %v", gotErr, wantErr) + } + return nil + } tests := []test{ { name: "returns error when system paniced caused by runtime error", - fn: func() error { - _ = []string{}[10] - return nil + args: args{ + fn: func() error { + _ = []string{}[10] + return nil + }, + }, + want: want{ + wantPanic: func() error { + return errors.New("system paniced caused by runtime error: runtime error: index out of range [10] with length 0") + }, }, - runtimeErr: true, - want: errors.New("system paniced caused by runtime error: runtime error: index out of range [10] with length 0"), }, - { name: "returns error when system paniced caused by panic with string value", - fn: func() error { - panic("panic") + args: args{ + fn: func() error { + panic("panic") + }, + }, + want: want{ + want: func() error { + return errors.New("panic recovered: panic") + }, }, - want: errors.New("panic recovered: panic"), }, - { name: "returns error when system paniced caused by panic with error", - fn: func() error { - panic(errors.Errorf("error")) + args: args{ + fn: func() error { + panic(errors.Errorf("error")) + }, + }, + want: want{ + want: func() error { + return errors.Errorf("error") + }, }, - want: errors.New("error"), }, - { name: "returns error when system paniced caused by panic with int value", - fn: func() error { - panic(10) + args: args{ + fn: func() error { + panic(10) + }, + }, + want: want{ + want: func() error { + return errors.New("panic recovered: 10") + }, }, - want: errors.New("panic recovered: 10"), }, } - log.Init() + for _, test := range tests { + t.Run(test.name, func(tt *testing.T) { + defer goleak.VerifyNone(tt, goleakIgnoreOptions...) - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - defer func() { - if ok := tt.runtimeErr; ok { - if want, got := tt.want, recover().(error); !errors.Is(got, want) { - t.Errorf("not equals. want: %v, got: %v", want, got) - } + defer func(w want, tt *testing.T) { + gotPanic := recover() + if w.wantPanic == nil && gotPanic == nil { + return } - }() + panicErr, ok := gotPanic.(error) + if !ok { + tt.Errorf("cannot cast panic to error, panic: %v", gotPanic) + return + } + if want := w.wantPanic(); !errors.Is(want, panicErr) { + tt.Errorf("want: %v, got: %v", want, panicErr) + } + }(test.want, tt) - got := RecoverFunc(tt.fn)() - if !errors.Is(got, tt.want) { - t.Errorf("not equals. want: %v, got: %v", tt.want, got) + if test.beforeFunc != nil { + test.beforeFunc(test.args) + } + if test.afterFunc != nil { + defer test.afterFunc(test.args) + } + if test.checkFunc == nil { + test.checkFunc = defaultCheckFunc + } + + got := RecoverFunc(test.args.fn) + if err := test.checkFunc(test.want, got); err != nil { + tt.Errorf("error = %v", err) } }) } @@ -139,7 +212,7 @@ func TestRecoverWithoutPanicFunc(t *testing.T) { for _, test := range tests { t.Run(test.name, func(tt *testing.T) { - defer goleak.VerifyNone(t) + defer goleak.VerifyNone(tt) if test.beforeFunc != nil { test.beforeFunc(test.args) } @@ -213,7 +286,7 @@ func Test_recoverFunc(t *testing.T) { for _, test := range tests { t.Run(test.name, func(tt *testing.T) { - defer goleak.VerifyNone(t) + defer goleak.VerifyNone(tt) if test.beforeFunc != nil { test.beforeFunc(test.args) }