Fix CI, drop the V2 label requirement, and test through Ruby 4.0.5 - #5
Conversation
Org policy requires all actions be pinned to a full-length SHA; the floating tags blocked every Ruby test job at job setup.
The label-checker job was the only job in pr-review.yml, and the V2 label does not exist in this fork, so every PR failed it.
fail-fast defaults to true, so one failing version cancelled the rest and hid whether they actually passed.
minitest dropped the MiniTest compatibility alias; spec_helper already uses Minitest, so this was the last reference to the old spelling.
Both pins held gems that require stdlib files no longer shipped by default, and spec/environment.rb swallows $stderr, so the resulting LoadErrors surfaced as a silent exit with no test output at all.
Ruby 3.0 removed the bare Proc.new implicit-block idiom, so XML#each raised ArgumentError on every Ruby from 3.0 on.
Ruby 3.x onward does not preserve instance variable order across a Marshal round-trip; the assertion only cares that the set matches.
Extends the matrix over the 3.x line up to 4.0.5, the version Evergreen runs. Versions are quoted so YAML does not read them as floats.
The buffer was discarded unconditionally, so a LoadError in spec_helper produced a silent exit with no test output and no cause.
minitest 6 removed the global must_* expectations and Minitest::Mock, and rejects assert_equal nil. The two shim gems restore the first two for every Ruby in the matrix; must_be_nil works on minitest 5 and 6.
Ruby 4.0's bundled minitest 6 wins activation over the gemspec pin, and it no longer ships Mock.
grantcox
changed the title
There was a problem hiding this comment.
This and the same change in lib/recurly_v2/xml/rexml.rb are both "broken in production" bugs (ArgumentError will be raised in Ruby 3 onwards), they're not just CI issues.
However, Claude has traced this and the only use is for redaction of CVVs in debug log messages, and so is only invoked if the RECURLY_INSECURE_DEBUG env var is true. I've confirmed we don't set this env var anywhere, so that's why we haven't had any issues ourselves.
There was a problem hiding this comment.
Note that the issue in rexml.rb was fixed upstream in recurly@f516a11#diff-b2b37bc25c4fe579fc6b01c5356a20e8d4f7fb730fd2f2cba26c942f38d72722 , although this nokogiri.rb issue is still present.
I will create another PR updating us to the latest upstream v2 branch. But that will come after this one - we want CI to actually be running (and green) to confirm that this version update does still pass completely, on all relevant Ruby versions.
Evergreen is on 4.0.5 today, so keep that exact version covered, and add a '4' entry that setup-ruby resolves to the newest 4.x it knows about.
| # '4' is a prefix match resolved against the version list baked into the | ||
| # setup-ruby commit pinned below, so it only tracks new 4.x releases as far | ||
| # as that pin. Bump the pin to pick up newer ones. | ||
| ruby: ['2.3', '2.4', '2.5', '2.6', '2.7', '3.0', '3.1', '3.2', '3.3', '3.4', '4.0.5', '4', 'jruby-9.3'] |
There was a problem hiding this comment.
4.0.5 is what we use in Evergreen currently, hence that version specifically. 4 is just to test any future versions (currently 4.0.6)
| runs-on: ubuntu-latest | ||
| name: Ruby ${{ matrix.ruby }} tests | ||
| strategy: | ||
| fail-fast: false |
There was a problem hiding this comment.
This doesn't really matter, it just means "if one test in the matrix fails, do we cancel all others". I think it's worth seeing what does succeed vs fail, hence the change. But when everything is green, it's no difference.
| steps: | ||
| - name: Check Labels | ||
| uses: docker://agilepathway/pull-request-label-checker:latest | ||
| with: | ||
| one_of: V2 |
There was a problem hiding this comment.
This was requiring every PR to have a V2 tag. This may make sense in the upstream repo, but it doesn't for us, and we don't want an irrelevant failing CI step.
2 participants
Nine commits, each self-contained.
CI plumbing
Pin actions to full-length SHAs. Org policy rejects floating tags, which failed all six Ruby jobs at job setup.
actions/checkout@v3→a37ce91,ruby/setup-ruby@v1→95ef2b0(v1.321.0). Same versions.fail-fast: false. One failing version was cancelling the rest, so siblings that had already passed reported as failures.Remove the V2 label requirement.
pr-review.ymlrequiredone_of: V2, but no such label exists in this fork, so every PR failed it unconditionally.label-checkerwas the file's only job, so the file is gone. The orgtier-2-reposruleset has no required-status-check rule, so nothing waits on a check that will never report.Making the suite run on modern Ruby
The matrix stopped at 2.7, and everything past it was broken. Each of these was a hard stop:
rake ~> 11.3pinned rake 11, which requiresostruct— not a default gem in Ruby 4.0. Relaxed to>= 11.3.webmock ~> 2.3pinned webmock 2.3.2 (2017), which requiresbase64— not a default gem from Ruby 3.4. Relaxed to>= 2.3.2.loggerleft the default gems in Ruby 4.0 andspec_helperrequires it. Added.&Proc.newin both XML backends — the implicit-block idiom Ruby 3.0 removed.XML#eachraisedArgumentErroron every Ruby from 3.0 on. This one is library code, not test-only.MiniTest::Mock— minitest dropped theMiniTestalias.spec_helperalready usedMinitest, so this was the last old-spelling reference.must_*expectations this suite is written in, movedMinitest::Mockto a separate gem, and rejectsassert_equal nil. Handled withminitest-global_expectations,minitest-mock(only on Ruby >= 4.0 — it needs Ruby >= 3.1, which the old half of the matrix cannot meet), andmust_equal nil→must_be_nil.Marshalround-trip on Ruby 3+; four marshal specs compared the arrays directly. Now compared as sets.spec/environment.rbwas swallowing$stderrunconditionally, so all of the above surfaced as a silent exit with no test output and no cause — the buffer is now released when the run fails. Worth knowing about before debugging anything else here.Matrix
2.3, 2.4, 2.5, 2.6, 2.7, 3.0, 3.1, 3.2, 3.3, 3.4, 4.0.5, jruby-9.3, quoted so YAML does not read them as floats. All 12 green.