Skip to content

Commit

Permalink
Fix failed test case of internal/safety package (#464)
Browse files Browse the repository at this point in the history 
* fix test case

* fix

* genereate missing test code

* add coding doc

* fix doc

* Apply suggestions from code review

Co-authored-by: Hiroto Funakoshi <hiroto.funakoshi.hiroto@gmail.com>

* Apply suggestions from code review

Co-authored-by: Kiichiro YUKAWA <kyukawa315@gmail.com>

* Apply suggestions from code review

Co-authored-by: Kiichiro YUKAWA <kyukawa315@gmail.com>

* fix formatting

* add title

Co-authored-by: Hiroto Funakoshi <hiroto.funakoshi.hiroto@gmail.com>
Co-authored-by: Kiichiro YUKAWA <kyukawa315@gmail.com>
  • Loading branch information
@kevindiu @hlts2 @vankichi
3 people authored Jun 15, 2020
1 parent 972e943 commit 32a86e3
Show file tree
Hide file tree
Showing 2 changed files with 201 additions and 34 deletions.
94 changes: 94 additions & 0 deletions docs/contributing/coding-style.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
```
141 changes: 107 additions & 34 deletions internal/safety/safety_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down

0 comments on commit 32a86e3

Please sign in to comment.