-
-
Notifications
You must be signed in to change notification settings - Fork 1k
Fix WrongScopeError being thrown on Rails master:
#2215
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Appears the fix triggered an existing rubocop error. |
JonRowe
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A minor change suggested, and is there a test that fails without this?
5bfbfff to
12b0e5d
Compare
|
Thanks for the quick review. I made the suggested changes |
lib/rspec/rails/fixture_support.rb
Outdated
| end | ||
| end | ||
|
|
||
| if ::ActiveRecord.version >= Gem::Version.new('6.1.0') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry you can't use Gem::Version, as we don't have a dependency on rubygems being available. A simple
| if ::ActiveRecord.version >= Gem::Version.new('6.1.0') | |
| if Rails.version.to_f > 6.0 |
is preferred.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rubygem is part of the standard library since ruby 1.9. I can use Rails.version.to_f if you really want to but I don't see any real reason
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't load any part of the standard library unless 100% necessary as it causes environment poisoning (where someone might not have required the right file but we do, tests pass, but it fails outside of tests). There is also a speed benefit to not using rubygems which we ourselves utilise in our tests by disabling it (which gives us the speed benefit and tests we're not poisoning the env).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
e.g Its simpler to not use it so please don't :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok I'll use Rails.version.to_f, thanks for quick answer :)
Not trying to argue on such a small detail but Rails.version already loads rubygem 🤷♂ . But anyway having the syntax you proposed will make things consistent since that's what it's used in this library.
| end | ||
|
|
||
| it "doesn't raise a WrongScopeError on Rails 6.1" do | ||
| allow(ActiveRecord).to receive(:version) { Gem::Version.new('6.1.0') } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Proposed change removes this necessity
|
If you rebase this the rubocop build should be fixed |
|
@Edouard-chin - Looking forward to this fixed version. |
|
+1 |
a428956 to
4f980ff
Compare
|
Rails::VERSION::STRING.to_f |
4f980ff to
0507fc2
Compare
|
Not too sure what's up with the jruby builds, otherwise it's 🍏 |
|
Thanks @pirj. I just reran the 2 builds, the issue on json seems to be fixed now. |
|
@Edouard-chin thanks a lot for the last commit. Do you think you can handle @JonRowe review? Thanks :) |
Sure happy to, but which comment did I not address ? If you are referring to #2215 (comment) I added a test to load fixtures |
|
Sorry @Edouard-chin. I just noticed. @JonRowe are you ok with the last changes? Cheers :) |
| end | ||
|
|
||
| expect { group.new.name }.to_not raise_error | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test can be removed, the other is fine.
JonRowe
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nearly there, thanks for working on this!
a187433 to
921e67d
Compare
|
Rebased and CI is 🍏 . Thanks for your patience |
Fix `WrongScopeError` being thrown on Rails master:
Related errors below.
== Test WrongScopeError
Error:
```
Failures:
1) Posts API (Custom Controller) GET /posts returns a list of posts
Failure/Error:
raise WrongScopeError,
"`#{name}` is not available from within an example (e.g. an " \
"`it` block) or from constructs that run in the scope of an " \
"example (e.g. `before`, `let`, etc). It is only available " \
"on an example group (e.g. a `describe` or `context` block)."
`name` is not available from within an example (e.g. an `it` block) or from constructs that run in the scope of an example (e.g. `before`, `let`, etc). It is only available on an example group (e.g. a `describe` or `context` block).
# /Users/alexkitchens/.rbenv/versions/2.6.1/lib/ruby/gems/2.6.0/gems/rspec-core-3.8.0/lib/rspec/core/example_group.rb:742:in `method_missing'
# /Users/alexkitchens/fun/journey/dependencies/rails/actionpack/lib/action_dispatch/testing/assertions/routing.rb:188:in `method_missing'
# /Users/alexkitchens/fun/journey/dependencies/rails/actionpack/lib/action_dispatch/testing/integration.rb:422:in `method_missing'
# /Users/alexkitchens/fun/journey/dependencies/rails/activerecord/lib/active_record/test_fixtures.rb:101:in `run_in_transaction?'
# /Users/alexkitchens/fun/journey/dependencies/rails/activerecord/lib/active_record/test_fixtures.rb:115:in `setup_fixtures'
# /Users/alexkitchens/fun/journey/dependencies/rails/activerecord/lib/active_record/test_fixtures.rb:8:in `before_setup'
# /Users/alexkitchens/fun/journey/dependencies/rails/actionpack/lib/action_dispatch/testing/integration.rb:331:in `before_setup'
# /Users/alexkitchens/.rbenv/versions/2.6.1/lib/ruby/gems/2.6.0/gems/rspec-rails-3.8.2/lib/rspec/rails/adapters.rb:126:in `block (2 levels) in <module:MinitestLifecycleAdapter>'
# /Users/alexkitchens/.rbenv/versions/2.6.1/lib/ruby/gems/2.6.0/gems/rspec-core-3.8.0/lib/rspec/core/example.rb:447:in `instance_exec'
# /Users/alexkitchens/.rbenv/versions/2.6.1/lib/ruby/gems/2.6.0/gems/rspec-core-3.8.0/lib/rspec/core/example.rb:447:in `instance_exec'
# /Users/alexkitchens/.rbenv/versions/2.6.1/lib/ruby/gems/2.6.0/gems/rspec-core-3.8.0/lib/rspec/core/hooks.rb:373:in `execute_with'
#
# Showing full backtrace because every line was filtered out.
# See docs for RSpec::Configuration#backtrace_exclusion_patterns and
# RSpec::Configuration#backtrace_inclusion_patterns for more information.
```
Bisect led to d4367eb72601f6e5bba3206443e9d141e9753af8
Issue opened here: rails/rails#37783
Fix in Rspec Rails here: rspec/rspec-rails#2215
The fix is not in a full release yet, so I've pinned to the latest beta release.
```
commit d4367eb72601f6e5bba3206443e9d141e9753af8
Author: Edouard CHIN <[email protected]>
Date: Thu Nov 21 18:39:54 2019 +0100
Modify ActiveRecord::TestFixtures to not rely on AS::TestCase:
- ### Problem
If one wants to use ActiveRecord::TestFixtures it is mandatory for
the test suite to inherit from `ActiveSupport::TestCase`.
TestFixtures makes use of specific method from AS::TestCase
(`file_fixture_path` and `method_name`).
### Solution
This PR fixes that by not making use of method_name and file_fixture_path.
```
== Time Issue
Error:
```
Failure/Error: require File.expand_path('../../config/environment', __FILE__)
NoMethodError:
undefined method `hour' for 1:Integer
Run options: include {:locations=>{"./spec/requests/api/posts_spec.rb"=>[10]}}
```
Bisect led to commit 12959dee0f19a8e050dd1236b031c2c690729905
Issue open at rails/rails#37391
Rails 6.1 requires an upgrade to rspec-rails 4.0.0 which fixes a WrongScopeError: rspec/rspec-rails#2215 Rspec-rails 4.0.0 dropped support for Rails below 5.0, so Rails 4 has been removed from the appraisals.
Rails 6.1 requires an upgrade to rspec-rails 4.0.0 which fixes a WrongScopeError: rspec/rspec-rails#2215 Rspec-rails 4.0.0 dropped support for Rails below 5.0, so Rails 4 has been removed from the appraisals.
Rails 6.1 requires an upgrade to rspec-rails 4.0.0 which fixes a WrongScopeError: rspec/rspec-rails#2215 Rspec-rails 4.0.0 dropped support for Rails below 5.0, so Rails 4 has been removed from the appraisals.
Rails 6.1 requires an upgrade to rspec-rails 4.0.0 which fixes a WrongScopeError: rspec/rspec-rails#2215 Rspec-rails 4.0.0 dropped support for Rails below 5.0, so Rails 4 has been removed from the appraisals.
It seems like rails 6.1 broke rspec <4.0 until it was fixed with rspec/rspec-rails#2215 then fixed differently with rspec/rspec-rails#2461 so we skip rspec 3x with rails 6.1 in CI
It seems like rails 6.1 broke rspec <4.0 until it was fixed with rspec/rspec-rails#2215 then fixed differently with rspec/rspec-rails#2461 so we skip rspec 3x with rails 6.1 in CI
It seems like rails 6.1 broke rspec-rails. RSpec-rails 4.0 fixed it with rspec/rspec-rails#2215 then fixed differently with rspec/rspec-rails#2461 so we skip rspec-rails >4 with rails 6.1 in CI
It seems like rails 6.1 broke rspec-rails. RSpec-rails 4.0 fixed it with rspec/rspec-rails#2215 then fixed differently with rspec/rspec-rails#2461 so we skip rspec-rails >4 with rails 6.1 in CI
It seems like rails 6.1 broke rspec-rails. RSpec-rails 4.0 fixed it with rspec/rspec-rails#2215 then fixed differently with rspec/rspec-rails#2461 so we skip rspec-rails >4 with rails 6.1 in CI
It seems like rails 6.1 broke rspec-rails. RSpec-rails 4.0 fixed it with rspec/rspec-rails#2215 then fixed differently with rspec/rspec-rails#2461 so we skip rspec-rails >4 with rails 6.1 in CI
It seems like rails 6.1 broke rspec-rails. RSpec-rails 4.0 fixed it with rspec/rspec-rails#2215 then fixed differently with rspec/rspec-rails#2461 so we skip rspec-rails >4 with rails 6.1 in CI
It seems like rails 6.1 broke rspec-rails. RSpec-rails 4.0 fixed it with rspec/rspec-rails#2215 then fixed differently with rspec/rspec-rails#2461 so we skip rspec-rails >4 with rails 6.1 in CI
Some of the missing combinations in the matrix: * Ruby 3 with Rails 5.2 are skipped because my tests error when trying to start rails * Rspec-rails 3 with Rails 6.1 because of a rails change rspec-rails needs rspec/rspec-rails#2215 or the more recent rspec/rspec-rails#2461
Some of the missing combinations in the matrix: * Ruby 3 with Rails 5.2 are skipped because my tests error when trying to start rails * Rspec-rails 3 with Rails 6.1 because of a rails change rspec-rails needs rspec/rspec-rails#2215 or the more recent rspec/rspec-rails#2461
Some of the missing combinations in the matrix: * Ruby 3 with Rails 5.2 are skipped because my tests error when trying to start rails * Rspec-rails 3 with Rails 6.1 because of a rails change rspec-rails needs rspec/rspec-rails#2215 or the more recent rspec/rspec-rails#2461
There's an issue with Rails 6.1 that caused error `WrongScopeError`, which was only fixed in rspec-rails 4.0 by rspec#2215.
Fix
WrongScopeErrorbeing thrown on Rails master:Problem
Rails made a change in rails/rails@d4367eb
and TestFixtures don't make use of
method_nameanymore, but simplyname.nameon RSpec raises aWrongScopeErrorwhen called within anexample.
This patch overrides
nameto return the example name instead.