-
Notifications
You must be signed in to change notification settings - Fork 567
Explicitly select Java8 via Jabba before running integration tests #1202
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
Conversation
…before running integration tests
|
Confirmed that after this change we no longer see failures in DSE integration tests around an inability to support Java11 |
msmygit
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
| . ${HOME}/environment.txt | ||
| set +o allexport | ||
| . ${JABBA_SHELL} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: should we extract this as a method and reuse it where needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps, but there are some downsides to that approach. Each of these shell blocks start up with zero context so you have to configure everything each time. There are some config options in the openstack-jenkins-drivers repo which provides tools to help a bit with this; that's where environment.txt is built, for instance. So we could add the function there and just call it here; that would be the easiest way to make it fairly common.
Problem with that is that your Jenkinsfile is now more explicitly tied to the configuration of the runner. We already have numerous other cases of that in the build right now so it's not like you're creating a new dependency there. But if we were wanting to change that I'd want it to go in the other way; I'd rather have this build be more self-contained so that we can get as close as possible to the idea of taking this Jenkinsfile, dropping it in a new Jenkins install and largely having what we need. We'll never have that completely; there's too much config built into openstack-jenkins-drivers as-is. But I'd like to at least keep moving in that direction.
joao-r-reis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Idk enough about the CI situation for this driver to know if we're now back to the state we were in before the jenkins upgrade irt the test failures but the changes lgtm.
…before running integration tests (apache#1202)
Fix for unexpected test issues after recent Jenkins upgrade forced us to move to Java11 as the default for all runners