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 RAILS_ENV to test when executing the default rake task #405

Merged
merged 1 commit into from Jul 11, 2020

Conversation

dsander
Copy link
Contributor

@dsander dsander commented Jan 3, 2020

This is a followup of #241 and brings the dotenv-rails environment
assigning hack in line with the logic rails uses when the
test_unit/railtie is loaded
.

This is a breaking change for all non test-unit users, but brings the
behavior of dotenv-rails in line across all uses of rake test, rake rspec, rspec and rake.

Before this change dotenv-rails loaded different env files when using
a rails application with rspec for tests. rake spec was covered by
the previous logic and set the RAILS_ENV to test. Only running rake
(which invokes rspec by default) wasn't covered and thus loaded first
the development env files and later the test env.

This is a followup of bkeepers#241 and brings the `dotenv-rails` environment
assigning hack in line with the [logic rails uses when the
`test_unit/railtie` is loaded][1].

This is a breaking change for all non `test-unit` users, but brings the
behavior of `dotenv-rails` in line across all uses of `rake test`, `rake
rspec`, `rspec` and `rake`.

Before this change `dotenv-rails` loaded different env files when using
a rails application with `rspec` for tests. `rake spec` was covered by
the previous logic and set the RAILS_ENV to test. Only running `rake`
(which invokes rspec by default) wasn't covered and thus loaded first
the development env files and later the test env.

[1]: https://github.com/rails/rails/blob/6-0-stable/railties/lib/rails/test_unit/railtie.rb#L5-L7
lawrence-forooghian added a commit to DFE-Digital/claim-additional-payments-for-teaching that referenced this pull request Jan 23, 2020
We use the default Rake task as our standard invocation for running the
test suite and other code quality checks. We noticed that running
`bundle exec rake` was causing dotenv to use the ENVIRONMENT_NAME
variable from the .env.development file, and not .env.test as expected.
A recent change in 1d57789 to the values of ENVIRONMENT_NAME in
.env.development hence caused the tests to start failing when run
locally.

We found out that other people have also been experiencing this issue
[1]. The linked comment explains in detail what causes this to happen:

> Actually, I found that it will load `.env.test`, but because `.env.development` was already loaded before `.env.test`, and since `Dotenv.load` won't override the env var if it exist, it will use var defined in `.env.development` not `.env.test`.
>
> The whole process is like this:
>
> 1. `rake spec` call Rakefile in root folder, which will
>    `require File.expand_path('../config/application', __FILE__)`
> 2. application will trigger [`before_configuration` hook defined by dotenv](https://github.com/bkeepers/dotenv/blob/master/lib/dotenv/rails.rb#L20), which will load following 3 env files:
>
>    * `.env.local`
>    * `.env.development`
>    * `.env`
> 3. spec rake task get call, and call this code in your spec:
>    ```
>    require 'rails_helper'
>    ```
> 4. In rails_helper.rb, ENV['RAILS_ENV'] was set to 'test'.
> 5. Then, config/application.rb was required again by `config/environment`, and `before_configuration` hook was triggered, only now `Rails.env == 'test'`. Therefore, it will load following 3 files:
>
>    * `.env.local`
>    * `.env.test`
>    * `.env`
>
> But since `before_configuration` hook is using `load`, it won't override existing value.

The gist is that dotenv ends up loading the .env.development file
first, and then loads the .env.test file. However, by default dotenv
will not overwrite the variables defined in the first file with those
defined in the second, so we end up with the incorrect ENVIRONMENT_NAME
being used.

The same comment suggested adding `Dotenv.overload('.env.test')` to
`spec/rails_helper.rb`. This wasn't sufficient for us, since by the time
this happens, our config/initializers/dfe_sign_in.rb had already grabbed
the (wrong) values from the environment.

Anyway, ideally, we don't want to load the .env.development file _at
all_ when running the tests, so I didn't try to figure out how to make
the previous approach work. Instead, we noticed that dotenv contains a
special hack, which sets RAILS_ENV to `test` when the currently-running
Rake task is `spec` [2]. There is currently an open pull request on
dotenv which does the same thing when running the default Rake task [3],
but it doesn't seem to have attracted any attention.

In the end I decided to emulate the dotenv hack, by setting RAILS_ENV to
`test` if running the default Rake task. I do this in the Rakefile,
before any of the Rails application is loaded, to ensure that setting
this environment variable happens before dotenv has a chance to load
.development.env.

[1] bkeepers/dotenv#219 (comment)
[2] bkeepers/dotenv#241
[3] bkeepers/dotenv#405
@stale
Copy link

stale bot commented Mar 3, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Mar 3, 2020
@dsander dsander closed this Mar 3, 2020
@dsander dsander reopened this Mar 3, 2020
@stale stale bot removed the wontfix label Mar 3, 2020
@stale
Copy link

stale bot commented May 2, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label May 2, 2020
@dsander dsander closed this May 2, 2020
@dsander dsander reopened this May 2, 2020
@stale stale bot removed the wontfix label May 2, 2020
@stale
Copy link

stale bot commented Jul 3, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jul 3, 2020
@dsander dsander closed this Jul 5, 2020
@dsander dsander reopened this Jul 5, 2020
@stale stale bot removed the wontfix label Jul 5, 2020
@cadwallion cadwallion self-assigned this Jul 11, 2020
Copy link
Collaborator

@cadwallion cadwallion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Thanks for the contribution, and apologies that this took so long to review!

@cadwallion cadwallion merged commit 08f2214 into bkeepers:master Jul 11, 2020
@nertzy
Copy link

nertzy commented Jul 13, 2020

This is definitely a breaking change for non-test-unit projects, like you mentioned.

I see gems from my development group getting loaded into my test Rails environment while running rake tasks, so something seems broken here.

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

3 participants