-
Notifications
You must be signed in to change notification settings - Fork 67
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
Enable tests for all platforms #220
Conversation
95e1039
to
fd0d355
Compare
Signed-off-by: Derek McGowan <[email protected]>
fd0d355
to
41738e0
Compare
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've been meaning to do FreeBSD tests for this too on Cirrus, though that can be a separate PR.
fs/copy_unix.go
Outdated
@@ -71,6 +71,10 @@ func copyFileContent(dst, src *os.File) error { | |||
func copyXAttrs(dst, src string, excludes map[string]struct{}, errorHandler XAttrErrorHandler) error { | |||
xattrKeys, err := sysx.LListxattr(src) | |||
if err != nil { | |||
if os.IsPermission(err) { |
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.
if os.IsPermission(err) { | |
if os.IsPermission(err) && runtime.GOOS == "darwin" { |
?
It'd also be good to test more filesystems: #172 |
Signed-off-by: Derek McGowan <[email protected]>
Signed-off-by: Derek McGowan <[email protected]>
41738e0
to
b449cd0
Compare
@samuelkarp agreed, would be a little bit more involved. I'm not completely sure about the Windows test, at least what has changed, we should at least have the tests pass for our CI environment. Expanding to more environments would be good though given how the fs library is used. |
I opened #221 to track adding coverage for more filesystems. |
Try to run tests on more platforms