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

Set attempted_path in warden.options before calling failure app in controller test helpers #5768

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cgunther
Copy link

@cgunther cgunther commented Mar 1, 2025

In controller tests, the failure app seems to be called directly rather than going through Warden. As such, it duplicates some of the set up Warden does before calling the failure app, however didn't set attempted_path, like Warden does. Therefore, if your custom failure app relied on attempted_path, it wouldn't be set in your controller tests, leading to different behavior in tests versus production.

This now brings the test helper closer in-line with the Warden implementation.

https://github.com/wardencommunity/warden/blob/67f5ba6baaa7564ec79afef02cf3a4d0f7d312e5/lib/warden/manager.rb#L138-L143

This is similar in nature to #3968, but includes a test, though in my case, my issue wasn't related to relative_url_root, so I can't speak to that side of the original issue.

…n controller test helpers

In controller tests, the failure app seems to be called directly rather
than going through Warden. As such, it duplicates some of the set up
Warden does before calling the failure app, however didn't set
`attempted_path`, like Warden does. Therefore, if your custom failure
app relied on `attempted_path`, it wouldn't be set in your controller
tests, leading to different behavior in tests versus production.

This now brings the test helper closer in-line with the Warden
implementation.

https://github.com/wardencommunity/warden/blob/67f5ba6baaa7564ec79afef02cf3a4d0f7d312e5/lib/warden/manager.rb#L138-L143
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant