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

Integration tests fail on macOS ARM because of missing go release #1394

Closed
karelbilek opened this issue Dec 6, 2021 · 4 comments
Closed

Comments

@karelbilek
Copy link

What version of protobuf and what language are you using?
Master ( https://github.com/protocolbuffers/protobuf-go/tree/26e8bcb3c743193558d1a0ff540c9e05f999267d ), go

What did you do?
Clone the repo
Run go test -v integration_test.go

What did you expect to see?
Tests will work

What did you see instead?

~/protobuf-go$ go test -v integration_test.go
=== RUN   Test
download protobuf-3.15.3
build protobuf-3.15.3
download go1.9.7
    integration_test.go:331: gzip: invalid header

Anything else we should know about your project / environment?
I am using M1 Mac. Go 1.9.7 does not have ARM darwin release.

@karelbilek
Copy link
Author

karelbilek commented Dec 6, 2021

What works is changing all cases of runtime.GOOS in the downloaders to amd64, which downloads everything through rosetta. Or what also works is removing the old go versions; but staticcheck still needs to run through rosetta.

But then it fails on Test/Go1.16.1/PureGo anyway, with SIGABRT: abort, and then it dumps stack and registries in hexadecimal.

@puellanivis
Copy link
Collaborator

I think this is probably Working As Intend. So long as we include go1.9.7 in our integration tests (supposing that because it is declared that is is a necessarily supported version) then its failure on ARM darwin is a proper integration test failure. Lack of support is itself a form of test failure.

I suppose we might be able to sunset support for go1.9 entirely, and therefore just remove it, and then we ARM darwin would no longer be considered non-compliant for all supported versions. 🤷‍♀️

@karelbilek
Copy link
Author

I am not entirely sure if it is working as intended :)

the recommended guide in contributing.md recommends adding it as pre-push hook. Which would stop me from making a PR :)

but anyway I just ran the actual important tests and it worked there, so, I guess that's ok.

@puellanivis
Copy link
Collaborator

Well, if we say we support go1.9, and you cannot run the integration tests for go1.9, then that’s a failure. The fix isn’t necessarily to work around it, but to recognize that maybe we don’t need to be running tests for go1.9 anymore, and remove them.

I understand you want to get your PR made, but we really probably should clean up older versions in the integration tests…?

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

No branches or pull requests

2 participants