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 with_routing not working with get :index #1777

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sclinede
Copy link

As guys previously mentioned in the thread the problem is that #get is called on other context then #with_routing.

It is caused by https://github.com/rspec/rspec-rails/blob/master/lib/rspec/rails/example/controller_example_group.rb#L5.
ActionDispatch::Assertions::RoutingAssertions is adding #with_routing to an assertion_instance, and ActionController::TestCase::Behavior is adding #get to ControllerExampleGroup.
But they are included to different classes and do not affect each other.

So I decided to add an ControllerAssertionDelegator which will include both of them. Actually AssertionDelegator missed some methods, so I featured them in that delegator.

Closes #1652

@sclinede
Copy link
Author

sclinede commented Jan 20, 2017

Hi penople. This is my first try for rspec and also just a working draft rather then full solution.
Just want to be sure that I'm on right track)

@sclinede sclinede changed the title with_routing not working with get :index Fix with_routing not working with get :index Jan 20, 2017
@sclinede
Copy link
Author

sclinede commented Jan 20, 2017

On the other hand we can do it like that https://gist.github.com/sclinede/3dc0565e5ef9cb9d99bbcf6719302c5d#file-controller_example_group-rb-L4-L18
I mean move ActionDispatch::Assertions::RoutingAssertions to ControllerExampleGroup

@pirj
Copy link
Member

pirj commented Jan 8, 2020

@sclinede Can you please add a test that would fail without the patch?

@sclinede
Copy link
Author

sclinede commented Jan 9, 2020

@pirj You could find failing spec in an additional commit a9adc65

@sclinede
Copy link
Author

sclinede commented Jan 9, 2020

The solution was tested against https://github.com/samphippen/test_issue_817 provided by @penelopezone

As guys previously mentioned in the thread the problem is that `#get` is called on
other context then `#with_routing`.

It is caused by
https://github.com/rspec/rspec-rails/blob/master/lib/rspec/rails/example/controller_example_group.rb#L5.
ActionDispatch::Assertions::RoutingAssertions is adding #with_routing to
an assertion_instance, and ActionController::TestCase::Behavior is
adding #get to ControllerExampleGroup.

So I decided to add an ControllerAssertionDelegator which will include
both of them. Actually AssertionDelegator missed some methods, so I
featured them in that delegator.

Closes rspec#1652
@sclinede
Copy link
Author

sclinede commented Jan 9, 2020

I don't get it. The test suit is failing around teardown method somewhere deep. If you have any suggestions I'll appreciate.
Meanwhile, trying to figure it out by myself...
@pirj

def setup(*methods, &block)
@setup_methods += methods
@setup_blocks << block if block
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may have found it yourself already, but anyway - if you add a def teardown(*); end here, the spec starts failing with a different error.

@sclinede
Copy link
Author

So... it looks like I need more time to invest in that change. Hope I'll find it soon :)
I don't give up

@pirj
Copy link
Member

pirj commented Mar 13, 2020

@sclinede Have you had a chance to take a closer look at teardown problem?

@JonRowe JonRowe changed the base branch from master to main August 2, 2020 02:13
@JonRowe JonRowe mentioned this pull request Jan 18, 2022
21 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants