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

x/*: review tests skipped on darwin/arm64 after Go 1.17 release #45696

Open
tklauser opened this issue Apr 22, 2021 · 5 comments
Open

x/*: review tests skipped on darwin/arm64 after Go 1.17 release #45696

tklauser opened this issue Apr 22, 2021 · 5 comments

Comments

@tklauser
Copy link
Member

@tklauser tklauser commented Apr 22, 2021

Go 1.16 renamed the iOS port from darwin/arm64 to ios/arm64 anddarwin/arm64 was repurposed for the macOS ARM64 port (see https://golang.org/doc/go1.16#darwin). Thegolang.org/x/* repositories were updated to use the new ios build tag to skip certain tests on IOS, but they also needed to retain the old runtime.GOOS == "darwin" && (runtime.GOARCH == "arm64" || runtime.GOARCH == "arm") checks due to Go 1.15 compatibilty. In some cases this leads to tests being unnecessarily skipped with Go 1.16 on darwin/arm64, see e.g. https://golang.org/cl/312250.

Once Go 1.17 is released and thus Go 1.16 becomes the oldest supported release per https://golang.org/doc/devel/release.html#policy, we might be able to remove some of the old GOOS == "darwin" && (GOARCH == "arm64" || GOARCH === "arm") checks and thus increase the test coverage for the macOS ARM64 port in the golang.org/x/* repos.

Some examples:

https://go.googlesource.com/sys/+/33663a62ff0820461192226283cafe57a83ac6b7/unix/syscall_unix_test.go#169
https://go.googlesource.com/sys/+/33663a62ff0820461192226283cafe57a83ac6b7/unix/syscall_unix_test.go#491
https://go.googlesource.com/net/+/4e50805a0758a198d5c9ea96b2e8dacd7338f2ee/nettest/nettest.go#102
https://go.googlesource.com/term/+/f5beecf764ed751c785fa21705385df593b1852d/terminal_test.go#410

@tklauser tklauser added this to the Go1.18 milestone Apr 22, 2021
@justplesh
Copy link
Contributor

@justplesh justplesh commented Apr 22, 2021

Hi @tklauser, I'll work on it.

@tklauser
Copy link
Member Author

@tklauser tklauser commented Apr 22, 2021

@justplesh Thanks. Please note that we can only submit these changes once Go 1.17 is released, otherwise we'd break tests on Go 1.15 which is still supported until then. This issue for now is many here so we don't forget to review these tests once Go 1.17 is out.

@justplesh
Copy link
Contributor

@justplesh justplesh commented Apr 22, 2021

@tklauser Okay, sure, got you. How do you think, does it make sense to create the PR with changes now and merge it as soon as Go 1.17 will be released or a lot of changes will be made and it will make more sense to wait until the 1.17 release?

@tklauser
Copy link
Member Author

@tklauser tklauser commented Apr 22, 2021

I think it's probably easier to send these changes once 1.17 is released. That way we can make sure all cases are covered and it might safe some trouble with rebases and/or merge conflicts.

@justplesh
Copy link
Contributor

@justplesh justplesh commented Apr 22, 2021

Okay, I'll get back as soon, as 1.17 will be released :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants