From 52d24e66cec1fa6c0100adb64effd2073869cd8b Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sat, 18 May 2019 21:58:21 -0700 Subject: Bump shoulda-matchers to 4.0.1 This removes the warning when using Ruby 2.6: BigDecimal.new is deprecated; use BigDecimal() method instead. This also adds Rails 5 support, which eliminates the need for the monkey patch to handle https://github.com/thoughtbot/shoulda-matchers/issues/913. --- Gemfile | 2 +- Gemfile.lock | 6 +++--- spec/support/shoulda/matchers/rails_shim.rb | 27 --------------------------- 3 files changed, 4 insertions(+), 31 deletions(-) delete mode 100644 spec/support/shoulda/matchers/rails_shim.rb diff --git a/Gemfile b/Gemfile index b6804a94971..e2842ca27db 100644 --- a/Gemfile +++ b/Gemfile @@ -379,7 +379,7 @@ group :development, :test do end group :test do - gem 'shoulda-matchers', '~> 3.1.2', require: false + gem 'shoulda-matchers', '~> 4.0.1', require: false gem 'email_spec', '~> 2.2.0' gem 'json-schema', '~> 2.8.0' gem 'webmock', '~> 3.5.1' diff --git a/Gemfile.lock b/Gemfile.lock index 2dbff1674df..a63b7cc84aa 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -857,8 +857,8 @@ GEM sexp_processor (4.12.0) sham_rack (1.3.6) rack - shoulda-matchers (3.1.2) - activesupport (>= 4.0.0) + shoulda-matchers (4.0.1) + activesupport (>= 4.2.0) sidekiq (5.2.7) connection_pool (~> 2.2, >= 2.2.2) rack (>= 1.5.0) @@ -1199,7 +1199,7 @@ DEPENDENCIES sentry-raven (~> 2.7) settingslogic (~> 2.0.9) sham_rack (~> 1.3.6) - shoulda-matchers (~> 3.1.2) + shoulda-matchers (~> 4.0.1) sidekiq (~> 5.2.7) sidekiq-cron (~> 1.0) simple_po_parser (~> 1.1.2) diff --git a/spec/support/shoulda/matchers/rails_shim.rb b/spec/support/shoulda/matchers/rails_shim.rb deleted file mode 100644 index 8d70598beb5..00000000000 --- a/spec/support/shoulda/matchers/rails_shim.rb +++ /dev/null @@ -1,27 +0,0 @@ -# monkey patch which fixes serialization matcher in Rails 5 -# https://github.com/thoughtbot/shoulda-matchers/issues/913 -# This can be removed when a new version of shoulda-matchers -# is released -module Shoulda - module Matchers - class RailsShim - def self.serialized_attributes_for(model) - if defined?(::ActiveRecord::Type::Serialized) - # Rails 5+ - serialized_columns = model.columns.select do |column| - model.type_for_attribute(column.name).is_a?( - ::ActiveRecord::Type::Serialized - ) - end - - serialized_columns.inject({}) do |hash, column| # rubocop:disable Style/EachWithObject - hash[column.name.to_s] = model.type_for_attribute(column.name).coder - hash - end - else - model.serialized_attributes - end - end - end - end -end -- cgit v1.2.1 From d707e2a49f2efe4670b5ebe62fb61554640ca7e9 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sun, 19 May 2019 04:01:16 -0700 Subject: Ensure subject passes validations Rails 5 requires that belongs_to associations have the associated record present. These tests were failing because they had nil values. --- spec/models/ci/pipeline_schedule_spec.rb | 2 ++ spec/models/clusters/cluster_spec.rb | 2 ++ 2 files changed, 4 insertions(+) diff --git a/spec/models/ci/pipeline_schedule_spec.rb b/spec/models/ci/pipeline_schedule_spec.rb index 1bfc14d2839..42d4769a921 100644 --- a/spec/models/ci/pipeline_schedule_spec.rb +++ b/spec/models/ci/pipeline_schedule_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' describe Ci::PipelineSchedule do + subject { build(:ci_pipeline_schedule) } + it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:owner) } diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb index 58203da5b22..f9d8ffd06e0 100644 --- a/spec/models/clusters/cluster_spec.rb +++ b/spec/models/clusters/cluster_spec.rb @@ -5,6 +5,8 @@ require 'spec_helper' describe Clusters::Cluster do it_behaves_like 'having unique enum values' + subject { build(:cluster) } + it { is_expected.to belong_to(:user) } it { is_expected.to have_many(:cluster_projects) } it { is_expected.to have_many(:projects) } -- cgit v1.2.1 From 37a335e60edd4f4c8b61fd2f9ffe0f1d27e3bfb8 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Tue, 21 May 2019 14:07:37 -0700 Subject: Fix remaining failures in shoulda-matcher Starting with Rails 5, belongs_to now adds a presence validation to the association, and so as of shoulda-matchers 4.0.0 the belong_to matcher follows suit and tests that this validation is there by setting the association to nil and asserting that there are validation errors. This exposed an error with the `validate_branches` method: we need to check the source and target project exist. --- app/models/merge_request.rb | 2 ++ spec/models/deployment_spec.rb | 4 ++-- spec/models/environment_spec.rb | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index df162e4844c..311ba1ce6bd 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -581,6 +581,8 @@ class MergeRequest < ApplicationRecord end def validate_branches + return unless target_project && source_project + if target_project == source_project && target_branch == source_branch errors.add :branch_conflict, "You can't use same project/branch for source and target" return diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index f51322e1404..1dceef3fc00 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -5,8 +5,8 @@ require 'spec_helper' describe Deployment do subject { build(:deployment) } - it { is_expected.to belong_to(:project) } - it { is_expected.to belong_to(:environment) } + it { is_expected.to belong_to(:project).required } + it { is_expected.to belong_to(:environment).required } it { is_expected.to belong_to(:user) } it { is_expected.to belong_to(:deployable) } diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 17246f238e0..7233d2454c6 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -6,7 +6,7 @@ describe Environment do let(:project) { create(:project, :stubbed_repository) } subject(:environment) { create(:environment, project: project) } - it { is_expected.to belong_to(:project) } + it { is_expected.to belong_to(:project).required } it { is_expected.to have_many(:deployments) } it { is_expected.to delegate_method(:stop_action).to(:last_deployment) } -- cgit v1.2.1