From 07215f7f2adc3ab3196ee4b353c9da66de1acc0b Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 18 May 2015 16:39:15 -0400 Subject: [PATCH 01/10] Bump shoulda-matchers and webmock gem versions --- Gemfile | 6 +++--- Gemfile.lock | 18 +++++++++--------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/Gemfile b/Gemfile index 0c1fff0bc14..c73fab4ed7c 100644 --- a/Gemfile +++ b/Gemfile @@ -254,10 +254,10 @@ group :development, :test do end group :test do - gem "simplecov", require: false - gem "shoulda-matchers", "~> 2.7.0" + gem 'simplecov', require: false + gem 'shoulda-matchers', '~> 2.8.0' gem 'email_spec' - gem "webmock" + gem 'webmock', '~> 1.21.0' gem 'test_after_commit' end diff --git a/Gemfile.lock b/Gemfile.lock index 9d87de7d4e0..6ba4cd96912 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -35,7 +35,7 @@ GEM tzinfo (~> 1.1) acts-as-taggable-on (3.5.0) activerecord (>= 3.2, < 5) - addressable (2.3.5) + addressable (2.3.8) annotate (2.6.0) activerecord (>= 2.3.0) rake (>= 0.8.7) @@ -118,8 +118,8 @@ GEM simplecov (>= 0.7) term-ansicolor thor - crack (0.4.1) - safe_yaml (~> 0.9.0) + crack (0.4.2) + safe_yaml (~> 1.0.0) creole (0.3.8) d3_rails (3.5.5) railties (>= 3.1.0) @@ -513,7 +513,7 @@ GEM rubypants (0.2.0) rugged (0.22.2) rugments (1.0.0.beta7) - safe_yaml (0.9.7) + safe_yaml (1.0.4) sanitize (2.1.0) nokogiri (>= 1.4.4) sass (3.2.19) @@ -535,7 +535,7 @@ GEM thor (~> 0.14) settingslogic (2.0.9) sexp_processor (4.4.5) - shoulda-matchers (2.7.0) + shoulda-matchers (2.8.0) activesupport (>= 3.0.0) sidekiq (3.3.0) celluloid (>= 0.16.0) @@ -652,8 +652,8 @@ GEM equalizer (~> 0.0.7) warden (1.2.3) rack (>= 1.0) - webmock (1.16.0) - addressable (>= 2.2.7) + webmock (1.21.0) + addressable (>= 2.3.6) crack (>= 0.3.2) websocket-driver (0.3.3) wikicloth (0.8.1) @@ -772,7 +772,7 @@ DEPENDENCIES seed-fu select2-rails settingslogic - shoulda-matchers (~> 2.7.0) + shoulda-matchers (~> 2.8.0) sidekiq (~> 3.3) sidetiq (= 0.6.3) simplecov @@ -801,5 +801,5 @@ DEPENDENCIES unicorn-worker-killer version_sorter virtus - webmock + webmock (~> 1.21.0) wikicloth (= 0.8.1) From 47251b85e06c84e9303ddbd06098ecc06c05d269 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 18 May 2015 16:40:10 -0400 Subject: [PATCH 02/10] ensure_length_of -> validate_length_of --- spec/models/concerns/issuable_spec.rb | 2 +- spec/models/key_spec.rb | 4 ++-- spec/models/project_spec.rb | 8 ++++---- spec/models/snippet_spec.rb | 4 ++-- spec/models/user_spec.rb | 2 +- spec/support/matchers.rb | 2 +- 6 files changed, 11 insertions(+), 11 deletions(-) diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 557c71b4d2c..86c395a8e8e 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -16,7 +16,7 @@ describe Issue, "Issuable" do it { is_expected.to validate_presence_of(:iid) } it { is_expected.to validate_presence_of(:author) } it { is_expected.to validate_presence_of(:title) } - it { is_expected.to ensure_length_of(:title).is_at_least(0).is_at_most(255) } + it { is_expected.to validate_length_of(:title).is_at_least(0).is_at_most(255) } end describe "Scope" do diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index 6eb1208a7f2..fbb9e162952 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -26,8 +26,8 @@ describe Key do describe "Validation" do it { is_expected.to validate_presence_of(:title) } it { is_expected.to validate_presence_of(:key) } - it { is_expected.to ensure_length_of(:title).is_within(0..255) } - it { is_expected.to ensure_length_of(:key).is_within(0..5000) } + it { is_expected.to validate_length_of(:title).is_within(0..255) } + it { is_expected.to validate_length_of(:key).is_within(0..5000) } end describe "Methods" do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 48568e2a3ff..87c67fa32c3 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -69,14 +69,14 @@ describe Project do it { is_expected.to validate_presence_of(:name) } it { is_expected.to validate_uniqueness_of(:name).scoped_to(:namespace_id) } - it { is_expected.to ensure_length_of(:name).is_within(0..255) } + it { is_expected.to validate_length_of(:name).is_within(0..255) } it { is_expected.to validate_presence_of(:path) } it { is_expected.to validate_uniqueness_of(:path).scoped_to(:namespace_id) } - it { is_expected.to ensure_length_of(:path).is_within(0..255) } - it { is_expected.to ensure_length_of(:description).is_within(0..2000) } + it { is_expected.to validate_length_of(:path).is_within(0..255) } + it { is_expected.to validate_length_of(:description).is_within(0..2000) } it { is_expected.to validate_presence_of(:creator) } - it { is_expected.to ensure_length_of(:issues_tracker_id).is_within(0..255) } + it { is_expected.to validate_length_of(:issues_tracker_id).is_within(0..255) } it { is_expected.to validate_presence_of(:namespace) } it 'should not allow new projects beyond user limits' do diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index c81dd36ef4b..c786d0bf103 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -38,10 +38,10 @@ describe Snippet do it { is_expected.to validate_presence_of(:author) } it { is_expected.to validate_presence_of(:title) } - it { is_expected.to ensure_length_of(:title).is_within(0..255) } + it { is_expected.to validate_length_of(:title).is_within(0..255) } it { is_expected.to validate_presence_of(:file_name) } - it { is_expected.to ensure_length_of(:file_name).is_within(0..255) } + it { is_expected.to validate_length_of(:file_name).is_within(0..255) } it { is_expected.to validate_presence_of(:content) } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index be0b70395d6..f1b8afa5854 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -96,7 +96,7 @@ describe User do it { is_expected.to allow_value(0).for(:projects_limit) } it { is_expected.not_to allow_value(-1).for(:projects_limit) } - it { is_expected.to ensure_length_of(:bio).is_within(0..255) } + it { is_expected.to validate_length_of(:bio).is_within(0..255) } describe 'email' do it 'accepts info@example.com' do diff --git a/spec/support/matchers.rb b/spec/support/matchers.rb index 52b11bd6323..f8cce2ea5a3 100644 --- a/spec/support/matchers.rb +++ b/spec/support/matchers.rb @@ -70,7 +70,7 @@ end # Extend shoulda-matchers module Shoulda::Matchers::ActiveModel - class EnsureLengthOfMatcher + class ValidateLengthOfMatcher # Shortcut for is_at_least and is_at_most def is_within(range) is_at_least(range.min) && is_at_most(range.max) From 1c4604bff67f612d8c74be499891b04f51a7821a Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 18 May 2015 16:40:32 -0400 Subject: [PATCH 03/10] "expect { }.not_to raise_error" no longer takes a specific class --- spec/models/deploy_keys_project_spec.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/spec/models/deploy_keys_project_spec.rb b/spec/models/deploy_keys_project_spec.rb index 7032b777144..705ef257d86 100644 --- a/spec/models/deploy_keys_project_spec.rb +++ b/spec/models/deploy_keys_project_spec.rb @@ -36,9 +36,7 @@ describe DeployKeysProject do it "doesn't destroy the deploy key" do subject.destroy - expect { - deploy_key.reload - }.not_to raise_error(ActiveRecord::RecordNotFound) + expect { deploy_key.reload }.not_to raise_error end end @@ -63,9 +61,7 @@ describe DeployKeysProject do it "doesn't destroy the deploy key" do subject.destroy - expect { - deploy_key.reload - }.not_to raise_error(ActiveRecord::RecordNotFound) + expect { deploy_key.reload }.not_to raise_error end end end From 2f3ab0ab858f3343807716d5ef033f7a90112d51 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Thu, 21 May 2015 17:34:12 -0400 Subject: [PATCH 04/10] Define GITORIOUS_HOST only once --- lib/gitlab/gitorious_import.rb | 5 +++++ lib/gitlab/gitorious_import/client.rb | 2 -- lib/gitlab/gitorious_import/repository.rb | 2 -- 3 files changed, 5 insertions(+), 4 deletions(-) create mode 100644 lib/gitlab/gitorious_import.rb diff --git a/lib/gitlab/gitorious_import.rb b/lib/gitlab/gitorious_import.rb new file mode 100644 index 00000000000..8d0132a744c --- /dev/null +++ b/lib/gitlab/gitorious_import.rb @@ -0,0 +1,5 @@ +module Gitlab + module GitoriousImport + GITORIOUS_HOST = "https://gitorious.org" + end +end diff --git a/lib/gitlab/gitorious_import/client.rb b/lib/gitlab/gitorious_import/client.rb index 1fa89dba448..99fe5bdebfc 100644 --- a/lib/gitlab/gitorious_import/client.rb +++ b/lib/gitlab/gitorious_import/client.rb @@ -1,7 +1,5 @@ module Gitlab module GitoriousImport - GITORIOUS_HOST = "https://gitorious.org" - class Client attr_reader :repo_list diff --git a/lib/gitlab/gitorious_import/repository.rb b/lib/gitlab/gitorious_import/repository.rb index f702797dc6e..c88f1ae358d 100644 --- a/lib/gitlab/gitorious_import/repository.rb +++ b/lib/gitlab/gitorious_import/repository.rb @@ -1,7 +1,5 @@ module Gitlab module GitoriousImport - GITORIOUS_HOST = "https://gitorious.org" - Repository = Struct.new(:full_name) do def id Digest::SHA1.hexdigest(full_name) From 57830201a9152b56ccf65a98275601617f44653d Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Thu, 21 May 2015 18:24:46 -0400 Subject: [PATCH 05/10] Add spec/support files for WebMock and test coverage --- spec/spec_helper.rb | 12 ------------ spec/support/coverage.rb | 8 ++++++++ spec/support/webmock.rb | 4 ++++ 3 files changed, 12 insertions(+), 12 deletions(-) create mode 100644 spec/support/coverage.rb create mode 100644 spec/support/webmock.rb diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 8fe51cf4add..8cd00bdb2f9 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,16 +1,6 @@ -if ENV['SIMPLECOV'] - require 'simplecov' -end - -if ENV['COVERALLS'] - require 'coveralls' - Coveralls.wear_merged! -end - ENV["RAILS_ENV"] ||= 'test' require File.expand_path("../../config/environment", __FILE__) require 'rspec/rails' -require 'webmock/rspec' require 'email_spec' require 'sidekiq/testing/inline' @@ -18,8 +8,6 @@ require 'sidekiq/testing/inline' # in spec/support/ and its subdirectories. Dir[Rails.root.join("spec/support/**/*.rb")].each { |f| require f } -WebMock.disable_net_connect!(allow_localhost: true) - RSpec.configure do |config| config.use_transactional_fixtures = false config.use_instantiated_fixtures = false diff --git a/spec/support/coverage.rb b/spec/support/coverage.rb new file mode 100644 index 00000000000..a54bf03380c --- /dev/null +++ b/spec/support/coverage.rb @@ -0,0 +1,8 @@ +if ENV['SIMPLECOV'] + require 'simplecov' +end + +if ENV['COVERALLS'] + require 'coveralls' + Coveralls.wear_merged! +end diff --git a/spec/support/webmock.rb b/spec/support/webmock.rb new file mode 100644 index 00000000000..af2906b7568 --- /dev/null +++ b/spec/support/webmock.rb @@ -0,0 +1,4 @@ +require 'webmock' +require 'webmock/rspec' + +WebMock.disable_net_connect!(allow_localhost: true) From 33df3ed384563b1e302e5a4f2186b156c35bafc5 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Sat, 23 May 2015 18:38:44 -0400 Subject: [PATCH 06/10] Add `respond_to_missing?` for Commit and Repository As of Ruby 1.9, this is the correct way to handle `respond_to?` for methods implemented by `method_missing`. See https://robots.thoughtbot.com/always-define-respond-to-missing-when-overriding --- app/models/commit.rb | 6 ++---- app/models/repository.rb | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/app/models/commit.rb b/app/models/commit.rb index f02fe240540..9d721661629 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -172,10 +172,8 @@ class Commit @raw.send(m, *args, &block) end - def respond_to?(method) - return true if @raw.respond_to?(method) - - super + def respond_to_missing?(method, include_private = false) + @raw.respond_to?(method, include_private) || super end # Truncate sha to 8 characters diff --git a/app/models/repository.rb b/app/models/repository.rb index 1ca97017637..2c6347222aa 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -163,10 +163,8 @@ class Repository end end - def respond_to?(method) - return true if raw_repository.respond_to?(method) - - super + def respond_to_missing?(method, include_private = false) + raw_repository.respond_to?(method, include_private) || super end def blob_at(sha, path) From 1f9a2ab7c9faf9b3a5fcc2a8ea695efee2314196 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Sun, 31 May 2015 23:03:19 -0400 Subject: [PATCH 07/10] Memoize result of JSON.parse in json_response This might see a minor speedup in test cases that call this method many times. --- spec/support/api_helpers.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/support/api_helpers.rb b/spec/support/api_helpers.rb index ec9a326a1ea..f63322776d4 100644 --- a/spec/support/api_helpers.rb +++ b/spec/support/api_helpers.rb @@ -29,6 +29,6 @@ module ApiHelpers end def json_response - JSON.parse(response.body) + @_json_response ||= JSON.parse(response.body) end end From 700d4387ce3d57330ed76e6b8545f27996d99c5f Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Sun, 31 May 2015 23:38:07 -0400 Subject: [PATCH 08/10] Use stub_const to stub constants instead of redefining them They're called constants for a reason, after all. --- features/steps/project/commits/commits.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/features/steps/project/commits/commits.rb b/features/steps/project/commits/commits.rb index c888e82e207..30d53333b78 100644 --- a/features/steps/project/commits/commits.rb +++ b/features/steps/project/commits/commits.rb @@ -69,14 +69,13 @@ class Spinach::Features::ProjectCommits < Spinach::FeatureSteps end step 'I visit big commit page' do - Commit::DIFF_SAFE_FILES = 20 + stub_const('Commit::DIFF_SAFE_FILES', 20) visit namespace_project_commit_path(@project.namespace, @project, sample_big_commit.id) end step 'I see big commit warning' do page.should have_content sample_big_commit.message page.should have_content "Too many changes" - Commit::DIFF_SAFE_FILES = 100 end step 'I visit a commit with an image that changed' do From 11c611bc620519badeb4ce4a6bbb5d53ac96c207 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 10 Jun 2015 01:38:36 -0400 Subject: [PATCH 09/10] Fix shoulda-matchers require --- Gemfile | 2 +- spec/spec_helper.rb | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index c73fab4ed7c..642df54c173 100644 --- a/Gemfile +++ b/Gemfile @@ -255,7 +255,7 @@ end group :test do gem 'simplecov', require: false - gem 'shoulda-matchers', '~> 2.8.0' + gem 'shoulda-matchers', '~> 2.8.0', require: false gem 'email_spec' gem 'webmock', '~> 1.21.0' gem 'test_after_commit' diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 8cd00bdb2f9..9c8004ab555 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,6 +1,7 @@ ENV["RAILS_ENV"] ||= 'test' require File.expand_path("../../config/environment", __FILE__) require 'rspec/rails' +require 'shoulda/matchers' require 'email_spec' require 'sidekiq/testing/inline' From 94cbf0091f44e879c02066aab7ec0a44eaeb14c3 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 10 Jun 2015 01:38:45 -0400 Subject: [PATCH 10/10] Fix taskable require --- app/models/concerns/taskable.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/models/concerns/taskable.rb b/app/models/concerns/taskable.rb index 33b4814d7ec..660e58b876d 100644 --- a/app/models/concerns/taskable.rb +++ b/app/models/concerns/taskable.rb @@ -1,4 +1,5 @@ require 'task_list' +require 'task_list/filter' # Contains functionality for objects that can have task lists in their # descriptions. Task list items can be added with Markdown like "* [x] Fix