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

Testsuite doesn't check regressions #150

Closed
dolmen opened this issue Nov 10, 2017 · 3 comments
Closed

Testsuite doesn't check regressions #150

dolmen opened this issue Nov 10, 2017 · 3 comments

Comments

@dolmen
Copy link
Contributor

dolmen commented Nov 10, 2017

The testsuite uses almost only t.Log instead of validating assertions and failing with t.Error, t.Fatal or t.Fail:

$ grep -E -c 'Error|Fatal|Fail|panic' *_test.go
excelize_test.go:3
sheet_test.go:0

This means that only code coverage is verified, but regressions are not checked automatically (humans have to read the go test -v output and have to know what is expected: that's not helpful for external contributors).

@dolmen
Copy link
Contributor Author

dolmen commented Nov 10, 2017

It seems that most if err != nil blocks in *_test.go files that call t.Log could call t.Fatal instead.

Before:

if err  != nil {
    t.Log(err)
}

After:

if err  != nil {
    t.Fatal(err)
}

@dolmen dolmen changed the title Testsuite (almost) never fails Testsuite doesn't check regressions Nov 10, 2017
@dolmen
Copy link
Contributor Author

dolmen commented Nov 23, 2017

@xuri Would you accept a patch that fixes this issue in the way I suggest in previous comment?

@xuri
Copy link
Member

xuri commented Jan 11, 2019

This is a late reply, this issue resolved by #322.

@xuri xuri closed this as completed Jan 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants