-
-
Notifications
You must be signed in to change notification settings - Fork 327
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
Rubocop legacy fix: Erroneous Auto Inheritance of ActionDispatch::IntegrationTest in the World #505
Comments
I've been using the fact that World inherits from ActionDispatch::IntegrationTest in a patch that actually enables proper Rails transaction-mode database cleanup with no database_cleaner required at all. Been using this patch for more than a year with no issues. I was going to make a PR to upstream the patch into cucumber-rails itself. If you no longer inherit from ActionDispatch::IntegrationTest, Rails transaction-mode database cleanup won't work. Just saying: It's not a bug that World inherits from ActionDispatch::IntegrationTest. It's a bug that it doesn't take advantage of the lifecycle code available in that class. |
Thanks for the heads up. It's likely we need to fix this in a breaking change release. The reason I made this ticket was because we can streamline what rails should be and the inheritance (at that point), was only to get some test helpers (nothing more). Maybe in doing so we find that we need more of the inheritance chain. I don't know. But definitely good you've posted this so I can be careful as/when I tackle this. |
@BrianHawley - As a heads up. I'm going to try a small tweak, to just ensure that the class is setup correctly by calling Now this "could" be potentially problematic. So for now I'm just going to push the branch up and invite a few people to play with it. EDIT: branch ref is |
As an update this branch is merged into I dug into some of the code and found the initializer was a "hack" set up 13 years ago. With the comment "Remove this". So after checking and removing it. I found that 4 levels deep the This obviously doesn't fix the issue of inheriting from it, but when I removed that I found a whole word of hurt happened including things like standard rails methods ( As such I'll keep this issue open for a couple of weeks, but then I'll close it off as no longer relevant. |
Closing as no longer relevant. cc/ @BrianHawley |
Summary
We inherit from ActionDispatch::IntegrationTest which inherits from about 4 other things down the chain. We need to try remedy this and only inherit what we need (Which are the test helpers).
The text was updated successfully, but these errors were encountered: