allow helper module specs to spec internal classes, fix #1339

[pdswan]

No description provided.

[JonRowe]

This isn't necessarily present and in anycase is superfluous as below

[pdswan]

@JonRowe thanks for the feedback.

What i'm running into is a bit tricky and due to a confluence of issues:

• Fix nested described class rspec-core#1361 changes the behavior of described_class

• the change to described_class means that in RSpec 3.x the following is true:

  describe FoosHelper do
    specify { expect(described_class).to eq(FoosHelper) }
  
    describe FoosHelper::InternalClass do
      specify { expect(described_class).to eq(FoosHelper::InternalClass) }
    end
  end

  whereas in RSpec 2.x it was:

  describe FoosHelper do
    specify { expect(described_class).to eq(FoosHelper) }
  
    describe FoosHelper::InternalClass do
      specify { expect(described_class).to eq(FoosHelper) }
    end
  end

• ActiveSupport/ActionView 4.x provide facilities for excluding Classes from being returned by determine_default_helper_class through the use of determine_constant_from_test_name, but this functionality is not available in ActiveSupport 3.x.

I suppose writing this out makes the solution more obvious: the behavior should be the same regardless of the presence of determine_constant_from_test_name.

[JonRowe]

• ActiveSupport/ActionView 4.x provide facilities for excluding Classes from being returned by determine_default_helper_class through the use of determine_constant_from_test_name, but this functionality is not available in ActiveSupport 3.x.

This is what needs to be addressed, the changes to the behaviour of rspec-core are expected and desired.

[pdswan]

@JonRowe agreed. execution of the intention got muddled while chasing down dependencies and figuring out what was going on.

apologies for the PR noise, didn't have things set up to test across multiple versions of rails locally, realize now i opened this too early.

[cupakromer]

My comments from the initial issue:

I'm not sure this is something we should support.

Metadata is inherited by nested example groups and examples. An example group with metadata type: :helper has specific meaning in rspec-rails. By writing the inner child class' specs as a nested example group it inherits the type: :helper metadata. However, it is not an example group for a helper.

My suggested course of action would be one of the following:

• Implicitly test the inner class via the helper's methods
• Break the inner class' specs into a new top-level group or even it's own spec file

I did notice that even if the type is overwritten in a nested context it still has the same problem. This is because the parent example group class, which the nested example group class is a child of, has already included the HelperExampleGroup. Changing the metadata after the fact won't change that. Due to this, making changes to the helper group behavior makes me a little concerned what other issues it will cause because of wiring to the inner workings of Rails land.

[pdswan]

@cupakromer think that's a fair point, though as someone navigating the upgrade process and continuing to leverage infer_spec_type_from_file_location! this was an unexpected error that was relatively difficult to diagnose.

Actually, as I look at this more I think there may be one very compelling reason to support this use case: the definition of determine_default_helper_class in ActionView::TestCase::Behavior::ClassMethods already supports it in addition to supporting the behavior implemented by RSpec::Rails::HelperExampleGroup; i.e. I think this code can be removed entirely from rspec-rails. Maybe I'm missing something?

Going to run some exploratory tests now.

On that note, I'm having difficulty replicating green builds locally using other versions of rails as described at https://github.com/rspec/rspec-rails/blob/master/README_DEV.md

Am I doing something wrong?

pdswan ~/code/forks/rspec-rails (master) $ ruby -v
ruby 2.1.5p273 (2014-11-13 revision 48405) [x86_64-darwin13.0]
pdswan ~/code/forks/rspec-rails (master) $ bin/thor version:use 3.1.12
      remove  Gemfile.lock
         run  echo '3.1.12' > ./.rails-version from "."
         run  bundle install --binstubs from "."
The git source git://github.com/rspec/rspec.git is not yet checked out. Please run `bundle install` before trying to start your application
pdswan ~/code/forks/rspec-rails (master) $ bundle install --binstubs
pdswan ~/code/forks/rspec-rails (master) $ bin/rspec spec/rspec/rails/example/feature_example_group_spec.rb:8
[full spec output omitted]
Finished in 0.13075 seconds (files took 2.03 seconds to load)
1 example, 1 failure

Failed examples:

rspec ./spec/rspec/rails/example/feature_example_group_spec.rb:8 # RSpec::Rails::FeatureExampleGroup includes Rails route helpers

appreciate the help, thanks.

[cupakromer]

@pdswan were you able to make more progress on this? I think supporting determine_default_helper_class is the way to go.

[fables-tales]

@pdswan are you likely to be able to look at this any time soon? If not, don't worry. I'd like to see this merged, and I'm happy to finish it off otherwise.

[pdswan]

@samphippen unlikely i'll be able to look at it soon. thanks for checking in and picking it up; much appreciated!

[fables-tales]

@pdswan closing this in favour of #1499 which I can push to.