From c52c583ed2d1197db0ed402c9e232242be93be0e Mon Sep 17 00:00:00 2001 From: Simon Coffey Date: Mon, 19 Jun 2023 14:27:56 +0100 Subject: [PATCH 01/12] Fix broken validation messages A typo in https://github.com/hallelujah/valid_email/pull/122 appears to have introduced an error here that's causing a large number of test failures. Closes #124, closes #123 --- lib/valid_email/email_validator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/valid_email/email_validator.rb b/lib/valid_email/email_validator.rb index 72302ca..0fe7998 100644 --- a/lib/valid_email/email_validator.rb +++ b/lib/valid_email/email_validator.rb @@ -22,7 +22,7 @@ def validate_each(record,attribute,value) end unless r msg = (options[:message] || I18n.t(:invalid, :scope => "valid_email.validations.email")) - record.errors.add attribute, message: I18n.interpolate(msgs, value: value) + record.errors.add attribute, message: I18n.interpolate(msg, value: value) end end end From f5d0a8f7d0bad73fdb464b29daf739cc15e0e572 Mon Sep 17 00:00:00 2001 From: Simon Coffey Date: Mon, 19 Jun 2023 14:27:59 +0100 Subject: [PATCH 02/12] Fix broken MX-with-fallback test 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. --- spec/email_validator_spec.rb | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/spec/email_validator_spec.rb b/spec/email_validator_spec.rb index 2fbdf0e..bbeddd2 100644 --- a/spec/email_validator_spec.rb +++ b/spec/email_validator_spec.rb @@ -135,22 +135,32 @@ def self.model_name end describe "validating email with MX and fallback to A" do + let(:dns) { instance_double('Resolv::DNS', :dns) } + subject { person_class_mx_with_fallback.new } + before do + allow(Resolv::DNS).to receive(:open).and_yield(dns) + end + it "passes when email domain has MX record" do - subject.email = 'john@gmail.com' + allow(dns).to receive(:getresources).with('has-mx-record.org', Resolv::DNS::Resource::IN::MX).and_return(['1.2.3.4']) + subject.email = 'john@has-mx-record.org' expect(subject.valid?).to be_truthy expect(subject.errors[:email]).to be_empty end it "passes when email domain has no MX record but has an A record" do - subject.email = 'john@subdomain.rubyonrails.org' + allow(dns).to receive(:getresources).with('has-a-record.org', Resolv::DNS::Resource::IN::MX).and_return([]) + allow(dns).to receive(:getresources).with('has-a-record.org', Resolv::DNS::Resource::IN::A).and_return(['1.2.3.4']) + subject.email = 'john@has-a-record.org' expect(subject.valid?).to be_truthy expect(subject.errors[:email]).to be_empty end it "fails when domain does not exists" do - subject.email = 'john@nonexistentdomain.abc' + allow(dns).to receive(:getresources).with('does-not-exist.org', anything).and_return([]) + subject.email = 'john@does-not-exist.org' expect(subject.valid?).to be_falsey expect(subject.errors[:email]).to eq errors end From 346715467849e938d0f26f56338a0e8f3598d6b1 Mon Sep 17 00:00:00 2001 From: Simon Coffey Date: Mon, 19 Jun 2023 14:28:02 +0100 Subject: [PATCH 03/12] Clarify build output 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. --- .rspec | 1 - spec/spec_helper.rb | 1 - 2 files changed, 2 deletions(-) diff --git a/.rspec b/.rspec index df95f23..e69de29 100644 --- a/.rspec +++ b/.rspec @@ -1 +0,0 @@ ---warning diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 9467a48..40a7319 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -3,7 +3,6 @@ RSpec.configure do |config| config.order = :random - config.profile_examples = 10 config.raise_errors_for_deprecations! Kernel.srand config.seed end From afea0792329eb23317a2fef4d4aca76a76bce533 Mon Sep 17 00:00:00 2001 From: Simon Coffey Date: Mon, 19 Jun 2023 14:28:05 +0100 Subject: [PATCH 04/12] Introduce basic GitHub Actions build 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. --- .github/workflows/ci.yaml | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 .github/workflows/ci.yaml diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml new file mode 100644 index 0000000..ada94c1 --- /dev/null +++ b/.github/workflows/ci.yaml @@ -0,0 +1,24 @@ +name: CI +on: + workflow_dispatch: + push: + +permissions: + checks: write + contents: read + pull-requests: write + +jobs: + rspec: + name: RSpec + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v3 + - name: Setup ruby + uses: ruby/setup-ruby@v1 + with: + ruby-version: '3.0' + bundler-cache: true + - name: Run tests + run: bundle exec rake From c4364a71b1c82e06bbc19ad7a1f8cdcda06d3c0b Mon Sep 17 00:00:00 2001 From: Simon Coffey Date: Mon, 19 Jun 2023 14:28:08 +0100 Subject: [PATCH 05/12] Unpin rake version Rake 10.x produces the following error with ruby 3.2: rake aborted! NoMethodError: undefined method `=~' for # 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. --- valid_email.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/valid_email.gemspec b/valid_email.gemspec index 9bc104a..263a299 100644 --- a/valid_email.gemspec +++ b/valid_email.gemspec @@ -19,7 +19,7 @@ Gem::Specification.new do |s| # specify any dependencies here; for example: s.add_development_dependency "rspec", "~> 3.10" - s.add_development_dependency "rake", '< 11.0' + s.add_development_dependency "rake" s.add_runtime_dependency "mail", ">= 2.6.1" s.add_runtime_dependency "simpleidn" if Gem::Version.new(RUBY_VERSION) <= Gem::Version.new('2.0') From 6ab7800946439cc5098de8bdd8ce75d9f7c6d844 Mon Sep 17 00:00:00 2001 From: Simon Coffey Date: Mon, 19 Jun 2023 14:28:10 +0100 Subject: [PATCH 06/12] Add all passing ruby versions to matrix CI is now passing for all versions of ruby back to 2.5, so we can add these to a build matrix. --- .github/workflows/ci.yaml | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index ada94c1..29bbe8c 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -10,15 +10,27 @@ permissions: jobs: rspec: - name: RSpec + name: RSpec (ruby ${{ matrix.ruby-version }}) runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + ruby-version: + - '2.5' + - '2.6' + - '2.7' + - '3.0' + - '3.1' + - '3.2' + - 'jruby-head' + - 'truffleruby-head' steps: - name: Checkout uses: actions/checkout@v3 - name: Setup ruby uses: ruby/setup-ruby@v1 with: - ruby-version: '3.0' + ruby-version: ${{ matrix.ruby-version }} bundler-cache: true - name: Run tests run: bundle exec rake From d99c9038b7254da1be26992f4f9bce0c53bba9b7 Mon Sep 17 00:00:00 2001 From: Simon Coffey Date: Mon, 19 Jun 2023 14:28:13 +0100 Subject: [PATCH 07/12] Show ruby warnings in CI 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. --- .github/workflows/ci.yaml | 2 ++ spec/spec_helper.rb | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 29bbe8c..367373a 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -24,6 +24,8 @@ jobs: - '3.2' - 'jruby-head' - 'truffleruby-head' + env: + CI: true steps: - name: Checkout uses: actions/checkout@v3 diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 40a7319..57d7fc3 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -5,4 +5,8 @@ config.order = :random config.raise_errors_for_deprecations! Kernel.srand config.seed + + if ENV['CI'] + config.warnings = true + end end From fce1f0f769fa41d854174cceb99ad563fae91a27 Mon Sep 17 00:00:00 2001 From: Simon Coffey Date: Mon, 19 Jun 2023 14:28:16 +0100 Subject: [PATCH 08/12] Fix error assertions for ActiveModel < 6.0 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. --- spec/email_validator_spec.rb | 46 ++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/spec/email_validator_spec.rb b/spec/email_validator_spec.rb index bbeddd2..e9d4628 100644 --- a/spec/email_validator_spec.rb +++ b/spec/email_validator_spec.rb @@ -1,5 +1,21 @@ require 'spec_helper' +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 + describe EmailValidator do email_class = Class.new do include ActiveModel::Validations @@ -69,43 +85,43 @@ def self.model_name it "fails when email empty" do expect(subject.valid?).to be_falsey - expect(subject.errors[:email]).to eq errors + expect(subject).to have_error_messages(:email, errors) end it "fails when email is not valid" do subject.email = 'joh@doe' expect(subject.valid?).to be_falsey - expect(subject.errors[:email]).to eq errors + expect(subject).to have_error_messages(:email, errors) end it "fails when email domain is prefixed with dot" do subject.email = 'john@.doe' expect(subject.valid?).to be_falsey - expect(subject.errors[:email]).to eq errors + expect(subject).to have_error_messages(:email, errors) end it "fails when email domain contains two consecutive dots" do subject.email = 'john@doe-two..com' expect(subject.valid?).to be_falsey - expect(subject.errors[:email]).to eq errors + expect(subject).to have_error_messages(:email, errors) end it "fails when email ends with a period" do subject.email = 'john@doe.com.' expect(subject.valid?).to be_falsey - expect(subject.errors[:email]).to eq errors + expect(subject).to have_error_messages(:email, errors) end it "fails when email ends with special characters" do subject.email = 'john@doe.com&' expect(subject.valid?).to be_falsey - expect(subject.errors[:email]).to eq errors + expect(subject).to have_error_messages(:email, errors) end it "fails when email is valid with information" do subject.email = '"John Doe" ' expect(subject.valid?).to be_falsey - expect(subject.errors[:email]).to eq errors + expect(subject).to have_error_messages(:email, errors) end it "passes when email is simple email address" do @@ -117,19 +133,19 @@ def self.model_name it "fails when email is simple email address not stripped" do subject.email = 'john@doe.com ' expect(subject.valid?).to be_falsey - expect(subject.errors[:email]).to eq errors + expect(subject).to have_error_messages(:email, errors) end it "fails when domain contains a space" do subject.email = 'john@doe .com' expect(subject.valid?).to be_falsey - expect(subject.errors[:email]).to eq errors + expect(subject).to have_error_messages(:email, errors) end it "fails when passing multiple simple email addresses" do subject.email = 'john@doe.com, maria@doe.com' expect(subject.valid?).to be_falsey - expect(subject.errors[:email]).to eq errors + expect(subject).to have_error_messages(:email, errors) end end @@ -162,7 +178,7 @@ def self.model_name allow(dns).to receive(:getresources).with('does-not-exist.org', anything).and_return([]) subject.email = 'john@does-not-exist.org' expect(subject.valid?).to be_falsey - expect(subject.errors[:email]).to eq errors + expect(subject).to have_error_messages(:email, errors) end end @@ -178,13 +194,13 @@ def self.model_name it "fails when email domain has no MX record" do subject.email = 'john@subdomain.rubyonrails.org' expect(subject.valid?).to be_falsey - expect(subject.errors[:email]).to eq errors + expect(subject).to have_error_messages(:email, errors) end it "fails when domain does not exists" do subject.email = 'john@nonexistentdomain.abc' expect(subject.valid?).to be_falsey - expect(subject.errors[:email]).to eq errors + expect(subject).to have_error_messages(:email, errors) end end @@ -228,7 +244,7 @@ def self.model_name it "fails when email from disposable email services" do subject.email = 'john@grr.la' expect(subject.valid?).to be_falsey - expect(subject.errors[:email]).to eq errors + expect(subject).to have_error_messages(:email, errors) end end @@ -238,7 +254,7 @@ def self.model_name it "does not pass with an invalid domain" do subject.email = "test@example.org$\'" expect(subject.valid?).to be_falsey - expect(subject.errors[:email]).to eq errors + expect(subject).to have_error_messages(:email, errors) end it "passes with valid domain" do From db311d9a275a0d5fa15b2ec44f9746ffdbd8518a Mon Sep 17 00:00:00 2001 From: Simon Coffey Date: Mon, 19 Jun 2023 14:28:19 +0100 Subject: [PATCH 09/12] Add all remaining ruby 2.x versions to build matrix 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: https://github.com/ruby/setup-ruby/issues/496 --- .github/workflows/ci.yaml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 367373a..1cbef43 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -11,11 +11,16 @@ permissions: jobs: rspec: name: RSpec (ruby ${{ matrix.ruby-version }}) - runs-on: ubuntu-latest + runs-on: ubuntu-20.04 strategy: fail-fast: false matrix: ruby-version: + - '2.0' + - '2.1' + - '2.2' + - '2.3' + - '2.4' - '2.5' - '2.6' - '2.7' From b47313a867f0aba133a40aa602851769770ddd9b Mon Sep 17 00:00:00 2001 From: Simon Coffey Date: Mon, 19 Jun 2023 14:28:21 +0100 Subject: [PATCH 10/12] Fix gemspec for ruby 1.9.3 and add to matrix 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. --- .github/workflows/ci.yaml | 1 + valid_email.gemspec | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 1cbef43..018249b 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -16,6 +16,7 @@ jobs: fail-fast: false matrix: ruby-version: + - '1.9.3' - '2.0' - '2.1' - '2.2' diff --git a/valid_email.gemspec b/valid_email.gemspec index 263a299..4e80325 100644 --- a/valid_email.gemspec +++ b/valid_email.gemspec @@ -22,10 +22,10 @@ Gem::Specification.new do |s| s.add_development_dependency "rake" s.add_runtime_dependency "mail", ">= 2.6.1" s.add_runtime_dependency "simpleidn" - if Gem::Version.new(RUBY_VERSION) <= Gem::Version.new('2.0') + if Gem::Version.new(RUBY_VERSION.dup) <= Gem::Version.new('2.0') s.add_runtime_dependency 'mime-types', '<= 2.6.2' end - if Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.2.2') + if Gem::Version.new(RUBY_VERSION.dup) < Gem::Version.new('2.2.2') s.add_runtime_dependency "activemodel", '< 5.0.0' else s.add_runtime_dependency "activemodel" From 97a25736cb1f4faa0da4e3afac686249a162fb4b Mon Sep 17 00:00:00 2001 From: Simon Coffey Date: Mon, 19 Jun 2023 14:28:24 +0100 Subject: [PATCH 11/12] Remove travis-ci config We've migrated to GitHub Actions so this can be removed. --- .travis.yml | 21 --------------------- 1 file changed, 21 deletions(-) delete mode 100644 .travis.yml diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index d727d38..0000000 --- a/.travis.yml +++ /dev/null @@ -1,21 +0,0 @@ -before_install: - - gem install bundler:1.17.3 -cache: bundler -dist: xenial -language: ruby -rvm: - - 1.9.3 - - 2.0 - - 2.1 - - 2.2 - - 2.2 - - 2.3 - - 2.4 - - 2.5 - - 2.6 - - 2.7 - - 3.0 - - 3.1 - - 3.2 - - jruby-head - - truffleruby-head From 1fab0f248468fb576390dc0aae12ef196d39574f Mon Sep 17 00:00:00 2001 From: Simon Coffey Date: Mon, 19 Jun 2023 14:28:27 +0100 Subject: [PATCH 12/12] Switch build status badge to GitHub Actions --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 1f450b9..ae6a072 100644 --- a/README.md +++ b/README.md @@ -122,7 +122,7 @@ nil.email? # => false ## Code Status -[![Build Status](https://travis-ci.org/hallelujah/valid_email.svg?branch=master)](https://travis-ci.org/hallelujah/valid_email) +[![Build Status](https://github.com/hallelujah/valid_email/actions/workflows/ci.yaml/badge.svg)](https://github.com/hallelujah/valid_email/actions/workflows/ci.yaml) ## Credits