-
-
Notifications
You must be signed in to change notification settings - Fork 202
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
Better parallelize CI jobs #1507
Conversation
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.
💯 🥳 🥇
This is amazing!!!! THANK YOU!! You cleaned up so many problems and it's so much better now (and TIL eval_gemfile
).
I'm a little surprised that there aren't more gem tweaks that are necessary because I felt like I was frequently struggling with Appraisal, so this is great!
.github/workflows/test.yml
Outdated
@@ -186,13 +174,13 @@ jobs: | |||
uses: actions/upload-artifact@v4 | |||
if: failure() | |||
with: | |||
name: screenshots | |||
name: screenshots-${{ matrix.pg }}-${{ matrix.gemfile }}-${{ matrix.ruby }} |
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.
you fixed a longstanding annoyance! 🙌🏻
.github/workflows/test.yml
Outdated
@@ -86,22 +86,39 @@ jobs: | |||
path: demo/log | |||
|
|||
test: | |||
name: Test | |||
name: Test - Ruby ${{ matrix.ruby }} - Postgres ${{ matrix.pg }} - ${{ matrix.gemfile }} |
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.
I think Rails is likely to be the culprit in most matrix-related failures:
name: Test - Ruby ${{ matrix.ruby }} - Postgres ${{ matrix.pg }} - ${{ matrix.gemfile }} | |
name: Test - ${{ matrix.gemfile }} - Ruby ${{ matrix.ruby }} - PG ${{ matrix.pg }} |
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.
Yes, that makes sense. I'll fix that up
btw, that failure with Rails 6.1 is sorta known: #1280 It's fixed by pulling from the Rails 6.1-stable git branch, though I'm a little surprised it shows up now rather than before. |
6.1 previously only ran with Ruby 3.0 which explains why it didn't pop up before. I could add excludes to the matrix to preserve that but just pulling 6-1-stable seems fine for CI. WDYT? |
* matrix: teamcapybara/capybara#2468 * `nokogiri 🤷 but there's no version constraint anyways * `net-*`: mikel/mail#1472 It's been quite a while so these seem safe to drop
Ditch appraisals while we're at it
348594a
to
3b336fe
Compare
For (I'm hugely surprised that this PR turned out well with just one try. Usually my interactions with github actions require much more force-pushes 🙃) |
🫶 thank you! |
Drop appraisal while we're at it. Makes it a bit easier