Parallelize GitHub Action jobs in CI

[torrocus]

Pull Request Summary

CI tests take a long time because they run different cases one after the other. If something fails, no further tests will be run. CI tests have not been passed for some time now. Fast tests didn't pass on CI for MacOS. This change is intended to help us solve this problem more easily.

Changes proposed in this pull request:

• split a long-running tests job into smaller jobs that can be parallelized

Feedback

The total CI execution time is the same, but the waiting time for the test results is shorter.

Before

After

[torrocus]

@pkuczynski what do you think about this change?

[pkuczynski]

It make sense to run things in parallel if it speeds up the tests, that's for sure. Just the code duplication is aweful. Can we somehow avoid this?

@eregon wdyt?

[eregon]

What about using a 2-dimensional matrix for that? That should address the duplication and keep things concise.

[eregon]

CI tests have not been passed for some time now.

Only on macOS which is a PITA. They work fine on Linux AFAIK.

[torrocus]

@pkuczynski said:
It make sense to run things in parallel if it speeds up the tests, that's for sure. Just the code duplication is aweful. Can we somehow avoid this?

I don't like these duplications either. Unfortunately, every job must perform similar activities:

• git checkout repo
• install ruby via rvm
• install tf gem
• run selected tests

At the moment I have no idea how to do it better, although I would love to share these parts of the job.

@eregon said:
What about using a 2-dimensional matrix for that? That should address the duplication and keep things concise.

If you mean strategies (matrix for OS), it won't change anything. To introduce parallelism, we need separate jobs. Anyway, where we run jobs on different OSs, I used matrix. I only removed where it was conditional.

[eregon]

Some general feedback: I think 9min vs 14min doesn't make a huge difference, in either case no one will wait for that actively. And then of course it will redo some work multiple times which is rather wasteful (e.g. compile ruby 2.7.7).
But having isolation between these groups of tests is valuable, especially given the existing issues between them, so this seems worth it.

[eregon]

If you mean strategies (matrix for OS), it won't change anything. To introduce parallelism, we need separate jobs. Anyway, where we run jobs on different OSs, I used matrix. I only removed where it was conditional.

Use a matrix e.g. like here: https://github.com/ruby/spec/blob/1afa2eab1cab5df6c5706fc5b7e92756b1db4f07/.github/workflows/ci.yml#L10-L12 (the keys would be os and test instead)
Those jobs all run in parallel.
And that solves the duplication, at the cost of some existing if conditionals for tests which don't work on macOS yet (but should eventually).

You can even avoid those if: with an explicit include list like https://github.com/ruby/spec/blob/1afa2eab1cab5df6c5706fc5b7e92756b1db4f07/.github/workflows/ci.yml#L14-L17

[torrocus]

Some general feedback: I think 9min vs 14min doesn't make a huge difference, in either case no one will wait for that actively. And then of course it will redo some work multiple times which is rather wasteful (e.g. compile ruby 2.7.7). But having isolation between these groups of tests is valuable, especially given the existing issues between them, so this seems worth it.

Yes, separation is a big advantage. If we add caching in the future, we should gain more time.

[torrocus]

If you mean strategies (matrix for OS), it won't change anything. To introduce parallelism, we need separate jobs. Anyway, where we run jobs on different OSs, I used matrix. I only removed where it was conditional.

Use a matrix e.g. like here: https://github.com/ruby/spec/blob/1afa2eab1cab5df6c5706fc5b7e92756b1db4f07/.github/workflows/ci.yml#L10-L12 (the keys would be os and test instead) Those jobs all run in parallel. And that solves the duplication, at the cost of some existing if conditionals for tests which don't work on macOS yet (but should eventually).

You can even avoid those if: with an explicit include list like https://github.com/ruby/spec/blob/1afa2eab1cab5df6c5706fc5b7e92756b1db4f07/.github/workflows/ci.yml#L14-L17

Interesting idea. I'll see what I can do with it.

[torrocus]

@pkuczynski @eregon I made changes and it looks better. What do you think?

[eregon]

Looks great now, 2 more comments

[eregon]

@pkuczynski Could you merge? (I cannot)
And also adapt the Required status checks to the new job names?

[torrocus]

@pkuczynski Since we have changed the CI process, we also need to change the settings in -> Settings -> Branches -> Branch protection rules. Then select Edit for the "master" branch. And then in the "Require status checks to pass before merging" option, remove the status checks "tests" and add "CI/tests".
Otherwise, only people with force push permission can merge ("tests" check status is waiting for report over and over again).

[pkuczynski]

Sure I can :) Give me few mins.

[pkuczynski]

MacOS tests are taking ridicilous long. Maybe we could check why...

[pkuczynski]

Thank you @torrocus for your contribution

[eregon]

MacOS tests are taking ridicilous long. Maybe we could check why...

Looking at https://github.com/rvm/rvm/actions/runs/8945080363/usage and specific logs for test/fast/*, it looks like on Linux it's mostly (only?) prebuilt Ruby binaries being used, while on macOS every ruby installed seems compiled from source, which takes a while.

One thing to improve might be to always use the same Ruby version for all tests and avoid uninstalling it.
And obviously if there could be prebuilt Rubies on macOS that'd be even better.

[pkuczynski]

That make sense. We had issues with building movable ruby in the past for macOS :(