Fix build and migrate to GitHub Actions #125
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
urbanautomaton
force-pushed
the
fix-ci
branch
2 times, most recently
from
9659fc8
to
25531c6
Compare
added 2 commits
June 19, 2023 14:27
A typo in hallelujah#122 appears to have introduced an error here that's causing a large number of test failures. Closes hallelujah#124, closes hallelujah#123
This test currently fails because subdomain.rubyonrails.org is not returning an A record. Whether it ever did or not, this is a brittle test because it's making a real DNS request. To fix this, and ensure the other tests in this group are also less brittle, we can stub the lookup responses appropriately for each example.
added 10 commits
June 19, 2023 14:28
We currently print warnings and profile examples by default on every rspec run. However the only warnings in the project are emitted by the mail gem, over which we have no control. There also really aren't any slow tests at all, so I don't think the profiling is particularly useful. This removes both options to make the primary output plainer in local development. I intend to add these back as CI-only options in a subsequent commit when I migrate the project to GitHub Actions.
Travis CI doesn't seem to be running any more, so this adds a simple workflow to get the rspec test suite running on new branches and PRs. I'll add the full ruby version matrix in a subsequent commit once this is passing.
Rake 10.x produces the following error with ruby 3.2:
rake aborted!
NoMethodError: undefined method `=~' for #<Proc:0x0000000105127e78
.gem/ruby/3.2.0/gems/rake-10.5.0/lib/rake/application.rb:393 (lambda)>
I'm not sure exactly why this is, but I can no longer replicate the
error that caused rake to be pinned to version 10.x in 5e46ecd, so I'm
unpinning it again to fix the build.
CI is now passing for all versions of ruby back to 2.5, so we can add these to a build matrix.
I previously removed ruby warnings from the default RSpec options, because they weren't actionable and were clouding the output. This reinstates them on CI only. The only warnings currently are from the mail gem which we don't control, but this will at least allow us to check that no new warnings are being introduced. I decided not to reinstate the example profiling that I also removed earlier, because the build is very quick, so I feel that profiling output isn't useful.
In ActiveModel 5.x, subject.errors[:email] sometimes returns an array of message strings, sometimes an array of hashes with a message key. ActiveModel 5.x is installed for ruby 2.4 and below, causing the build to fail for these rubies. This patch fixes up the various validation message assertions in the test suite by unwrapping any hashes in the validation errors before making the assertion. This workaround can be removed once support is dropped for ActiveModel 5.x.
These builds now all pass for the time being, so since they were in the original .travis.yml I'm adding them here. To get ruby 2.2 working with the official setup-ruby action I've had to pin the runner on ubuntu-20.04 due to a known issue with setup-ruby, 2.2 and ubuntu-latest: ruby/setup-ruby#496
In ruby 1.9.3 on ruby/setup-ruby, the bundle step fails with the error
There was an error while loading `valid_email.gemspec`: can't modify
frozen String. Bundler cannot continue.
This error emits from Gem::Version.new(RUBY_VERSION); adding a .dup to
avoid the frozen RUBY_VERSION value gets us a passing build on ruby 1.9.
We've migrated to GitHub Actions so this can be removed.
torrocus
reviewed
Jun 22, 2023
Comment on lines
+3
to
+17
| RSpec::Matchers.define :have_error_messages do |field, expected| | ||
| match do |model| | ||
| errors = model.errors[field] | ||
|
|
||
| messages = errors.map do |error| | ||
| case error | ||
| when String then error | ||
| when Hash then error[:message] | ||
| else fail ArgumentError, "Unknown model error type #{error.class}" | ||
| end | ||
| end | ||
|
|
||
| expect(messages).to eq expected | ||
| end | ||
| end |
There was a problem hiding this comment.
I like the idea of adding a matcher 👍
|
@hallelujah Can you review this, please? It looks like this fixes #120 and #107 and supersedes #123 and #124. |
hallelujah
approved these changes
Sep 8, 2023
This was referenced Oct 4, 2023
|
@hallelujah @torrocus Can you push a new release to RubyGems including these changes, please? |
|
@torrocus @hallelujah Sorry to bother you, but is there a specific date for when there is going to be the next release of the gem with these changes? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I noticed there are a number of failing tests in this project's build, and that it appears Travis CI have shut off this project at some point (😭), so there's no build feedback on PRs any more.
This PR fixes the build, and adds a GitHub Actions CI step covering all the previously-tested ruby versions. You can see the build success status for each commit in the compare view over in my forked repo.
Any and all feedback welcome – I did a little tidying of the build output (e.g. warnings are now only shown on CI, and rspec profiling is disabled), but if it's preferred that these are both kept by default I'm happy to add them back.