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

Ensure that the default Rake task uses the test environment by default #810

Merged
merged 1 commit into from
Jan 23, 2020

Conversation

lawrence-forooghian
Copy link
Contributor

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, 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

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
@tekin tekin temporarily deployed to acceptance-t-fix-defaul-m4dsig January 23, 2020 16:08 Inactive
@tekin
Copy link
Contributor

tekin commented Jan 23, 2020

Nice work on the thorough research and the detailed write up in the commit message 💯

@lawrence-forooghian lawrence-forooghian merged commit c15f97e into master Jan 23, 2020
@lawrence-forooghian lawrence-forooghian deleted the fix-default-rake-task-environment branch January 23, 2020 16:24
@pezholio
Copy link
Contributor

Yeah, I think this is the best solution. I've fiddled around with some other options, but I think this is the best way round it. Nice work! 👍

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.

3 participants