Fixed failing tests due to the change we check for file readability. #9
Conversation
|
I added another commit with a serving suggesting :) I strongly believe, you should not try to parse error strings. Use sentinel errors instead. Or return the errors you got from the os, if possible. And, if you do |
|
@HeikoSchlittermann, thanks for the suggestion. In tests now I use errors.Is(...) instead of parsing error strings. |
gledki_test.go
Outdated
| if errors.Is(err, os.ErrNotExist) { | ||
| t.Logf("Right error: %v", err) | ||
| } else { | ||
| t.Fatalf("Wrong error: errstr") | ||
| t.Fatalf("Wrong error: %v", err) |
There was a problem hiding this comment.
I strongly suggest not to use t.Fatal*, unless you really want to abort the test. Better to use t.Error*.
t.Error*-> t.Log + t.Fail, which marks the testcase as failed, but continuest.Fatal*-> t.Log + t.FailNow, which aborts the current test function
Same applies to the following test cases
| @@ -331,21 +332,21 @@ func TestErrors(t *testing.T) { | |||
| out.Reset() | |||
| if _, err := tpls.Execute(&out, "no_include"); err != nil { | |||
| errstr := err.Error() | |||
| if strings.Contains(errstr, "could not be read") { | |||
| t.Logf("Right error: %s", err.Error()) | |||
| if errors.Is(err, syscall.ENOENT) { | |||
There was a problem hiding this comment.
Not sure if syscall.ENOENT isn't a way to specific here, the more general os.ErrNotExist should be better suitable, IMHO. I'm not sure about portability of ENOENT (maybe other systems report an other errno, which hopefully will be translated by the Go libraries into os.ErrNotExist
There was a problem hiding this comment.
The system returns "no such file or directory". I found that this string comes from syscall.ENOENT and just used it as a sentinel error. os.ErrNotExist contains "file does not exist", which is a different string.
There was a problem hiding this comment.
The syscall package is os dependend.
GOOS=windows go doc syscall
GOOS=linux go doc syscall
e.g. shows different things. Fortunately for the current situation, syscall.ENOENT seems to be available in both cases. But do we know about other OS?
I'd suggest, using the highest abstraction available, which is os.ErrNotExist, IMHO
No description provided.