Skip to content
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

Fix anonymous controller route helpers #905

Closed
wants to merge 2 commits into from
Closed

Conversation

@thomasfedb
Copy link
Contributor

thomasfedb commented Jan 3, 2014

Continuing on from #881.

Changed orig_routes to @orig_routes as it is referred to in method_missing on line 138. Bug introduced in 86aea05. Added a test that fails when fix is reverted.

@thomasfedb
Copy link
Contributor Author

thomasfedb commented Jan 3, 2014

Travis is failing with:

/home/travis/build/rspec/rspec-rails/lib/rspec/rails/matchers/relation_match_array.rb:2:
in `<top (required)>': uninitialized constant RSpec::Matchers::BuiltIn::MatchArray (NameError)

Which leaves me baffled, as I didn't touch that file or anything I can think would affect it.

@myronmarston
Copy link
Member

myronmarston commented Jan 3, 2014

In rspec-expectations I recently renamed that class. See rspec/rspec-expectations@6bde36e and rspec/rspec-expectations#398.

Use RSpec::Matchers::BuiltIn::ContainExactly instead.

@thomasfedb
Copy link
Contributor Author

thomasfedb commented Jan 3, 2014

@myronmarston Okay, thanks.

I guess that should be a separate PR however.
Would you be prepared to do so? Or shall I?

@myronmarston
Copy link
Member

myronmarston commented Jan 3, 2014

Would you be prepared to do so? Or shall I?

Have at it!

@thomasfedb
Copy link
Contributor Author

thomasfedb commented Jan 3, 2014

@myronmarston The new ContainExactly doesn't recognise ActiveRecord::Relation objects as being enumerable, for the obvious reason that ActiveRecord::Relation.new.ancestors doesn't include Enumerable.

I could either monkeypatch RSpec::Matchers::Composable and change the enumerable? method in the rspec-rails gem, or perhaps you could check for #to_a in the rspec-expectations gem.

This was introduced in rspec/rspec-expectations@ec93540.

@myronmarston
Copy link
Member

myronmarston commented Jan 3, 2014

I've got a fix in rspec/rspec-expectations#404.

@myronmarston
Copy link
Member

myronmarston commented Jan 3, 2014

I just merged that PR so you should be good to go now :). Sorry about the regression!

@thomasfedb
Copy link
Contributor Author

thomasfedb commented Jan 3, 2014

Cheers @myronmarston.

…n an

anonymous controller has been defined.
@thomasfedb
Copy link
Contributor Author

thomasfedb commented Jan 4, 2014

Just squashed my tests into the same commit for neatness, and to trigger Travis.

@thomasfedb
Copy link
Contributor Author

thomasfedb commented Jan 4, 2014

Travis is failing on one spec that appears unrelated, as mentioned in #906 by @alindeman.

@alindeman alindeman closed this in b731d91 Jan 4, 2014
@alindeman
Copy link
Contributor

alindeman commented Jan 4, 2014

For now, I decided simply to partially revert the commit. I'd love to have it covered by a spec, but I think we need a common way to run examples (extracted from -core?) first.

@thomasfedb
Copy link
Contributor Author

thomasfedb commented Jan 4, 2014

Consider using the spec attached to this issue once you're ready.

@myronmarston
Copy link
Member

myronmarston commented Jan 5, 2014

I think we need a common way to run examples (extracted from -core?) first.

rspec-support could be a good home for the supporting code. I'm working on improving rspec-mocks so that no monkey patching is needed to get it to work in that fashion.

@thomasfedb
Copy link
Contributor Author

thomasfedb commented Jan 5, 2014

@myronmarston I did notice that the spec was a little tricky to write.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.