-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Tests for mfsr.go #3618
Tests for mfsr.go #3618
Conversation
37b869d
to
77e46b4
Compare
@@ -23,8 +23,8 @@ func (rp RepoPath) Version() (int, error) { | |||
} | |||
|
|||
fn := rp.VersionFile() | |||
if _, err := os.Stat(fn); os.IsNotExist(err) { | |||
return 0, VersionFileNotFound(rp) | |||
if _, err := os.Stat(fn); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why was this changed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed this because I noticed that the only caller of the method (https://github.com/Zanadar/go-ipfs/blob/77e46b40b1ec1e7799915c6dd1a4a38e9d741622/repo/fsrepo/fsrepo.go#L142) explicitly checks for os.IsNotExist(err)
and when we transform the error here, that check in the caller never returns true.
On second consideration, it may be better to check in the caller for err == VersionFileNotFound
as this error gives a little more context.
Hope that clarifies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, that makes sense then.
@Zanadar do you mind rebasing this PR on latest master? To get CI on jenkins running properly we need the new makefile changes |
License: MIT Signed-off-by: Zander Mackie <[email protected]>
- fsrepo calls and checks for this error (`fsrepo.go:140`) License: MIT Signed-off-by: Zander Mackie <[email protected]>
License: MIT Signed-off-by: Zander Mackie <[email protected]>
License: MIT Signed-off-by: Zander Mackie <[email protected]>
77e46b4
to
310c08c
Compare
Rebased. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @Zanadar !
Adds tests for
msfr.go
. Does not hit coverage target of 80%.Remove code that was transforming errs when the version file didn't exist, which may have confused callers of the
Version
method.