Fix spec type inferrence #1002
Merged
Fix spec type inferrence #1002
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| config.before(:context, :type => :controller) do | ||
| puts "Running a controller example group" | ||
| end | ||
| end |
There was a problem hiding this comment.
This seems anecdotal to the feature, but I'm guessing you went this way because it's easy to prove it works? Do you intend to leave this in here?
There was a problem hiding this comment.
It's intentionally here as the test coverage I added for #825, but you're right it's odd here. I think I can use some of what I did to the be_included_in_files_in custom matcher to test this in a spec rather than here (I updated the matcher last). I'll take a stab at that.
|
I'm a fan of this. ❤️ |
This lets us re-use that method when we isolate config in our specs.
- Use a shared example group instead of a matcher. There are many more aspects to test than just one matchers worth. - Rather than stubbing the metadata, actually write out spec files in an appropriate directory. - Add coverage for `spec/api` mapping to `:request` specs.
- In #970 we added `infer_spec_type_from_file_location!` but forgot to remove the code in `lib/rspec/rails/configuration.rb` that made it always infer - so the opt-in API was there but it was always enabled. Here I've made it truly opt-in. - The logic of `infer_spec_type_from_file_location!` also setup the helper module inclusion via explicit `:type` metadata but was only intended to setup the implicit mapping of spec file location to type. Here I've made the module inclusion via `:type` always work w/o an explicit config option needed to enable it. - We used to mutate the `:type` metadata on inclusion of the helper module, but this caused bugs such as #825. The problem is that rspec-core uses a raw hash for the metadata and doesn't re-apply filtering/inclusion logic when the metadata hash is mutated. Instead, we use a new explicit `define_derived_metadata` API. Fixes #825 and closes #829.
There's no need to anymore, and it created some surprising bugs (such as #825: `before(:all)` hooks would not properly be applied because they were registered before the modules were included).
|
OK, @JonRowe, I fixed this up. The tests are improved even more and there are no cuke changes. AFAIK this is ready to merge except for the rspec-core stuff needing to get done first (which I plan to take care of later tonight if I can find time). |
|
Sorry on the run and not able to review this tonight. Thanks for tackling! |
| # only on 1.9.2 (and only on travis; can't repro locally). The warning is: | ||
| # /home/travis/.rvm/rubies/ruby-1.9.2-p320/lib/ruby/1.9.1/net/smtp.rb:584: warning: previous definition of tlsconnect was here | ||
| # For now, we're just going to silence the warning. | ||
| around { |ex| with_isolated_stderr(&ex) } if RUBY_VERSION == '1.9.2' |
|
LGTM, merge when green |
This was referenced Apr 22, 2014
Mutating metadata is problematic so using an ivar is better. Also, remove a deprecated API while we're at it.
Just set the controller class instead. Mutating metadata has some odd edge case bugs (as rspec-core doesn't update it's filters/hooks/etc when metadata is mutated), so it's best to avoid it.
[ci skip]
|
I just force pushed with the Gemfile change removed. I'll merge when green. |
|
LGTM sorry for the late review. |
amatriain
added a commit
to amatriain/feedbunch
that referenced
this pull request
Jun 3, 2014
…rectories. Added metadata to all tests to let rspec-rails know the type of each test suite (controller, model, feature etc). Older rspec-rails versions demanded a particular directory structure for the specs, so that e.g. controllers tests (which had to be in /specs/controllers) had a module mixed in during testing, to enable helper methods necessary for each type of test (e.g. accessing response headers directly during controller tests). This is being deprecated and it will become opt-in in rspec-rails 3. A particular directory structure will no longer be enforced, each test suite will have to specify the type of test by using metadata (a :type named argument to the "describe" method at the top of each file). I've gone ahead and, in preparation for rspec-rails 3, I've added metadata to all tests which make use of rspec-rails helper methods. Now that a particular directory structure is no longer enforced, I've reorganized tests in a "unit_tests" and "acceptance_tests" structure which is more confortable for me. For details about the old directory structure and the new metadata-based spect type specification, see: https://relishapp.com/rspec/rspec-rails/v/3-0/docs/directory-structure For interesting discussions about this topic see: rspec/rspec-rails#662 https://github.com/rspec/rspec-rails/pull/970/files rspec/rspec-rails#1002
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix spec type inferrence.
infer_spec_type_from_file_location!but forgotto remove the code in
lib/rspec/rails/configuration.rbthatmade it always infer - so the opt-in API was there but it
was always enabled. Here I've made it truly opt-in.
infer_spec_type_from_file_location!also setupthe helper module inclusion via explicit
:typemetadata butwas only intended to setup the implicit mapping of spec file
location to type. Here I've made the module inclusion via
:typealways work w/o an explicit config option needed toenable it.
:typemetadata on inclusion of the helpermodule, but this caused bugs such as Global before :all hooks with specified :type are not run with 2.14.0 #825. The problem is that
rspec-core uses a raw hash for the metadata and doesn't re-apply
filtering/inclusion logic when the metadata hash is mutated.
Instead, we use a new explicit
define_derived_metadataAPI.Fixes #825 and closes #829.
/cc @JonRowe @cupakromer @alindeman @xaviershay