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

Protect development database from tests when RAILS_ENV is set #60

Conversation

MarkDBlackwell
Copy link
Contributor

No description provided.

@akshatpradhan
Copy link
Owner

I'm not sure I understand why this is here.

@MarkDBlackwell
Copy link
Contributor Author

Please see Rails issue 7175 for a discussion of the problem.

# It's dangerous to use `||=' when setting ENV['RAILS_ENV'] here; see
# https://github.com/rails/rails/issues/7175
ENV["RAILS_ENV"] = 'test'

If by your question you mean: why did I change it to '=':

Because in some cases and with some software, the environment variable RAILS_ENV may be predefined with some value. Then, using '||=', running the tests will fail to change that value to 'test', because it already exists. This actually forces the initial database-dropping step to destroy the database of whatever environment RAILS_ENV was predefined to. This may be 'development' or 'production'. It normally makes a fresh, empty 'test' environment database during testing. I have experienced this loss, BTW.

If by your question you mean: why do they have it as '||=':

It could be motivated by continuous integration servers running tests in a special Rails environment. My source for the possibility is this comment. However, for continuous integration, setting RAILS_ENV externally is unnecessary. Or if desired, the line can be changed to check specifically for the value 'CI'.

Dave Chemlinsky has explained he didn't want "to force it to 'test' because that ties everybody's hands." However, if we ourselves want to be sure it is 'test', that won't hurt anyone.

Never when running specs (which require spec/spec_helper.rb) would one want to use any Rails environment other than 'test', right? So, we might as well set it definitely to 'test'.

If you don't mind the slight frustration under certain circumstances of losing your own development database, it's okay with me if you want to leave this alone in the project. :)

@akshatpradhan
Copy link
Owner

i'm going to put this onto milestone2, but I will keep the pull request open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants