Out with the old, in with the new #261

mcmire · 2016-06-10T23:29:51Z

This commit is long overdue. The main change is to upgrade shoulda's
hard dependency on shoulda-matchers from 2.x to 3.x. Since this by
itself is a pretty consequential change, though, I also took the
opportunity to make some more at the same time that I think are
reasonable:

Remove support for Rails 3, upgrading dependency to 4, modifying
Appraisals file and fixing Cucumber specs to match.
Remove support for RSpec, along with related Cucumber specs. People
who use RSpec won't be using the shoulda gem -- they'll be using
shoulda-matchers instead. There's already a good collection of
integration specs there, so we don't need the tests here.
Remove support for Ruby 1.9, along with the entry in .travis.yml.

Due to the backward-incompatible nature of these changes, my thought is
to release a version 4 of shoulda that includes them.

houndci-bot · 2016-06-10T23:29:53Z

features/support/env.rb

+
+module RailsHelpers
+  def rails_version
+    Gem::Version.new(Bundler.definition.specs['rails'][0].version)


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

I don't think a type of quote is being used consistently in this project, although single quotes seems to be used more often. I'm going to ignore these warnings.

mcmire · 2016-07-13T17:29:04Z

@rmm5t Since you have done a lot of work on this for a while, what do you think about this change? I know you may not have context on this so let me know if you want me to give some.

rmm5t · 2016-07-13T18:13:42Z

@mcmire Fine by me. I only rely on shoulda-matchers directly anymore. I just have one legacy project that still has a direct shoulda dependency, but upon quick glance, these dependency changes look sane to me.

samuelpismel · 2016-10-19T17:07:58Z

No merge?

romdim · 2017-01-17T16:15:17Z

@rmm5t / @mcmire Could you please merge? Thanks!

rmm5t · 2017-01-17T18:33:13Z

@romdim It's been a while since this branch's build ran green. I'm happy to merge, but I don't have the time this week to debug in the event that new versions of dependencies turn the build red. @mcmire Do you have time to merge and release a new gem version?

mcmire · 2017-01-18T16:23:05Z

@rmm5t Sure. I'll take a look at this some time this week.

houndci-bot · 2017-01-30T14:46:34Z

features/support/env.rb

+
+module RailsHelpers
+  def rails_version
+    Gem::Version.new(Bundler.definition.specs['rails'][0].version)


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2017-01-30T14:46:34Z

features/support/env.rb

+  end
+
+  def append_to_gemfile(contents)
+    append_to('Gemfile', contents)


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2017-01-30T14:46:34Z

features/support/env.rb

+
+module AppendHelpers
+  def append_to(path, contents)
+    cd('.') do


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2017-01-30T14:46:35Z

features/support/env.rb

@@ -1,5 +1,8 @@
 require 'aruba/cucumber'

+PROJECT_ROOT = File.expand_path(File.join(File.dirname(__FILE__), '..', '..')).freeze
+APP_NAME     = 'testapp'.freeze


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2017-01-30T14:46:35Z

features/support/env.rb

@@ -1,5 +1,8 @@
 require 'aruba/cucumber'

+PROJECT_ROOT = File.expand_path(File.join(File.dirname(__FILE__), '..', '..')).freeze


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
Line is too long. [85/80]

houndci-bot · 2017-01-30T14:46:35Z

features/step_definitions/rails_steps.rb


-  def append_to_gemfile(contents)
-    append_to('Gemfile', contents)
+Then /^the output should indicate that (\d+) tests were run successfully$/ do |number|


Line is too long. [86/80]

houndci-bot · 2017-01-30T14:46:36Z

features/step_definitions/rails_steps.rb

 end

-When /^I configure a wildcard route$/ do
+When 'I configure a wildcard route' do


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2017-01-30T14:46:36Z

features/step_definitions/rails_steps.rb

-When /^I configure the application to use shoulda-matchers$/ do
-  append_to_gemfile "gem 'shoulda-matchers', '~> 1.0'"
-  steps %{And I run `bundle install --local`}
+  append_to 'test/test_helper.rb', <<-TEXT


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2017-01-30T14:46:36Z

features/step_definitions/rails_steps.rb

-
-When /^I configure the application to use rspec\-rails$/ do
-  append_to_gemfile "gem 'rspec-rails'"
+When 'I configure the application to use Shoulda' do


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2017-01-30T14:46:37Z

features/step_definitions/rails_steps.rb

-APP_NAME     = 'testapp'.freeze
-
-When /^I generate a new rails application$/ do
+When 'I generate a new Rails application' do


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2017-02-01T03:20:40Z

spec/warnings_spy/reporter.rb

+            irrelevant_warnings_file.puts(line)
+          end
+        end
+        puts "Non #{project_name} warnings were raised during the test run. These have been written to #{irrelevant_warnings_file.path}."


Line is too long. [137/80]

houndci-bot · 2017-02-01T03:20:40Z

spec/warnings_spy/reporter.rb

+          end
+        end
+        print_divider('-', 75)
+        puts "#{project_name} warnings written to #{relevant_warnings_file.path}."


Line is too long. [82/80]

houndci-bot · 2017-02-01T03:20:40Z

spec/warnings_spy.rb

+      puts "\n--- ERROR IN AT_EXIT --------------------------------"
+      puts "#{error.class}: #{error.message}"
+      puts error.backtrace.join("\n")
+      puts "-----------------------------------------------------"


Prefer single-quoted strings when you don't need string interpolation or special symbols.

houndci-bot · 2017-02-01T03:20:43Z

spec/support/tests/bundle.rb

+      @fs = Filesystem.new
+    end
+
+    def updating(&block)


Unused method argument - block. If it's necessary, use _ or _block as an argument name to indicate that it won't be used. You can also write as updating(*) if you want the method to accept any arguments but don't care about them.

houndci-bot · 2017-02-01T03:20:44Z

spec/support/acceptance/matchers/indicate_that_tests_were_run_matcher.rb

+
+      def formatted_actual_output
+        if actual_output.empty?
+          "Actual output: (empty)"


Prefer single-quoted strings when you don't need string interpolation or special symbols.

houndci-bot · 2017-02-01T03:20:44Z

spec/support/acceptance/matchers/indicate_that_tests_were_run_matcher.rb

+        if matching_expected_output
+          "Expected output:\n#{matching_actual_output}"
+        else
+          "Expected output: (n/a)"


Prefer single-quoted strings when you don't need string interpolation or special symbols.

houndci-bot · 2017-02-03T04:08:25Z

spec/support/acceptance/helpers/step_helpers.rb

+    def create_rails_application
+      fs.clean
+
+      command = "bundle exec rails new #{fs.project_directory} --skip-bundle --no-rc"


Line is too long. [85/80]

This commit is long overdue. The main change is to upgrade shoulda's hard dependency on shoulda-matchers from 2.x to 3.x. Since this by itself is a pretty consequential change, though, I also took the opportunity to make some more at the same time that I think are reasonable: * Remove support for Rails 3, Rails 4.0, and Rails 4.1, since they have been end-of-lifed; support Rails 4.2-5.0 instead. * Remove support for Ruby 1.9 and 2.0; support Ruby 2.1-2.4 instead. * Remove support for RSpec, along with related Cucumber specs. People who use RSpec won't be using the `shoulda` gem -- they'll be using `shoulda-matchers` instead. There's already a good collection of integration specs there, so we don't need the tests here. Please note that we are using the Rails Git repo to refer to 4.2 because the latest release of 4.2 isn't compatible with Ruby 2.4 just yet (but it's fixed on the 4-2-stable branch).

houndci-bot · 2017-02-03T06:35:22Z

spec/support/acceptance/matchers/indicate_that_tests_were_run_matcher.rb

+
+      def expected_output
+        total = series.inject(:+)
+        /#{total} (tests|runs), #{total} assertions, 0 failures, 0 errors(, 0 skips)?/


Line is too long. [86/80]

houndci-bot · 2017-02-03T06:35:22Z

spec/support/acceptance/matchers/indicate_number_of_tests_was_run_matcher.rb

+      private
+
+      def expected_output
+        /#{number} (?:tests?|runs?|examples?)(?:, #{number} assertions)?, 0 failures(?:, 0 errors(?:, 0 skips)?)?/


Line is too long. [114/80]

houndci-bot · 2017-02-03T06:45:14Z

spec/warnings_spy/reporter.rb

+    def_delegators :partitioner, :relevant_warning_groups,
+      :irrelevant_warning_groups
+
+    def report_relevant_warning_groups


Assignment Branch Condition size for report_relevant_warning_groups is too high. [15.03/15]

houndci-bot · 2017-02-03T06:45:14Z

spec/support/tests/command_runner.rb

+    attr_accessor :command_prefix, :run_quickly, :run_successfully, :retries,
+      :timeout
+
+    def initialize(*args)


Assignment Branch Condition size for initialize is too high. [18.06/15]

houndci-bot · 2017-02-03T06:45:14Z

spec/support/acceptance/matchers/indicate_number_of_tests_was_run_matcher.rb

+        message = "Expected output to indicate that #{some_tests_were_run}.\n" +
+          "Expected output: #{expected_output}\n"
+
+        if actual_output.empty?


Use the return of the conditional for variable assignment and comparison.

houndci-bot · 2017-02-03T06:45:15Z

spec/support/acceptance/add_shoulda_to_project.rb

+      files = []
+
+      if integrates_with_nunit_and_rails?(test_framework, libraries) ||
+        integrates_with_nunit_only?(test_framework)


Use 4 (not 2) spaces for indenting a condition in an if statement spanning multiple lines.

houndci-bot · 2017-02-03T06:45:15Z

spec/support/acceptance/add_shoulda_to_project.rb

+      options[:test_frameworks].each do |test_framework|
+        libraries = options.fetch(:libraries, [])
+
+        test_helper_files_for(test_framework, libraries).each do |test_helper_file|


Line is too long. [83/80]

houndci-bot · 2017-02-03T06:45:15Z

spec/support/acceptance/add_shoulda_to_project.rb

+        add_configuration_block_to(
+          test_helper_file,
+          test_framework,
+          library


Put a comma after the last parameter of a multiline method call.

houndci-bot · 2017-02-03T06:45:15Z

spec/report_warnings.rb

@@ -0,0 +1,7 @@
+require File.expand_path('../warnings_spy', __FILE__)
+
+# Adapted from <http://myronmars.to/n/dev-blog/2011/08/making-your-gem-warning-free>


Line is too long. [84/80]

houndci-bot · 2017-02-03T06:45:16Z

spec/acceptance/rails_integration_spec.rb

+
+    expect(result).to indicate_that_tests_were_run(unit: 1, functional: 1)
+    expect(result).to have_output(
+      'User should validate that :name cannot be empty/falsy'


Put a comma after the last parameter of a multiline method call.

houndci-bot · 2017-02-03T06:45:16Z

spec/acceptance/rails_integration_spec.rb

+      add_gems_for_n_unit
+      add_shoulda_to_project(
+        test_frameworks: [:minitest],
+        libraries: [:rails]


Put a comma after the last parameter of a multiline method call.

Copy over the acceptance test framework that we are using for shoulda-matchers. This is superior because as a part of running the acceptance, we execute commands and we have it so that if one of those commands fails, the output from that command is printed to the console so that we can better debug the issue. This is something that Cucumber (via Aruba) does not do.

houndci-bot · 2017-02-03T06:54:51Z

spec/support/acceptance/add_shoulda_to_project.rb

+    def test_helper_files_for(test_framework, libraries)
+      files = []
+
+      if integrates_with_nunit_and_rails?(test_framework, libraries) || integrates_with_nunit_only?(test_framework)


Line is too long. [115/80]

houndci-bot · 2017-02-03T06:54:51Z

spec/support/acceptance/add_shoulda_to_project.rb

+    def test_helper_files_for(test_framework, libraries)
+      files = []
+
+      if integrates_with_nunit_and_rails?(test_framework, libraries) || integrates_with_nunit_only?(test_framework)


Line is too long. [115/80]

houndci-bot · 2017-02-03T06:54:51Z

spec/acceptance/rails_integration_spec.rb

+      end
+    FILE
+
+    run_rake_tasks!(["db:drop", "db:create", "db:migrate"])


Prefer single-quoted strings when you don't need string interpolation or special symbols.

houndci-bot · 2017-02-03T06:54:51Z

spec/acceptance/rails_integration_spec.rb

+      end
+    FILE
+
+    run_rake_tasks!(["db:drop", "db:create", "db:migrate"])


Prefer single-quoted strings when you don't need string interpolation or special symbols.

houndci-bot reviewed Jun 10, 2016
View reviewed changes

mcmire mentioned this pull request Jun 10, 2016

Dependence issue with shoulda-matchers 3 and should 3.5 thoughtbot/shoulda-matchers#825

Closed

mcmire self-assigned this Jan 18, 2017

houndci-bot reviewed Jan 30, 2017

View reviewed changes

houndci-bot reviewed Feb 1, 2017

View reviewed changes

mcmire force-pushed the ew-out-with-the-old branch from 14d09fa to 9fc3490 Compare February 3, 2017 04:08

houndci-bot reviewed Feb 3, 2017

View reviewed changes

mcmire force-pushed the ew-out-with-the-old branch from 9fc3490 to d31eb64 Compare February 3, 2017 04:13

mcmire force-pushed the ew-out-with-the-old branch from d31eb64 to 04e4ad2 Compare February 3, 2017 04:39

houndci-bot reviewed Feb 3, 2017

View reviewed changes

mcmire force-pushed the ew-out-with-the-old branch from 0eca60d to 27151cf Compare February 3, 2017 06:44

houndci-bot reviewed Feb 3, 2017

View reviewed changes

mcmire added 2 commits February 3, 2017 07:54

Add reasonable Rubocop configuration

9738074

mcmire force-pushed the ew-out-with-the-old branch from 27151cf to d9c504f Compare February 3, 2017 06:54

houndci-bot reviewed Feb 3, 2017

View reviewed changes

mcmire merged commit d9c504f into master Feb 3, 2017

mcmire deleted the ew-out-with-the-old branch February 3, 2017 06:58

sman591 mentioned this pull request Jul 1, 2018

shoulda-matchers #263

Closed

		@@ -1,5 +1,8 @@
		require 'aruba/cucumber'

		PROJECT_ROOT = File.expand_path(File.join(File.dirname(__FILE__), '..', '..')).freeze

		@@ -0,0 +1,7 @@
		require File.expand_path('../warnings_spy', __FILE__)

		# Adapted from <http://myronmars.to/n/dev-blog/2011/08/making-your-gem-warning-free>

Out with the old, in with the new #261

Out with the old, in with the new #261

Conversation

mcmire commented Jun 10, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcmire commented Jul 13, 2016 • edited Loading

rmm5t commented Jul 13, 2016

samuelpismel commented Oct 19, 2016

romdim commented Jan 17, 2017

rmm5t commented Jan 17, 2017

mcmire commented Jan 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcmire commented Jul 13, 2016 •

edited

Loading