rspec / rspec-core

RSpec runner and formatters
http://rspec.info
MIT License
1.22k stars 763 forks source link

RSpec+Figaro causes leaking of dev env variables to test env #2631

Closed dzirtusss closed 8 months ago

dzirtusss commented 5 years ago

Subject of the issue

When using RSpec with Figaro (maybe other similar gems as well) following happen:

  1. Figaro has env config as following:

    COMMON_VAR: "value"
    development:
    DEV_ONLY_VAR: "value"
  2. when rake spec, env is started in development and DEV_ONLY_VAR got populated and joined with ENV

  3. later rake spec spawns a testing process here with simple system(...) call, which causes DEV_ONLY_VAR to leak to test env as initial ENV var.

This was discussed in here and in here but no actual fix was provided mostly because problem originates from rspec's rake task.

IMO, system(...) should be replaced with Bundler.clean_system(...) which will execute process with initial shell env (before dev env changes applied).

Currently, I have a personal patch as following (which does the same but with monkey-patching):

# lib/tasks/rspec.rake
if defined?(RSpec)
  module RSpecRakeTaskExtensions
    def run_task(*)
      Bundler.with_clean_env { super }
    end
  end

   module RSpec
    module Core
      class RakeTask
        prepend RSpecRakeTaskExtensions
      end
    end
  end
end

If this seems to be a desired default behavior, I guess task logic should be updated. Alternatively, it will be ok to have this as a configurable option.

Your environment

Steps to reproduce

described in subject

Expected behavior

starts system in rake task with clean env

Actual behavior

starts system in rake task with dev env

JonRowe commented 5 years ago

This is a tricky one, in some use cases you want the environment to be populated through to your test suite, it really should be Figaro's job to clean the environment at the right point.

We even use certain environment variables to control how RSpec works so I certainly can't support making this a default!

We also cannot use Bundler.with_clean_env as we do not depend on Bundler, and it might not be present. However I've just added a flag in #2632 which allows using Ruby's own "clean" env support where required, fancy taking a look?

gaffneyc commented 8 months ago

I just ran into this very old issue today and was able to come up with a straight forward fix for anyone else who happens to run into it.

You can clear out ENV variables in Figaro by adding an empty value to the test environment. In our case we were expecting the ENV variable to be empty so adding it explicitly to the test environment was also a win for clarity.

# config/application.yml
development:
  SOME_KEY: dev

test:
  SOME_KEY:
Figaro.env.some_key # => nil
JonRowe commented 8 months ago

As indicated we released #2632 to allow handling situations like this, using a clean environment in the rake task as an opt in, so I'm closing this very old issue 😂