Defaults for Rails usage too opinionated

Question

Defaults for Rails usage too opinionated

Closed this issue 6 months ago · 7 comments

This great little tool has been extremely helpful in flushing out flakey tests. My only feedback is that the default run command for a Rails app is far too opinionated. Why would you assume that the user wouldn't have their test DB fully set up and migrated? The first time I tried it, I hadn't read through the entire readme, just the Usage section before trying it, and it completely nuked my setup.

I would consider leaving the default, bare-bones configuration regardless if a Rails app or not and letting the user decide if a custom run command is necessary.

Answer 1 · 2022-09-09T20:51:41.000Z

Thanks for the feedback. Is this the command that ran when you noticed the trouble?

DISABLE_DATABASE_ENVIRONMENT_CHECK=1 RAILS_ENV=test bundle exec rake db:drop db:create db:migrate && bundle exec rspec --order rand

Also, could you give some more details on why that caused problems? (rspec_n is trying to reset the test database to an empty state before each run. There could be tests that leave data in the database. The goal is to help each run stay independent by ensuring the database is in the same state at the start of each run).

Answer 2 · 2022-09-09T21:08:53.000Z

Migrate does not work on most large established rails apps as it is very common that there are things in earlier migrations that would no longer be supported currently in the app. In my instance, some data migration operations are acting on deprecated model attributes. Honestly, if you are going to do that, db:schema:load is the correct command. The second, and frankly larger issue, is the opinion that DISABLE_DATABASE_ENVIRONMENT_CHECK=1 is wanted or needed by the user. In the case of my app, the DB environment check is extremely important, and this is the thing that caused me so much trouble.

As I stated, the crux of the issue is this tool is making assumptions that it has no business making.

rspec_n is trying to reset the test database to an empty state before each run. There could be tests that leave data in the database. The goal is to help each run stay independent by ensuring the database is in the same state at the start of each run

To start with, clearing the DB is already built into Rspec. Additionally, why does this tool care? How does this change how this tool functions?

Answer 3 · 2022-09-13T08:18:19.000Z

Hey @tagCincy

I had a very similar issue and after digging around I found that the -c flag on the command line worked for me.

I ran something like rspec_n PATH_TO_FILE --command="bundle exec rspec" and it worked

Answer 4 · 2022-09-13T12:05:34.000Z

@manuelfcreis Yes, and that's how I ultimately used this tool effectively. This point is that the tool should default to the bare bones run command, and the -c should be used for those with additional runtime dependencies.

Answer 5 · 2022-10-16T03:22:41.000Z

@tagCincy Do you recall why DISABLE_DATABASE_ENVIRONMENT_CHECK=1 caused problems in your case? The gem is built around the idea that the test database can be dropped and reset between runs. Since the default command passes RAILS_ENV=test, the database environment check seemed unnecessary.

For the backstory, it was added because I've run into situations where Rails would refuse to drop the test database (Rails thought it was operating on the production environment). It looked like a bug in Rails. This was a workaround to guarantee the database could always be dropped. Perhaps that issue has been resolved, or passing RAILS_ENV=test is sufficient. I'll do some testing and see if the flag can be removed from the default command.

Answer 6 · 2024-01-27T16:25:12.000Z

+1 to everything that was said

running migrations will certainly crash (sooner or later).
The suggestion --command="bundle exec rspec" worked for me too

Answer 7 · 2024-03-09T04:25:28.000Z

Here's the current state:

Based on the above feedback, I merged PR #118 which makes rspec_n use db:schema:load to prepare the database (instead of db:migrate) and released v2.0.0.
I added a Considerations section to the README to highlight some of the potential pitfalls that were raised here.
At this time, I plan on keeping the code that makes rspec_n rebuild the database before each run. After some testing, I don't think that the comment above, which mentioned clearing the DB is already built into Rspec would work in the way rspec_n needs. To test, I created a new test database in an app and filled it with seeded data. When I start rspec, the seeded data is still present. When the test suite finishes, the seeded data is also still present. rspec_n assumes an empty database, and tries to maintain an empty database at the start of each run, so dropping, creating and loading the schema/structure is an easy way to achieve that. Other preparation strategies could be supported if someone has the time to work on it, and I'm happy to take another look if someone has more info on how rspec could handle the database clearing.
Since the database will continue to be dropped and rebuilt before each run (for now), it's helpful to keep DISABLE_DATABASE_ENVIRONMENT_CHECK=1 to ensure the database can be dropped. I don't know why Rails refuses to drop the test database at times, but I ran into it quite often at one point, so others may run into that problem if they have the right versions of Rails, Postgres, MySQL, etc...

I'm going to close this issue since I don't plan on any further action. If anyone has additional suggestions or pain points, feel free to add them to this thread, or create a new dedicated issue.