Adds RSpec guard against truncating prod db #601
Conversation
| @@ -1,6 +1,7 @@ | |||
| ENV["RAILS_ENV"] = "test" | |||
|
|
|||
| require File.expand_path("../../config/environment", __FILE__) | |||
| abort("The Rails environment is running in production mode!") if Rails.env.production? | |||
|
Can this ever happen, given we set in line 1 |
|
Ah, that is a good point since Suspenders differs slightly from the stock rspec-rails template in that it doesn't memoize the environment, it always sets it. I missed that point initially and need to run some checks to ensure that it is in fact sufficient. I'm pretty sure I've ran into issues before due to 'rake' and loading of envs. I'll report back as soon as I can. Feel free to close this meanwhile :) |
|
I think we do still have to worry about this. I think Rails will override the environment-specific configuration from |
👍 👏 @pedrosmmoreira, can you add this piece of information to the commit message? Then I'll merge. Thank you! |
|
@tute Sure, coming right up :) |
pedrosmmoreira
force-pushed
the
port_rspec_rails_guard_for_running_against_a_production_db
branch
2 times, most recently
from
d3bf022
to
78e6b60
Compare
I agree we need to worry about it, but this change doesn't do anything to help us with that, as best I can tell. It's not checking the database it's running against, it's checking the environment. In the case where you have copied down config you can still be running in development mode, just with a database url that points to production. |
|
@derekprior I see your point. Do you have any suggestions on how to proceed? One possible solution would be running the guard against the database host? |
|
Rails 5 is going to address this issue, but in the meantime we could raise if DATABASE_URL is set at all. |
I am in favor of this. |
|
@tute @derekprior Cool, I'll rebase and update as soon as possible then. |
|
Ping @pedrosmmoreira. Thank you! |
pedrosmmoreira
force-pushed
the
port_rspec_rails_guard_for_running_against_a_production_db
branch
from
78e6b60
to
d5fbeb3
Compare
| if ENV["DATABASE_URL"] | ||
| abort( | ||
| "A database url has been detected as set. " \ | ||
| "Please ensure that you are not connecting to a production database." |
There was a problem hiding this comment.
Put a comma after the last parameter of a multiline method call.
|
@tute Sorry, I was away on holidays. All updated now, let me know if you'd like to consider a different error message! |
| @@ -1,6 +1,12 @@ | |||
| ENV["RAILS_ENV"] = "test" | |||
|
|
|||
| require File.expand_path("../../config/environment", __FILE__) | |||
| if ENV["DATABASE_URL"] | |||
| abort( | |||
| "A database url has been detected as set. " \ | |||
There was a problem hiding this comment.
I'd say here a more concrete "DATABASE_URL environmental variable is set.", given it can be transparent to the user.
|
Left a small comment, ready after we address to rebase and merge. Thank you! |
RSpec-rails introduced a guard against running specs in production environment, this was introduced in version 3.3.3, in PR rspec/rspec-rails@b913f6a. Rails will override the environment-specific configuration from database.yml with DATABASE_URL if it's present, so accidentally pulling configuration from the Heroku environment is still a risk. The guard introduced in this PR checks if DATABASE_URL is set and raises an error.
pedrosmmoreira
force-pushed
the
port_rspec_rails_guard_for_running_against_a_production_db
branch
from
d5fbeb3
to
2d8c0b9
Compare
|
Should be good to go now |
|
Thank you! |
RSpec-rails introduced a guard against running specs in production environment, this was introduced in version 3.3.3, in PR rspec/rspec-rails@b913f6a.
Rails will override the environment-specific configuration from
database.ymlwithDATABASE_URLif it's present, so accidentally pulling configuration from the Heroku environment is still a risk.The guard introduced in this PR prevents the test suite from running against a production db and causing a loss of data.