Skip to content

Run "make test" on Travis CI #330

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

Merged
merged 3 commits into from
Nov 15, 2019
Merged

Conversation

kzys
Copy link
Contributor

@kzys kzys commented Nov 14, 2019

"make" doesn't compile *_test.go, but "make test" does.

121afa3 introduced
ProcessState.ExitCode which is not available in Go 1.11.

Signed-off-by: Kazuyoshi Kato [email protected]

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@nmeyerhans
Copy link
Contributor

While you're in here, it's probably worth fixing the make lint failures in travis...

@sipsma
Copy link
Contributor

sipsma commented Nov 14, 2019

Good catch on that test using that new API, I really, really wish the go docs made any effort to say that a given api was introduced in a particular version (let me know if there's some way I'm not aware of).... But that is what it is.

I can submit a separate PR to check the exit code the old way so we don't have a 1.12 requirement.

For the travis build, we already run the unit tests in buildkite, so I'm wondering if all we should do in travis is go test -c which will compile the tests into a binary but not run them: https://golang.org/cmd/go/#hdr-Test_packages

If that doesn't work it's not the end of the world to just run the unit tests in travis too though.

@kzys
Copy link
Contributor Author

kzys commented Nov 14, 2019

Well, they have "1.12" on the right :)

https://golang.org/pkg/os/#ProcessState.ExitCode

Actually I would rather just drop Go 1.11 support. Firecracker Go SDK may need to support older versions, but firecracker-containerd doesn't have to, I think.

@sipsma
Copy link
Contributor

sipsma commented Nov 14, 2019

Well, they have "1.12" on the right :)

Haha! My widescreen monitor is so widescreen I have to turn my neck to look at the right side. Good to know though, thanks!

@sipsma
Copy link
Contributor

sipsma commented Nov 14, 2019

Actually I would rather just drop Go 1.11 support. Firecracker Go SDK may need to support older versions, but firecracker-containerd doesn't have to, I think.

Given the fix to this issue is very straightforward, I feel like we may as well not drop go 1.11 if we don't have to. Debian buster, for example, is still on go 1.11 by default afaik, so it might make it easier for some users to build if we don't require 1.12+

@kzys
Copy link
Contributor Author

kzys commented Nov 14, 2019

You are right. Debian... https://packages.debian.org/buster/golang

Will update the test in this PR then.

@kzys kzys force-pushed the travis-build branch 3 times, most recently from 3db2a8b to 5e31220 Compare November 14, 2019 22:41
kzys added 3 commits November 14, 2019 15:40
"make" doesn't compile *_test.go, but "make test" does.

121afa3 introduced
ProcessState.ExitCode which is not available in Go 1.11.

Signed-off-by: Kazuyoshi Kato <[email protected]>
The default timeout is 1m, and we are constantly exceeding

Signed-off-by: Kazuyoshi Kato <[email protected]>
But we still want to support Go 1.11.

Signed-off-by: Kazuyoshi Kato <[email protected]>
@kzys kzys merged commit 386eba8 into firecracker-microvm:master Nov 15, 2019
@kzys kzys deleted the travis-build branch November 15, 2019 00:26
kzys added a commit to kzys/firecracker-containerd that referenced this pull request Nov 15, 2019
The exit code is in MSB. Not so sure why it was working on firecracker-microvm#330
itself's BuildKite check though.

Signed-off-by: Kazuyoshi Kato <[email protected]>
@kzys kzys mentioned this pull request Nov 15, 2019
fangn2 pushed a commit to fangn2/firecracker-containerd that referenced this pull request Mar 23, 2023
…ependabot/go_modules/github.com/sirupsen/logrus-1.8.1

Bump github.com/sirupsen/logrus from 1.8.0 to 1.8.1
BenjaminChun pushed a commit to char-1ee/firecracker-containerd that referenced this pull request Apr 25, 2024
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