Rspec metadata integration names file '.yml' when using rspec one-liner syntax #378

Open
jspooner opened this Issue Jan 22, 2014 · 14 comments

Comments

Projects
None yet
5 participants

When using the Rspec metadata integration and one-line syntax it { should be_valid } VCR names the cassette file ".yml". This was quite confusing at first file is invisibe yet vcr still works.

Rspec is able to take a test like it { longitude.should == -122.0839544 } and generate a description like Model initialization with longitude should == -122.0839544. I believe the file name should be something like 'should_eql_122.0839544.yml'.

Here is how I see this fitting into rspec_metadata.feature.

diff --git a/features/test_frameworks/rspec_metadata.feature b/features/test_frameworks/rspec_metadata.feature
index b0d296c..fc64d1b 100644
--- a/features/test_frameworks/rspec_metadata.feature
+++ b/features/test_frameworks/rspec_metadata.feature
@@ -76,6 +76,8 @@ Feature: Usage with RSpec metadata
           make_http_request.should == 'Hello'
         end

+        it { make_http_request.should == 'Hello' }
+
         context 'in a nested example group' do
           it 'records another one' do
             make_http_request.should == 'Hello'
@@ -93,6 +95,7 @@ Feature: Usage with RSpec metadata
     Then it should pass with "4 examples, 0 failures"
      And the file "spec/cassettes/VCR_example_group_metadata/records_an_http_request.yml" should contain "Hello"
      And the file "spec/cassettes/VCR_example_group_metadata/records_another_http_request.yml" should contain "Hello"
+     And the file "spec/cassettes/VCR_example_group_metadata/should_be_Hello.yml" should contain "Hello"
      And the file "spec/cassettes/VCR_example_group_metadata/in_a_nested_example_group/records_another_one.yml" should contain "Hello"
      And the file "spec/cassettes/VCR_example_metadata/records_an_http_request.yml" should contain "Hello"
Owner

myronmarston commented Jan 22, 2014

With how RSpec's generated descriptions work, I don't think this can possibly work. Here's why:

  • VCR has to know what the cassette name is when the cassette is inserted (e.g. before the body of the example runs).
  • RSpec's generates the description of the example based on the description of the matcher of the last expectation in the example. RSpec has no way to provide the description w/o running the example, which means it cannot provide a description for the example at the time VCR needs it.

I think this is just one of those cases where there's no way to reconcile how the RSpec feature works with how the VCR feature works. We could probably improve VCR's RSpec metadata integration so that it provides a clear warning and/or error for this case, though.

Yeah I've been debugging the metadata and I understand why it could be impossible. When using one line syntax the metadata[:description_args] is an empty array. I also see metadata[:example_group_block] that contains a proc the looks like the test. I'm not sure if there is anyway to get the expectation out of that Proc.

My temporary fix is to just disable configure_rspec_metadata! and handle the file naming in my spec_helper.rb file like this.

config.around(:each) do |example|
    if example.metadata[:vcr]
      name = example.metadata[:full_description].split(/\s+/, 2).join("/").underscore.gsub(/[^\w\/]+/, "_")
      # 'it {}' one line syntax if used with VCR.configure.configure_rspec_metadata! will name the file '.yml'
      # so as a temporary fix we just use the line number
      if example.metadata[:description_args].empty?
        example.metadata[:description_args] << "line_#{example.metadata[:line_number]}"
      end
      VCR.use_cassette(name, {record: :new_episodes}, &example)
    else
      VCR.turned_off(&example)
    end
  end
Owner

myronmarston commented Jan 22, 2014

I'm not sure if there is anyway to get the expectation out of that Proc.

There's no way to get the description without running the proc, but that can have all sorts of negative side effects.

My suggestion is to do this:

example vcr: { cassette_name: "name it manually" } do
  # your one liner
end

(You could also use it or specify instead of example, but the point of it is to have a nice readable expression for what comes after, and the metadata in there doesn't read well off of it).

Yeah I think naming the cassette in the test is reasonable. However we should fix the invisible file problem. This has tripped up my guys on several occasions.

  1. The file name should default to something like undefined_cassette_name.yml or undefined_cassette_on_line_4.yml
  2. Or just print a warning like "one line rspec syntax is unable to create a casset_name. Please define one 'it vcr: {cassette_name: "myName"} { should be_true }"

Let me know what you prefer and find time to submit a pull request.

Owner

myronmarston commented Jan 22, 2014

I don't like option #1. undefined_cassette_name.yml will cause problems if there are multiple one-liners, and undefiend_cassette_on_line_4.yml will not work when the example moves down further in the file (e.g. if you add another example above it). Both are problematic.

Option #2 is better, but I wonder if an error is better than a warning?

Yeah I like the idea of an error so that the .yml file is never created.

Owner

myronmarston commented Jan 22, 2014

Sounds good, want to open a PR?

Ok I figured out the issue and we don't need to implement either of our options. The problem is that description is a blank string here https://github.com/vcr/vcr/blob/master/lib/vcr/test_frameworks/rspec.rb#L14

I'll submit a PR but what branch should I use? I had a bunch of failing test on the master branch when I checked it out.

Owner

myronmarston commented Jan 23, 2014

Use master. We had a green travis build yesterday:

https://travis-ci.org/vcr/vcr/builds/17418196

What failures are you seeing?

@jspooner jspooner added a commit to jspooner/vcr that referenced this issue Jan 23, 2014

@jspooner jspooner Fix issue #378 Rspec integration names file '.yml' when using rspec o…
…ne line syntax
3e86333

Ok I'm raising an error from the before block but the error message is a little weird because it says Failure/Error: Unable to find matching line from backtrace before showing my error messages. You can see it below in my console output.

PR: jspooner#1

Console output

rspec spec/vcr/test_frameworks/rspec_spec.rb
WARNING: VCR::RSpec::Macros is deprecated. Use RSpec metadata options instead `:vcr => vcr_options`
Run options: include {:focus=>true}

All examples were filtered out; ignoring {:focus=>true}
..............F.

Failures:

  1) VCR::RSpec::Metadata an example group with a nested example group 
     Failure/Error: Unable to find matching line from backtrace
     VCR::Errors::NotSupportedError:
       VCR does not support rspec one line syntax 'it {}' found on line 19
     # ./lib/vcr/test_frameworks/rspec.rb:32:in `block (2 levels) in configure!'

Finished in 0.00996 seconds
16 examples, 1 failure

Failed examples:

rspec ./spec/vcr/test_frameworks/rspec_spec.rb:19 # VCR::RSpec::Metadata an example group with a nested example group 

Randomized with seed 14798

Alternatively we could mark the test as pending like this
jspooner/vcr@master...issue-378-markaspending

diff --git a/lib/vcr/test_frameworks/rspec.rb b/lib/vcr/test_frameworks/rspec.rb
index 0f3f094..82d5054 100644
--- a/lib/vcr/test_frameworks/rspec.rb
+++ b/lib/vcr/test_frameworks/rspec.rb
@@ -27,7 +27,13 @@ module VCR

             cassette_name = options.delete(:cassette_name) ||
                             vcr_cassette_name_for[example.metadata]
-            VCR.insert_cassette(cassette_name, options)
+
+            if cassette_name.strip[-1,1] == "/"
+              pending "VCR does not support rspec one line syntax on line #{example.metadata[:line_number]}"
+            else
+              VCR.insert_cassette(cassette_name, options)
+            end
+            
           end

           config.after(:each, when_tagged_with_vcr) do |ex|
(END) 

I was just testing this in my project and I actually prefer marking them as pending because the output is much more precise. And with 35 test to fix this output will ease the pain.

Pending:
  CityGrid::OfferBuilder An Ad An offers creative
    # VCR does not support rspec one line syntax on line 47
    # ./spec/models/city_grid/offer_builder_spec.rb:47
  CityGrid::OfferBuilder An Ad An offers creative template_id
    # VCR does not support rspec one line syntax on line 50
    # ./spec/models/city_grid/offer_builder_spec.rb:50

any updates on this?

Owner

myronmarston commented Oct 20, 2014

There's #382 that was an in progress solution, and I left feedback, but it's just sat since then.

krainboltgreene added the bug label Jan 3, 2015

krainboltgreene self-assigned this Jan 3, 2015

krainboltgreene added this to the v3.0.0 milestone Jan 3, 2015

Contributor

tessi commented Oct 30, 2015

I've opened a PR for another approach -- we could use rspecs nested scope for name the cassette -- see #514 for details.

krainboltgreene removed this from the v3.0.0 milestone Nov 6, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment