Skip to content

Enable tests for all platforms#220

Merged
estesp merged 3 commits into
containerd:mainfrom
dmcgowan:enable-tests-all-platforms
Apr 22, 2023
Merged

Enable tests for all platforms#220
estesp merged 3 commits into
containerd:mainfrom
dmcgowan:enable-tests-all-platforms

Conversation

@dmcgowan

Copy link
Copy Markdown
Member

Try to run tests on more platforms

@dmcgowan dmcgowan force-pushed the enable-tests-all-platforms branch from 95e1039 to fd0d355 Compare April 20, 2023 19:29
Signed-off-by: Derek McGowan <derek@mcg.dev>

@samuelkarp samuelkarp left a comment

Copy link
Copy Markdown
Member

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.

Comment thread fs/copy_unix.go Outdated
func copyXAttrs(dst, src string, excludes map[string]struct{}, errorHandler XAttrErrorHandler) error {
xattrKeys, err := sysx.LListxattr(src)
if err != nil {
if os.IsPermission(err) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
if os.IsPermission(err) {
if os.IsPermission(err) && runtime.GOOS == "darwin" {

?

@samuelkarp

Copy link
Copy Markdown
Member

It'd also be good to test more filesystems: #172

Signed-off-by: Derek McGowan <derek@mcg.dev>
Signed-off-by: Derek McGowan <derek@mcg.dev>
@dmcgowan dmcgowan force-pushed the enable-tests-all-platforms branch from 41738e0 to b449cd0 Compare April 21, 2023 04:11
@dmcgowan

Copy link
Copy Markdown
Member Author

@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.

@samuelkarp

Copy link
Copy Markdown
Member

I opened #221 to track adding coverage for more filesystems.

@estesp estesp merged commit 58654a2 into containerd:main Apr 22, 2023
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

Successfully merging this pull request may close these issues.

3 participants