From bfa2efd8fe3f836475bf19f8678b52a852f9d708 Mon Sep 17 00:00:00 2001 From: kevindiu Date: Wed, 10 Jun 2020 16:18:16 +0900 Subject: [PATCH 01/10] fix test case --- internal/safety/safety_test.go | 258 ++++++++++++--------------------- 1 file changed, 92 insertions(+), 166 deletions(-) diff --git a/internal/safety/safety_test.go b/internal/safety/safety_test.go index 0815b310c0..720a3b8d64 100644 --- a/internal/safety/safety_test.go +++ b/internal/safety/safety_test.go @@ -16,7 +16,6 @@ package safety import ( - "reflect" "testing" "github.com/vdaas/vald/internal/errors" @@ -24,76 +23,24 @@ import ( "go.uber.org/goleak" ) -func TestRecoverFunc(t *testing.T) { - type test struct { - name string - fn func() error - runtimeErr bool - want error - } - - tests := []test{ - { - name: "returns error when system paniced caused by runtime error", - fn: func() error { - _ = []string{}[10] - return nil - }, - 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") - }, - want: errors.New("panic recovered: panic"), - }, - - { - name: "returns error when system paniced caused by panic with error", - fn: func() error { - panic(errors.Errorf("error")) - }, - want: errors.New("error"), - }, - - { - name: "returns error when system paniced caused by panic with int value", - fn: func() error { - panic(10) - }, - want: errors.New("panic recovered: 10"), - }, +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() - - 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) - } - } - }() - - got := RecoverFunc(tt.fn)() - if !errors.Is(got, tt.want) { - t.Errorf("not equals. want: %v, got: %v", tt.want, got) - } - }) - } } -func TestRecoverWithoutPanicFunc(t *testing.T) { +func TestRecoverFunc(t *testing.T) { type args struct { fn func() error } type want struct { - want func() error + want func() error + wantPanic func() error } type test struct { name string @@ -104,116 +51,92 @@ func TestRecoverWithoutPanicFunc(t *testing.T) { afterFunc func(args) } defaultCheckFunc := func(w want, got func() error) error { - if !reflect.DeepEqual(got, w.want) { - return errors.Errorf("got = %v, want %v", got, w.want) + if (w.want == nil && got != nil) || (w.want != nil && got == nil) { + return errors.Errorf("got = %v, want %v", got, w) + } + gotErr := got() + wantErr := w.want() + if !errors.Is(gotErr, wantErr) { + return errors.Errorf("got error= %v, want error= %v", gotErr, wantErr) } return nil } tests := []test{ - // TODO test cases - /* - { - name: "test_case_1", - args: args { - fn: nil, - }, - want: want{}, - checkFunc: defaultCheckFunc, - }, - */ - - // TODO test cases - /* - func() test { - return test { - name: "test_case_2", - args: args { - fn: nil, - }, - want: want{}, - checkFunc: defaultCheckFunc, - } - }(), - */ + { + name: "returns error when system paniced caused by runtime error", + 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") + }, + }, + }, + { + name: "returns error when system paniced caused by panic with string value", + args: args{ + fn: func() error { + panic("panic") + }, + }, + want: want{ + want: func() error { + return errors.New("panic recovered: panic") + }, + }, + }, + { + name: "returns error when system paniced caused by panic with error", + args: args{ + fn: func() error { + panic(errors.Errorf("error")) + }, + }, + want: want{ + want: func() error { + return errors.Errorf("error") + }, + }, + }, + { + name: "returns error when system paniced caused by panic with int value", + args: args{ + fn: func() error { + panic(10) + }, + }, + want: want{ + want: func() error { + return errors.New("panic recovered: 10") + }, + }, + }, } for _, test := range tests { t.Run(test.name, func(tt *testing.T) { - defer goleak.VerifyNone(t) - 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 := RecoverWithoutPanicFunc(test.args.fn) - if err := test.checkFunc(test.want, got); err != nil { - tt.Errorf("error = %v", err) - } - - }) - } -} - -func Test_recoverFunc(t *testing.T) { - type args struct { - fn func() error - withPanic bool - } - type want struct { - want func() error - } - type test struct { - name string - args args - want want - checkFunc func(want, func() error) error - beforeFunc func(args) - afterFunc func(args) - } - defaultCheckFunc := func(w want, got func() error) error { - if !reflect.DeepEqual(got, w.want) { - return errors.Errorf("got = %v, want %v", got, w.want) - } - return nil - } - tests := []test{ - // TODO test cases - /* - { - name: "test_case_1", - args: args { - fn: nil, - withPanic: false, - }, - want: want{}, - checkFunc: defaultCheckFunc, - }, - */ + defer goleak.VerifyNone(tt, goleakIgnoreOptions...) - // TODO test cases - /* - func() test { - return test { - name: "test_case_2", - args: args { - fn: nil, - withPanic: false, - }, - want: want{}, - checkFunc: defaultCheckFunc, - } - }(), - */ - } + 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) + return + } + }(test.want, tt) - for _, test := range tests { - t.Run(test.name, func(tt *testing.T) { - defer goleak.VerifyNone(t) if test.beforeFunc != nil { test.beforeFunc(test.args) } @@ -224,11 +147,14 @@ func Test_recoverFunc(t *testing.T) { test.checkFunc = defaultCheckFunc } - got := recoverFunc(test.args.fn, test.args.withPanic) - if err := test.checkFunc(test.want, got); err != nil { - tt.Errorf("error = %v", err) + got := RecoverFunc(test.args.fn) + if test.want.wantPanic == nil { + if err := test.checkFunc(test.want, got); err != nil { + tt.Errorf("error = %v", err) + } + } else { + got() } - }) } } From fb2c5964ea6852e2a29c35ead39c7b950c7cfad0 Mon Sep 17 00:00:00 2001 From: kevindiu Date: Wed, 10 Jun 2020 16:24:47 +0900 Subject: [PATCH 02/10] fix --- internal/safety/safety_test.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/internal/safety/safety_test.go b/internal/safety/safety_test.go index 720a3b8d64..94c37edbfa 100644 --- a/internal/safety/safety_test.go +++ b/internal/safety/safety_test.go @@ -51,10 +51,16 @@ func TestRecoverFunc(t *testing.T) { 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) } - gotErr := got() wantErr := w.want() if !errors.Is(gotErr, wantErr) { return errors.Errorf("got error= %v, want error= %v", gotErr, wantErr) @@ -133,7 +139,6 @@ func TestRecoverFunc(t *testing.T) { } if want := w.wantPanic(); !errors.Is(want, panicErr) { tt.Errorf("want: %v, got: %v", want, panicErr) - return } }(test.want, tt) @@ -148,12 +153,8 @@ func TestRecoverFunc(t *testing.T) { } got := RecoverFunc(test.args.fn) - if test.want.wantPanic == nil { - if err := test.checkFunc(test.want, got); err != nil { - tt.Errorf("error = %v", err) - } - } else { - got() + if err := test.checkFunc(test.want, got); err != nil { + tt.Errorf("error = %v", err) } }) } From 07c3a51a7073d353297fa630c66db7e0cb422369 Mon Sep 17 00:00:00 2001 From: kevindiu Date: Wed, 10 Jun 2020 16:48:26 +0900 Subject: [PATCH 03/10] genereate missing test code --- internal/safety/safety_test.go | 146 +++++++++++++++++++++++++++++++++ 1 file changed, 146 insertions(+) diff --git a/internal/safety/safety_test.go b/internal/safety/safety_test.go index 94c37edbfa..d51fb4ea07 100644 --- a/internal/safety/safety_test.go +++ b/internal/safety/safety_test.go @@ -16,6 +16,7 @@ package safety import ( + "reflect" "testing" "github.com/vdaas/vald/internal/errors" @@ -159,3 +160,148 @@ func TestRecoverFunc(t *testing.T) { }) } } + +func TestRecoverWithoutPanicFunc(t *testing.T) { + type args struct { + fn func() error + } + type want struct { + want func() error + } + type test struct { + name string + args args + want want + checkFunc func(want, func() error) error + beforeFunc func(args) + afterFunc func(args) + } + defaultCheckFunc := func(w want, got func() error) error { + if !reflect.DeepEqual(got, w.want) { + return errors.Errorf("got = %v, want %v", got, w.want) + } + return nil + } + tests := []test{ + // TODO test cases + /* + { + name: "test_case_1", + args: args { + fn: nil, + }, + want: want{}, + checkFunc: defaultCheckFunc, + }, + */ + + // TODO test cases + /* + func() test { + return test { + name: "test_case_2", + args: args { + fn: nil, + }, + want: want{}, + checkFunc: defaultCheckFunc, + } + }(), + */ + } + + for _, test := range tests { + t.Run(test.name, func(tt *testing.T) { + defer goleak.VerifyNone(tt) + 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 := RecoverWithoutPanicFunc(test.args.fn) + if err := test.checkFunc(test.want, got); err != nil { + tt.Errorf("error = %v", err) + } + + }) + } +} + +func Test_recoverFunc(t *testing.T) { + type args struct { + fn func() error + withPanic bool + } + type want struct { + want func() error + } + type test struct { + name string + args args + want want + checkFunc func(want, func() error) error + beforeFunc func(args) + afterFunc func(args) + } + defaultCheckFunc := func(w want, got func() error) error { + if !reflect.DeepEqual(got, w.want) { + return errors.Errorf("got = %v, want %v", got, w.want) + } + return nil + } + tests := []test{ + // TODO test cases + /* + { + name: "test_case_1", + args: args { + fn: nil, + withPanic: false, + }, + want: want{}, + checkFunc: defaultCheckFunc, + }, + */ + + // TODO test cases + /* + func() test { + return test { + name: "test_case_2", + args: args { + fn: nil, + withPanic: false, + }, + want: want{}, + checkFunc: defaultCheckFunc, + } + }(), + */ + } + + for _, test := range tests { + t.Run(test.name, func(tt *testing.T) { + defer goleak.VerifyNone(tt) + 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, test.args.withPanic) + if err := test.checkFunc(test.want, got); err != nil { + tt.Errorf("error = %v", err) + } + + }) + } +} From 084e3045cc5f790e25b389af3ec2f408538300bb Mon Sep 17 00:00:00 2001 From: kevindiu Date: Thu, 11 Jun 2020 16:26:00 +0900 Subject: [PATCH 04/10] add coding doc --- docs/contributing/coding-style.md | 79 +++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/docs/contributing/coding-style.md b/docs/contributing/coding-style.md index 055e0820d0..fdec8e6e70 100644 --- a/docs/contributing/coding-style.md +++ b/docs/contributing/coding-style.md @@ -509,3 +509,82 @@ 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 want to generate test code, please use the following command. + +```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. + +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 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) { + 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...) + + 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 + ``` From ea1c9c5fe7f9a070550e72e3fdb828c53e040fea Mon Sep 17 00:00:00 2001 From: kevindiu Date: Thu, 11 Jun 2020 16:37:36 +0900 Subject: [PATCH 05/10] fix doc --- docs/contributing/coding-style.md | 121 ++++++++++++++++-------------- 1 file changed, 63 insertions(+), 58 deletions(-) diff --git a/docs/contributing/coding-style.md b/docs/contributing/coding-style.md index fdec8e6e70..36f25db55c 100644 --- a/docs/contributing/coding-style.md +++ b/docs/contributing/coding-style.md @@ -518,7 +518,7 @@ We implement our own [gotests](https://github.com/cweill/gotests) template to ge make gotests/install ``` -If you want to generate test code, please use the following command. +If you use the following command to generate the missing test code. ```bash make make gotests/gen @@ -530,61 +530,66 @@ The test code generated follows the table-driven test format. You can implement 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 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) { - 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...) - - 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 - ``` +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. + +2. 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 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...) +``` + +3. 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 +``` From 220e01ed2639ba6a7e6b683cabb1bcff61da8421 Mon Sep 17 00:00:00 2001 From: Kevin Diu Date: Thu, 11 Jun 2020 17:03:54 +0900 Subject: [PATCH 06/10] Apply suggestions from code review Co-authored-by: Hiroto Funakoshi --- docs/contributing/coding-style.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/contributing/coding-style.md b/docs/contributing/coding-style.md index 36f25db55c..a256fbf99f 100644 --- a/docs/contributing/coding-style.md +++ b/docs/contributing/coding-style.md @@ -512,7 +512,7 @@ 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. +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 @@ -545,7 +545,7 @@ and place it on the header of the test file. 2. 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 need to create the following variable to store the ignore function. +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 ( From 0f90ca54e0bae0469dc841ba6638e904730663f6 Mon Sep 17 00:00:00 2001 From: Kevin Diu Date: Mon, 15 Jun 2020 14:13:50 +0900 Subject: [PATCH 07/10] Apply suggestions from code review Co-authored-by: Kiichiro YUKAWA --- docs/contributing/coding-style.md | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/docs/contributing/coding-style.md b/docs/contributing/coding-style.md index a256fbf99f..6f85505c1b 100644 --- a/docs/contributing/coding-style.md +++ b/docs/contributing/coding-style.md @@ -512,7 +512,8 @@ 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. +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 @@ -525,13 +526,15 @@ 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. +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. 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. +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: @@ -541,11 +544,13 @@ func init() { } ``` -and place it on the header of the test file. +And place it on the header of the test file. 2. 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. +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 ( @@ -573,7 +578,10 @@ for _, test := range tests { 3. 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: +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 { From f987feed9bd09313320c973fdbe2b86b69794d43 Mon Sep 17 00:00:00 2001 From: Kevin Diu Date: Mon, 15 Jun 2020 14:14:17 +0900 Subject: [PATCH 08/10] Apply suggestions from code review Co-authored-by: Kiichiro YUKAWA --- docs/contributing/coding-style.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/contributing/coding-style.md b/docs/contributing/coding-style.md index 6f85505c1b..cef39aa0b7 100644 --- a/docs/contributing/coding-style.md +++ b/docs/contributing/coding-style.md @@ -546,7 +546,7 @@ func init() { And place it on the header of the test file. -2. goleak option +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. @@ -576,7 +576,7 @@ for _, test := range tests { defer goleak.VerifyNone(tt, goleakIgnoreOptions...) ``` -3. Defer function +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. From 562f84d16b695ed5f62d73e396ca20f203e19022 Mon Sep 17 00:00:00 2001 From: kevindiu Date: Mon, 15 Jun 2020 14:20:21 +0900 Subject: [PATCH 09/10] fix formatting --- docs/contributing/coding-style.md | 106 +++++++++++++++--------------- 1 file changed, 53 insertions(+), 53 deletions(-) diff --git a/docs/contributing/coding-style.md b/docs/contributing/coding-style.md index cef39aa0b7..46921b9be3 100644 --- a/docs/contributing/coding-style.md +++ b/docs/contributing/coding-style.md @@ -533,71 +533,71 @@ We do not suggest to modify the generated code other than the `tests` variable, 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() -} -``` + 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. + 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. + 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"), - } -) -``` + ```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. + And modify the generated test code. -```golang -// before -for _, test := range tests { - t.Run(test.name, func(tt *testing.T) { - defer goleak.VerifyNone(tt) + ```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...) -``` + // 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. + 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: + For example: -```golang -for _, test := range tests { - t.Run(test.name, func(tt *testing.T) { - defer goleak.VerifyNone(tt, goleakIgnoreOptions...) + ```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) + // 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 -``` + if test.beforeFunc != nil { + test.beforeFunc(test.args) + } + // generated test code + ``` From 60bb2fb52079057cebf63ce14eb028158b2faa35 Mon Sep 17 00:00:00 2001 From: kevindiu Date: Mon, 15 Jun 2020 14:23:44 +0900 Subject: [PATCH 10/10] add title --- docs/contributing/coding-style.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/contributing/coding-style.md b/docs/contributing/coding-style.md index 46921b9be3..2591662eb9 100644 --- a/docs/contributing/coding-style.md +++ b/docs/contributing/coding-style.md @@ -526,9 +526,11 @@ 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. +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