old rspec syntax in web_steps gives deprecation warnings

[tansaku]

  Scenario: find movie with same director                       # features/movies.feature:22
    Given I am on the details page for "Star Wars"              # features/step_definitions/web_steps.rb:44
    When I follow "Find Movies With Same Director"              # features/step_definitions/web_steps.rb:56

DEPRECATION: Using should from rspec-expectations' old :should syntax without explicitly enabling the syntax is deprecated. Use the new :expect syntax or explicitly enable :should with config.expect_with(:rspec) { |c| c.syntax = :should } instead. Called from /home/ubuntu/workspace/hw-acceptance-unit-test-cycle/rottenpotatoes/features/step_definitions/web_steps.rb:233:in `block in <top (required)>'.

[armandofox]

a PR to change to expect sntax would be welcome

[RobertStroud]

It's fairly easy to fix - you have to change about three lines in web_steps.rb from

foo.should match(bar)

to

expect(foo).to match(bar)

An alternative fix would be enable "should" syntax in the rspec configuration file, as per the deprecation warning above. I tried adding the necessary line to spec/rails_helper.rb in the obvious place, but it didn't work - I don't know why not...

[RobertStroud]

@armandofox - upon further investigation, it turns out that generating a pull request to fix this issue is not straightforward for a number of reasons.

The deprecated code in web_steps.rb is installed as part of cucumber-rails-training-wheels, which is itself deprecated - in fact, the author of cucumber-rails-training-wheels has promised that he will never ever upgrade the code:

This gem exists for the sole purpose of helping people who are reading books or blogs that were written prior to Cucumber-Rails 1.1.0. I really want this code to go away and be forgotten.

https://github.com/cucumber/cucumber-rails-training-wheels

In principle, you could patch web_steps.rb to make the deprecation warnings go away, but since web_steps.rb is installed as part of cucumber-rails-training-wheels, you would have to ensure that your changes were not overwritten when you installed cucumber-rails-training-wheels.

I think there are four possible solutions to the problem:

1. Don't install cucumber-rails-training-wheels in the first place
2. Use an earlier version of RSpec that doesn't clash with cucumber-rails-training-wheels
3. Configure RSpec not to generate deprecation messages
4. Patch web_steps.rb and somehow prevent it being overwritten

In my opinion:

1. is the best solution in the long-term but has implications for the lecture material.
2. is a quick and easy fix in the short-term, but a bit of an ostrich solution
3. is slightly better than 2 but still basically avoids the problem
4. is somewhat worse than 3, since it involves more extensive changes

My solution was a subset of 4 because I couldn't figure out how to get 3 to work.

As an experiment, I tried installing the project without cucumber-rails-training-wheels, but I quickly ran into a problem. In principle, it would be possible to do without web_steps.rb, and write movie_steps.rb using the Capybara API directly - however, the difficulty is that paths.rb is also part of cucumber-rails-training-wheels. On the face of it, paths.rb seems less egregious than web_steps.rb, but you can't have one without the other - it's both or none. But eliminating paths.rb means you need a different way of implementing Cucumber steps that refer to particular pages, so the change is more far-reaching than you might expect.

[armandofox]

i agree with your assessment. i'd do (3) for now - i'll try to address (1) in the Fall lecture materials but the changes are significant since it also entails changes to the book and requires explaining some of capybara before any of cucumber can be explained. i think the suppression of deprecation warnings in this narrow case is defensible, since we go to great lengths later to tell people not to use web_steps as a crutch and instead write their own step defs. i've been over this with a couple of people and keep coming back to the same opinion - training wheels have a valid purpose :-)

[RobertStroud]

@armandofox - thanks. I agree that (3) is the most pragmatic solution for now, but unfortunately, I can't get it to work. In particular, inserting the following line of code after line 31 of spec/spec_helper.rb doesn't fix the problem:

expectations.syntax = :should

I'm also concerned that spec/spec_helper.rb is generated automatically when you install RSpec, so this line of code should really go somewhere else, perhaps in .rspec-local. However, I haven't been able to figure out the correct syntax for specifying a value for the expect_with(:rspec).syntax configuration option via .rspec-local.

[armandofox]

You're right that running rspec:install creates a spec-helper file that doesn't include the workaround for deprecation. I think we may just need to instruct the students to add the following inside the configure block in that file (ie provide explicit instructions and an explanation in the toplevel README), which would allow either the old or new syntax to be used:

RSpec.configure do |config|
  # ...
  config.mock_with :rspec do |c|
    c.syntax = [:should, :expect]
  end
  config.expect_with :rspec do |c|
    c.syntax = [:should, :expect]
  end
end

[RobertStroud]

@armandofox - as noted earlier, specifying a value for the expect_with config option in spec/spec_helper.rb has no effect on Cucumber. You still get the DEPRECATION message.

According to the following web page, if you want to configure RSpec for Cucumber, you need to add some code to features/support/env.rb.

https://github.com/cucumber/cucumber/wiki/RSpec-Expectations

However, features/support/env.rb is generated when you install cucumber-rails and you are advised not to edit not to edit it directly:

# Consider adding your own code to a new file instead of editing this one. 
# Cucumber will automatically load all features/**/*.rb files.

Hence, the best solution is to ask students to create a separate features/support/rspec.rb file that contains the relevant code:

require 'rspec/core'

RSpec.configure do |config|
  config.mock_with :rspec do |c|
    c.syntax = [:should, :expect]
  end
  config.expect_with :rspec do |c|
    c.syntax = [:should, :expect]
  end
end

I think this approach is better than asking students to edit an auto-generated file. I can also confirm that it works!

[armandofox]

Sounds like a reasonable solution to me - want to implement it and do a PR? thanks for staying with this!

[RobertStroud]

@armandofox - thanks. I've just created a pull request to update README.md accordingly.