Skip to content

Conversation

@tscolari
Copy link
Contributor

We noticed that the travis output can be a bit misleading because it
says that it's running the tests against go 1.9 but the actual version
inside the docker image is 1.7.

Signed-off-by: Danail Branekov [email protected]

@mtrmac
Copy link
Collaborator

mtrmac commented Sep 28, 2017

Yeah, it’s reporting the Go version outside the container, not inside.

Updating from 1.7 to 1.8 is fine with me, though I worry a little about using what seems to be someone’s unofficial project, and about always using the latest version. We do want the library to work on common distributions (at least RHEL in my case) and always updating to the latest version might let us commit something which can’t be built on the targets platforms.

@runcom ?

[Or maybe we just need to use a centos image with 1.8 😎 ]

@tscolari tscolari force-pushed the run-tests-on-latest-goversion branch from 139ea97 to ec19496 Compare September 29, 2017 08:39
@tscolari tscolari changed the title Update test image to always use latest go version Update test image to always use go version 1.8 Sep 29, 2017
@tscolari
Copy link
Contributor Author

That makes a lot of sense.
I've updated the PR to use go version 1.8, and install it through the official golang release tar.

Centos is also a good option ^^, but I leave that for you to decide haha

@runcom
Copy link
Member

runcom commented Sep 29, 2017

LGTM if Travis is happy

Approved with PullApprove

@tscolari tscolari force-pushed the run-tests-on-latest-goversion branch 2 times, most recently from 41de5f8 to a37565c Compare September 29, 2017 15:10
@tscolari
Copy link
Contributor Author

So, I had to change the PATH configuration.
Because travis runs as an unknown user, we can't set the PATH on the Dockerfile, but linking the go binary works fine.

@mtrmac
Copy link
Collaborator

mtrmac commented Sep 30, 2017

The logs report gofmt: command not found. Would it be easy enough to cleanly override PATH in the Dockerfile?

@tscolari
Copy link
Contributor Author

tscolari commented Oct 2, 2017

Setting the PATH works only for the running container user of the Dockerfile (root as default).
Because on .travis.yml this is set: --user $(id -u):$(id -g) that user won't have the PATH, unless that ui/gid is predictable and then it's possible to set it as USER in the DockerFile.

Is there any reason to run the tests as $(id -u):$(id -g)?

@tscolari tscolari force-pushed the run-tests-on-latest-goversion branch from a37565c to 1af405b Compare October 2, 2017 09:14
@mtrmac
Copy link
Collaborator

mtrmac commented Oct 2, 2017

👍

This was added when converting the tests from non-containerized to containerized, so I guess the goal was to not change this aspect at that time.

Anyway, you’re right that setting PATH would be more hassle. Could you rebase this one last time, please?

Approved with PullApprove

Signed-off-by: Danail Branekov <[email protected]>
Signed-off-by: Will Martin <[email protected]>
@tscolari tscolari force-pushed the run-tests-on-latest-goversion branch from 1af405b to 43ae04b Compare October 3, 2017 10:52
@tscolari
Copy link
Contributor Author

tscolari commented Oct 3, 2017

done 👍

@mtrmac
Copy link
Collaborator

mtrmac commented Oct 3, 2017

Thanks!

@mtrmac mtrmac merged commit cabb6ce into containers:master Oct 3, 2017
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