Add Ruby 3.1 to Circle tests #2129
Conversation
73b826e to
a27ec17
Compare
Ruby 3.1 ships with Psych 4. Psych 4 switched it's default YAML parsing mode to a safer mode, including not allowing aliases. This broke parsing of "secrets.yml" in Rails, if the secrets file contains any aliases. Some discussion: https://bugs.ruby-lang.org/issues/17866 Rails PR 42687 implemented a fix, and that fix was included in Rails 7 here: rails/rails@0ebb708 It was also included in the Rails 6-1-stable branch here: rails/rails@aac3edb But that commit happened after Rails 6.1.4 was published, and no patch-level releases have been made for Rails 6.1 since that commit. Only security-level releases have been published. I assume if Rails 6.1.5 is released that it will contain the fix. Until Rails 6.1.5 is released, the best option to be able to test Ruby 3.1 against Rails 6.1 apps is probably to just eliminate any aliases in yaml files that Rails uses.
Using new CircleCI image "cimg/ruby"
a27ec17 to
3f11808
Compare
|
@nickcharlton @pablobm The other 2 PRs were merged (thank you!) so I rebased this, and the tests are green so I'm marking this "ready for review". There are a few thoughts/questions in the PR description that might merit feedback/changes. |
| - run: bundle exec appraisal install | ||
| - run: bundle exec appraisal rails61 rspec | ||
| docker: | ||
| - image: cimg/ruby:3.1-browsers |
There was a problem hiding this comment.
Do we need the browsers image here, or is the standard ruby:3.1 with the orb enough?
There was a problem hiding this comment.
If I understand correctly, the browsers variant supplies the dependencies needed for the orb to work:
The browsers variant is designed to work in conjunction with the CircleCI Browser Tools orb. You can use the orb to install a version of Google Chrome and/or Firefox into your build.
https://circleci.com/developer/images/image/cimg/ruby#browsers
But I'll push up a quick test commit just to confirm.
There was a problem hiding this comment.
@nickcharlton I pushed a commit with cimg/ruby:3.1 and it seems to work just fine. And looking at the 4 test runs of the previous commit vs the 4 test runs of that commit and it doesn't seem to take significantly longer to install chrome or anything like that on cimg/ruby:3.1 afaict. Shall we just go with this? I'm also opening a ticket w/ CircleCI to ask them what the -browsers image is needed for. Do you read that blurb from them differently? I'm not sure why one would need it.
There was a problem hiding this comment.
Ah, actually I forgot for a bit that this PR only uses cimg/ruby in the 3.1 tests. And there is a difference of 7s (with -browsers) vs 23s (without `-browsers) for "Install Google Chrome". We could try testing each version a few more times to confirm the speed difference. I've also opened that ticket w/ Circle to confirm.
There was a problem hiding this comment.
@nickcharlton I went ahead and did some speed testing and it does seem like the difference is probably that some stuff is preinstalled on -browsers, which makes the browser installation faster. However, I'd bet it probably makes the image download longer also.
What I saw:
| test run | "-browsers"? | env spin up (incl download) | chrome install | total |
|---|---|---|---|---|
| link | yes | 30s | 9s | 39s |
| link | yes | 6s | 7s | 13s |
| link | yes | 13s | 7s | 20s |
| link | no | 5s | 18s | 23s |
| link | no | 12s | 17s | 29s |
| link | no | 3s | 19s | 22s |
Lmk what you think and which you prefer. Circle has enough variability in timing that it's hard to really get a firm idea of things 😕
There was a problem hiding this comment.
Oh I see. Right, it is intentional on Circle's part. I didn't think to even look at that, so thanks for highlighting it.
I think we should go with what they recommend — it's a pity your table shows how variable the runs are (thanks for doing that!).
There was a problem hiding this comment.
👍 sounds good. I've force-pushed to remove the commit that tried out the non--browsers variant.
6b9d82c to
3f11808
Compare
|
Thanks for this! I'm going to go ahead and merge it now! |
This PR builds on #2127 and #2128 to add test coverage for Ruby 3.1.
i.e.: Ignore the first 2 commits in this PR.(update: Those PRs were merged and I've rebased this)A couple things:
example_appto work around a Psych 4 incompatibility with Rails 6. (See the commit message on that commit for more details).cimg/rubydocker image, which replaces the oldcircleci/rubydocker image, because the Ruby 3.1 hasn't been added to the latter and it sounds like it won't be. We could probably update everything tocimg/rubyif we want. Or we could test it out a bit on Ruby 3.1 for a while first.Does that all seem reasonable?