Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix failed test case of internal/safety package #464

Merged
merged 10 commits into from
Jun 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 94 additions & 0 deletions docs/contributing/coding-style.md
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
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{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[golangci] reported by reviewdog 🐶
goleakIgnoreOptions is a global variable (gochecknoglobals)

goleak.IgnoreTopFunction("github.com/kpango/fastime.(*Fastime).StartTimerD.func1"),
}
)

func init() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[golangci] reported by reviewdog 🐶
don't use init function (gochecknoinits)

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