Rewrite Foreman's testing to a matrix #523
Conversation
ekohl
force-pushed
the
rewrite-foreman-to-matrix
branch
from
8e514a0 to
b6c2740
Compare
ekohl
force-pushed
the
rewrite-foreman-to-matrix
branch
3 times, most recently
from
c4b125a to
d147dd6
Compare
| axes { | ||
| axis { | ||
| name 'RUBY_VER' | ||
| values '2.7.6' |
There was a problem hiding this comment.
I still struggle with this and variables, but I really didn't want to also implement https://www.jenkins.io/blog/2019/12/02/matrix-building-with-scripted-pipeline/. On the other hand, how we typically write our pipelines is actually more scripted anyway.
There was a problem hiding this comment.
What's the struggle here? That you can't define it in pipelines/vars/foreman/nightly.groovy and then say values my_variable?
There was a problem hiding this comment.
That's indeed my struggle. AFAIK you can't use a variable here and need to hardcode it. Then you could use filtering to bring it down, but that feels so complicated.
There was a problem hiding this comment.
We have a couple of parallel <weird_Map_that_defines_the_branches> in our repo, and I hate it… 👍 for not doing that here, TBH
There was a problem hiding this comment.
We can also do what katello did and simply remove Ruby from the testing matrix and only test on a single version. Then we can declare it a simple env var and use a variable again.
There was a problem hiding this comment.
I think for source creation that's an OK strategy. While it's more correct to test the matrix of Ruby versions, as you noted, the matrix method in Jenkins pipelines is annoying due to hard-coding.
|
@ehelms @evgeni my goal is to also replace the old When I look at |
| def databaseUrlForTask(task) { | ||
| return task == 'assets:precompile' ? 'production' : 'test' | ||
| } | ||
|
|
||
| def databaseFile(id) { |
There was a problem hiding this comment.
is this still used somewhere then?
There was a problem hiding this comment.
Do you want me to also replace that?
There was a problem hiding this comment.
theforeman.org/pipelines/release/source/katello.groovy does use this, but I guess it'd get the same treatment once this PR is done and we see it working fine
There was a problem hiding this comment.
Unrelated, but I had a closer look at the other functions and noticed addSettings was also unused: #524
ekohl
force-pushed
the
rewrite-foreman-to-matrix
branch
from
d147dd6 to
9de0441
Compare
| environment { | ||
| BUNDLE_WITHOUT = 'development' | ||
| RAILS_ENV = railsEnvForTask(TASK) | ||
| DATABASE_URL = databaseUrlForTask(TASK, env) |
There was a problem hiding this comment.
I suspect you're not allowed to pass in env here:
java.lang.IllegalArgumentException: One or more variables have some issues with their values: DATABASE_URL
Thoughts on just using some UUID for the DB name?
ekohl
force-pushed
the
rewrite-foreman-to-matrix
branch
from
6dfbd0d to
750d7f1
Compare
|
I manually applied the current changes and https://ci.theforeman.org/blue/organizations/jenkins/foreman-develop-source-release/detail/foreman-develop-source-release/2484/pipeline/118 at least passed with them. |
ekohl
force-pushed
the
rewrite-foreman-to-matrix
branch
from
750d7f1 to
8211590
Compare
|
Mhh, https://ci.theforeman.org/job/foreman-develop-source-release/ is failing, and the number of tests is different for the passed jobs? 🤔 |
| database = UUID.randomUUID().toString() | ||
| return "postgresql://foreman:foreman@localhost/foreman-${database}" |
There was a problem hiding this comment.
somehow all tests end up using the same DB. and that breaks things
| RAILS_ENV = railsEnvForTask(RAKE_TASK) | ||
| DATABASE_URL = databaseUrlForTask(RAKE_TASK) |
There was a problem hiding this comment.
I think this gets evaluated too early?
There was a problem hiding this comment.
https://www.jenkins.io/doc/book/pipeline/syntax/#matrix-cell-directives says you can also use it in the matrix itself, which is probably way cleaner. It could even be unused outside of it.
This makes it easier to change the Ruby versions and also test on multiple at the same time.
ekohl
force-pushed
the
rewrite-foreman-to-matrix
branch
from
8211590 to
c99d80a
Compare
This makes it easier to change the Ruby versions and also test on multiple at the same time.