-
-
Notifications
You must be signed in to change notification settings - Fork 136
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
Update and enhance the test environment #215
Conversation
Looks good, but is it worth dropping old systems, instead of keeping them and adding newer ones? |
Sorry, I forgot to give my rationale about this proposition of removals. It is mainly motivated by the fact that these versions are (or will "soon" be) end of life (see summary and references below), and thus EOL Dates Summary:
Please let me know the list of systems you want to keep in the CI setup, and I'll update it. References: |
@pkuczynski I added c5594e1 as an example, but I'll adapt it as soon as you decide which versions you want to keep in the test matrix. |
13cd233
to
70ed343
Compare
Is it overly aggressive to drop OS versions that are still supported upstream? Should we wait until they become EOL and put up quick PRs to remove them at that time? |
As I said, your list will be my list 😉 PS: I am baking a next PR adding support for Arch Linux 😇. |
I would keep testing old systems even beyond the EOL as long as it's not too costly. There has been many cases in RVM where people took ownership of some very old project and they had to maintain it before they could upgrade it. In this case, having the older systems does not cost much, so I would keep them in. |
70ed343
to
dea7aae
Compare
dea7aae
to
00c1252
Compare
😒 second time that the Travis build fails due to "transient problems". See here an example of rate limiting issue on https://github.com. I'll give a try to parallelise a little bit more. Not only via Ansible from the same run, but with the different travis jobs to try to avoid such problems, and speed up the build time. |
d2fab66
to
a1bd852
Compare
I think that a1bd852 is a good move. This will
|
d5c230f
to
bed3733
Compare
@pkuczynski I think this PR is now ready for your review. |
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.
Overall it looks good to me, with few minor comments. @thbar what you think?
tests/docker-compose.yml
Outdated
build: ./dockerfiles/centos6 | ||
cap_add: [ALL] | ||
|
||
debian10: |
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.
Same here, keep ASC version ordering
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.
up to you, but please my comment below, and "double confirm" your decision: ASC (or DESC).
Thank you
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.
Done in 6df0663. Please confirm it 😉
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.
@pkuczynski we keep the Ascending order, right?
Possibly we could consider moving to Github Actions in case Travis is not being fast or stable... |
@pkuczynski about ASC vs DESC ordering in I intentionally reversed it, with the intention to put the most recent stuff on the top (as you typically do with release notes in a CHANGELOG file). At this occasion, the diff looks a little bit weird, but it won't be the case in the future (if we keep the convention). If you prefer the ASC sorting convention, no problem, but I just needed to give you these details. |
In this particular case, I don't think that Travis is performing bad at all (I faced similar problems on my local testing, but didn't want to tackle this problem in the scope of this PR). That said, moving to GitHub Actions could be an interesting move, but certainly in a distinct pull request 😉 |
I did move some of my projects from Travis to GHA and it was pretty smooth yet seemed to be much faster than Travis. Should be done in a separate PR of course... |
Yeah, I agree. If this supposed to update the test env, it should also make sure that it's green. @gildegoma can you try to fix the failing build? |
I have prepared a patch for this, Edit: Finally, this will be part of this PR (was good to use to demonstrate the new caching setup). See f5d48d3 |
About the Travis CI failures... These are transient problems (that partially motivated me to split the jobs, so one can restart the failing parts without having to wait for a full rebuild. We'll come back onto this when we'll study a migration to GitHub Actions,... For the records, I put here some comments about the two jobs that failed (#395.1 and #395.2). 395.1: The rvm-installer script couldn't be download (from raw.githubusercontent.com, which returned a 502 error). I'll propose an until-retries-delay construct for the related get_url task (but again outside of the scope of this PR). Same fix could be applied elsewhere 395.2: This one is quite strange (never seen in previous builds). It looks like docker-compose detects a syntax error in the |
- Add more recent version of CentOS, Ubuntu and Debian - Integrate tests/root.yml in Travis CI build (not only user.yml) - Use latest versions of Ruby, Ansible and docker-compose in Travis CI build environment. This change fixes failing tests due to Bundler recent versions being incompatible with Ruby 2.2 and 2.3. - Minor changes in the tests playbooks (name plays and add tags)
The new assertion logic aims to validate rvm behaviours in a more abstract fashion, instead of directly hitting some installed files (which locations depends on RVM internals, i.e. current implementation).
9227e45
to
8ce4e84
Compare
⏫ rebased to amend my previous commits (I had GPG signing disabled on my current laptop). |
a4b671a
to
5dfdab6
Compare
5dfdab6
to
943ff23
Compare
@Kulgar @thbar @pkuczynski I think I have addressed the requested changes, and added a few more improvements. |
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.
@gildegoma excellent! Indeed I definitely vote to merge as soon as possible, because it will have great impacts on the rest of the PR (testing etc). I had a quick look today and it looks good to me. Thanks for your work on this!
I see that @pkuczynski already reviewed, so that's 3 pair of eyes, I'll go ahead and merge. |
As a prerequisite to further works (e.g. rebasing #52), I propose here to refresh the test environment with following changes:
tests/assertions.yml
to provide stronger validation of rvm behaviourstests/root.yml
in Travis CI build