From db9d318f76f398b75b82ebb427045da288c37277 Mon Sep 17 00:00:00 2001 From: Jarrett Lusso Date: Mon, 18 Mar 2024 12:37:01 -0400 Subject: [PATCH 1/2] Setup GitHub actions - Add RuboCop - Refactor tests - Fix failing lints - Remove travis - Add dependabot config --- .github/dependabot.yml | 12 ++ .github/workflows/ci.yml | 43 +++++++ .rubocop.yml | 4 + .rubocop_todo.yml | 7 ++ .ruby-version | 2 +- .travis.yml | 7 -- README.md | 2 +- emailable.gemspec | 1 + lib/emailable/batch.rb | 2 +- lib/emailable/client.rb | 4 +- lib/emailable/resources/api_resource.rb | 4 +- test/email_validator_test.rb | 111 +++++++++--------- test/emailable/batch_test.rb | 2 - test/emailable/resources/api_resource_test.rb | 2 +- test/emailable_test.rb | 20 ++-- 15 files changed, 138 insertions(+), 85 deletions(-) create mode 100644 .github/dependabot.yml create mode 100644 .github/workflows/ci.yml create mode 100644 .rubocop.yml create mode 100644 .rubocop_todo.yml delete mode 100644 .travis.yml diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..f0527e6 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,12 @@ +version: 2 +updates: +- package-ecosystem: bundler + directory: "/" + schedule: + interval: daily + open-pull-requests-limit: 10 +- package-ecosystem: github-actions + directory: "/" + schedule: + interval: daily + open-pull-requests-limit: 10 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..7728c8d --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,43 @@ +name: CI + +on: + push: + branches: [ master ] + pull_request: + branches: [ master ] + +jobs: + tests: + name: Tests + runs-on: ubuntu-latest + strategy: + matrix: + ruby-version: ['2.7', '3.0', '3.1', '3.2', '3.3'] + + steps: + - uses: actions/checkout@v4 + - name: Set up Ruby ${{ matrix.ruby-version }} + uses: ruby/setup-ruby@v1 + with: + ruby-version: ${{ matrix.ruby-version }} + bundler-cache: true + + - name: Run tests + run: bundle exec rake + + linters: + name: Linters + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Set up Ruby + uses: ruby/setup-ruby@v1 + with: + bundler-cache: true + + - name: RuboCop + run: bundle exec rubocop --parallel + if: always() diff --git a/.rubocop.yml b/.rubocop.yml new file mode 100644 index 0000000..6e0df8a --- /dev/null +++ b/.rubocop.yml @@ -0,0 +1,4 @@ +inherit_from: .rubocop_todo.yml + +inherit_gem: + rubocop-cache-ventures: rubocop.yml diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml new file mode 100644 index 0000000..ffb0488 --- /dev/null +++ b/.rubocop_todo.yml @@ -0,0 +1,7 @@ +# This configuration was generated by +# `rubocop --auto-gen-config --exclude-limit 10000` +# on 2024-03-18 16:04:55 UTC using RuboCop version 1.62.1. +# The point is for the user to remove these configuration records +# one by one as the offenses are removed from the code base. +# Note that changes in the inspected code, or installation of new +# versions of RuboCop, may require this file to be generated again. diff --git a/.ruby-version b/.ruby-version index a4dd9db..15a2799 100644 --- a/.ruby-version +++ b/.ruby-version @@ -1 +1 @@ -2.7.4 +3.3.0 diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index 9145582..0000000 --- a/.travis.yml +++ /dev/null @@ -1,7 +0,0 @@ ---- -sudo: false -language: ruby -cache: bundler -rvm: - - 2.7.4 -before_install: gem install bundler -v 2.2.15 diff --git a/README.md b/README.md index ccd7805..9eac84e 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ # Emailable Ruby Library [![Gem Version](https://badge.fury.io/rb/emailable.svg)](https://rubygems.org/gems/emailable) -[![Build Status](https://app.travis-ci.com/emailable/emailable-ruby.svg)](https://app.travis-ci.com/emailable/emailable-ruby) +![Build Status](https://github.com/emailable/emailable-ruby/actions/workflows/ci.yml/badge.svg) [![Maintainability](https://api.codeclimate.com/v1/badges/e7eef54e491adec95e6d/maintainability)](https://codeclimate.com/github/emailable/emailable-ruby/maintainability) This is the official ruby wrapper for the Emailable API. diff --git a/emailable.gemspec b/emailable.gemspec index 165c25e..86ab291 100644 --- a/emailable.gemspec +++ b/emailable.gemspec @@ -34,4 +34,5 @@ Gem::Specification.new do |s| s.add_development_dependency 'minitest', '~> 5.0' s.add_development_dependency 'minitest-reporters' s.add_development_dependency 'activemodel' + s.add_development_dependency 'rubocop-cache-ventures' end diff --git a/lib/emailable/batch.rb b/lib/emailable/batch.rb index 15ebbb8..518c55c 100644 --- a/lib/emailable/batch.rb +++ b/lib/emailable/batch.rb @@ -27,7 +27,7 @@ def verify(parameters = {}) end def status(parameters = {}) - return nil unless @id + return unless @id return @status if @status parameters[:id] = @id diff --git a/lib/emailable/client.rb b/lib/emailable/client.rb index d8218c0..7e2cc54 100644 --- a/lib/emailable/client.rb +++ b/lib/emailable/client.rb @@ -35,7 +35,7 @@ def request(method, endpoint, params = {}) response = Response.new(http_response) rescue => e - retry if (tries -= 1) > 0 && self.class.should_retry?(e, tries) + retry if (tries -= 1) > 0 && should_retry?(e, tries) raise e end @@ -70,7 +70,7 @@ def create_connection(uri) connection end - def self.should_retry?(error, num_retries) + def should_retry?(error, num_retries) return false if num_retries >= Emailable.max_network_retries case error diff --git a/lib/emailable/resources/api_resource.rb b/lib/emailable/resources/api_resource.rb index 6906575..f1ff5ed 100644 --- a/lib/emailable/resources/api_resource.rb +++ b/lib/emailable/resources/api_resource.rb @@ -15,8 +15,8 @@ def to_h alias_method :to_hash, :to_h - def to_json - JSON.generate(to_h) + def to_json(*args) + JSON.generate(to_h, *args) end def inspect diff --git a/test/email_validator_test.rb b/test/email_validator_test.rb index ad1db36..213bfa9 100644 --- a/test/email_validator_test.rb +++ b/test/email_validator_test.rb @@ -3,96 +3,97 @@ class EmailValidatorTest < Minitest::Test - def user_class( - smtp: true, states: %i[deliverable risky unknown], free: true, role: true, - accept_all: true, disposable: true, timeout: 3, **options - ) - Class.new do - include ActiveModel::Model - attr_accessor :email, :email_verification_result - - validates :email, presence: true, email: { - smtp: smtp, states: states, - free: free, role: role, disposable: disposable, accept_all: accept_all, - timeout: timeout - }.merge(options) - - def self.name - 'TestClass' - end - - # stub changes to always be true - def changes - { email: true } - end - end - end - def setup Emailable.api_key = 'test_7aff7fc0142c65f86a00' - sleep(0.25) end def test_valid - @user = user_class.new(email: 'deliverable@example.com') + valid_user = user_class.new(email: 'deliverable@example.com') - assert @user.valid? - assert @user.errors.empty? + assert valid_user.valid? + assert valid_user.errors.empty? end def test_invalid - @user = user_class.new(email: 'undeliverable@example.com') + invalid_user = user_class.new(email: 'undeliverable@example.com') - assert !@user.valid? - assert @user.errors[:email].present? + refute invalid_user.valid? + assert invalid_user.errors[:email].present? end def test_verification_result - @user = user_class.new(email: 'undeliverable@example.com') - @user.valid? + invalid_user = user_class.new(email: 'undeliverable@example.com') + invalid_user.valid? - refute_nil @user.email_verification_result - assert @user.email_verification_result.state, :undeliverable + refute_nil invalid_user.email_verification_result + assert_equal 'undeliverable', invalid_user.email_verification_result.state end def test_boolean_options %i[smtp free role disposable accept_all].each do |option| - invalid_user = user_class(option => 'string').new - valid_user = user_class.new + invalid_options = user_class(option => 'string').new + valid_options = user_class.new - assert !valid_user.valid? - assert_raises(ArgumentError) { invalid_user.valid? } + refute valid_options.valid? + assert_raises(ArgumentError) { invalid_options.valid? } end end def test_states_option - invalid_user = user_class(states: %i[invalid_state]).new - valid_user = user_class.new + invalid_options = user_class(states: %i[invalid_state]).new + valid_options = user_class.new - assert !valid_user.valid? - assert_raises(ArgumentError) { invalid_user.valid? } + refute valid_options.valid? + assert_raises(ArgumentError) { invalid_options.valid? } end def test_timeout_option - invalid_user1 = user_class(timeout: 'string').new - invalid_user2 = user_class(timeout: 1).new - valid_user = user_class.new + invalid_options1 = user_class(timeout: 'string').new + invalid_options2 = user_class(timeout: 1).new + valid_options = user_class.new - assert !valid_user.valid? - assert_raises(ArgumentError) { invalid_user1.valid? } - assert_raises(ArgumentError) { invalid_user2.valid? } + refute valid_options.valid? + assert_raises(ArgumentError) { invalid_options1.valid? } + assert_raises(ArgumentError) { invalid_options2.valid? } end def test_custom_option message = 'invalid message' - invalid_user = user_class(message: message, reportable: true).new( + valid_options = user_class(message: message, reportable: true).new( email: 'undeliverable@example.com' ) - refute invalid_user.valid? - assert invalid_user.errors[:email].present? - assert_equal message, invalid_user.errors[:email].first - assert invalid_user.errors.where(:email, reportable: true).present? + refute valid_options.valid? + assert valid_options.errors[:email].present? + assert_equal message, valid_options.errors[:email].first + assert valid_options.errors.where(:email, reportable: true).present? + end + + private + + def user_class( + smtp: true, states: %i[deliverable risky unknown], free: true, role: true, + accept_all: true, disposable: true, timeout: 3, **options + ) + Class.new do + include ActiveModel::Model + attr_accessor :email, :email_verification_result + + validates :email, presence: true, email: { + smtp: smtp, states: states, + free: free, role: role, disposable: disposable, accept_all: accept_all, + timeout: timeout + }.merge(options) + + def self.name + 'TestClass' + end + + # stub changes to always be true + def changes + { email: true } + end + end end end diff --git a/test/emailable/batch_test.rb b/test/emailable/batch_test.rb index d5ebbb8..b80f75e 100644 --- a/test/emailable/batch_test.rb +++ b/test/emailable/batch_test.rb @@ -4,12 +4,10 @@ module Emailable class BatchTest < Minitest::Test def setup - sleep(1) Emailable.api_key = 'test_7aff7fc0142c65f86a00' @emails = ['jarrett@emailable.com', 'support@emailable.com'] @batch = Emailable::Batch.new(@emails) @batch_id ||= @batch.verify - sleep(1) end def test_start_batch diff --git a/test/emailable/resources/api_resource_test.rb b/test/emailable/resources/api_resource_test.rb index 69d9054..b2b7c7f 100644 --- a/test/emailable/resources/api_resource_test.rb +++ b/test/emailable/resources/api_resource_test.rb @@ -14,7 +14,7 @@ def setup def test_init assert_equal 'abc', @e.foo assert_equal 123, @e.bar - assert_equal true, @e.baz + assert @e.baz end def test_to_h diff --git a/test/emailable_test.rb b/test/emailable_test.rb index 536e646..7057a2f 100644 --- a/test/emailable_test.rb +++ b/test/emailable_test.rb @@ -24,7 +24,7 @@ def test_verification_state def test_verification_tag result = Emailable.verify('jarrett+marketing@emailable.com') - assert result.tag == 'marketing' + assert_equal 'marketing', result.tag end def test_account @@ -49,33 +49,27 @@ def test_name_and_gender end def test_accept_all? - result = Emailable.verify('accept-all@example.com') - assert result.accept_all? + assert Emailable.verify('accept-all@example.com').accept_all? end def test_disposable? - result = Emailable.verify('disposable@example.com') - assert result.disposable? + assert Emailable.verify('disposable@example.com').disposable? end def test_free? - result = Emailable.verify('free@example.com') - assert result.free? + assert Emailable.verify('free@example.com').free? end def test_role? - result = Emailable.verify('role@example.com') - assert result.role? + assert Emailable.verify('role@example.com').role? end def test_mailbox_full? - result = Emailable.verify('mailbox-full@example.com') - assert result.mailbox_full? + assert Emailable.verify('mailbox-full@example.com').mailbox_full? end def test_no_reply? - result = Emailable.verify('no-reply@example.com') - assert result.no_reply? + assert Emailable.verify('no-reply@example.com').no_reply? end def test_slow_verification From bff8e455264df69e0260e649093d1daebfbd62fd Mon Sep 17 00:00:00 2001 From: Jarrett Lusso Date: Tue, 19 Mar 2024 10:39:46 -0400 Subject: [PATCH 2/2] Remove valid option testing that is confusing inside the invalid option testing --- test/email_validator_test.rb | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/test/email_validator_test.rb b/test/email_validator_test.rb index 213bfa9..96c9394 100644 --- a/test/email_validator_test.rb +++ b/test/email_validator_test.rb @@ -29,30 +29,24 @@ def test_verification_result assert_equal 'undeliverable', invalid_user.email_verification_result.state end - def test_boolean_options + def test_boolean_options_with_invalid_value %i[smtp free role disposable accept_all].each do |option| invalid_options = user_class(option => 'string').new - valid_options = user_class.new - refute valid_options.valid? assert_raises(ArgumentError) { invalid_options.valid? } end end - def test_states_option + def test_states_option_with_invalid_value invalid_options = user_class(states: %i[invalid_state]).new - valid_options = user_class.new - refute valid_options.valid? assert_raises(ArgumentError) { invalid_options.valid? } end - def test_timeout_option + def test_timeout_option_with_invalid_value invalid_options1 = user_class(timeout: 'string').new invalid_options2 = user_class(timeout: 1).new - valid_options = user_class.new - refute valid_options.valid? assert_raises(ArgumentError) { invalid_options1.valid? } assert_raises(ArgumentError) { invalid_options2.valid? } end