Add flipper/test_help to automatically configure Flipper in tests/specs.

[bkeepers]

This adds flipper/test_help to automatically configure Rails/RSpec the way we recommend in our testing docs.

Open questions:

1. Should Flipper::Engine automatically require this if RAILS_ENV=test? I think yes, but maybe we wait until 2.0 to do that?
2. As brought up in How to handle strict adapter with test? #807, what should the default value for strict be in test? Currently with strict = :warn, you also have to call Flipper.add for each feature. In theory, I like the explicitness of this. It will guard against typos in flag names, but also seems like busywork. One option is to establish a pattern for defining features in an initializer, like this:

# config/initializers/flipper.rb
module Feature
  Hubspot = Flipper.add(:hubspot)
  Seo = Flipper.add(:seo)
end

# app/jobs/hubspot_sync_job.rb
def perform(resource)
  return unless Feature::Hubspot.enabled?(resource)
end

[jnunemaker]

Looks good as always. Added some questions but I'm not sure on their answers.

I thought about mentioning Minitest but then I couldn't think of a scenario where it would be used.

Should Flipper::Engine automatically require this if RAILS_ENV=test? I think yes, but maybe we wait until 2.0 to do that?

Automatic seems good. We could add an env var or config to disable automatic requiring.

Why wait until 2.0? Does it seem like a breaking change? I wonder if it would be ok. If anyone has these things configured in rspec or active support test case, those configurations would overwrite these, wouldn't they?

As brought up in #807, what should the default value for strict be in test? Currently with strict = :warn, you also have to call Flipper.add for each feature. In theory, I like the explicitness of this. It will guard against typos in flag names, but also seems like busywork.

I can see :raise as default. In theory, I like the explicitness of that too.

One option is to establish a pattern for defining features in an initializer, like this:

I also like establishing a pattern for defining features. I'd probably do it through config, as you mentioned in chat (but I'll copy it in here for everyone else).

Flipper.configure do |config|
  config.add :a, :b, :c
end

The only downside of this is that it requires network calls on boot, which I'm usually anti. Or maybe we can avoid that by only doing the network calls if strict is turned on in some fashion? That would limit it to only in dev/test. Thoughts?

[davidwessman]

Went down a nice little rabbit hole while investigating our Flipper-test-setup, guess I am waiting for the next release!
Well done 🚀

[fcheung]

I got some spec failures after this - you get a new flipper instance but using the same memory adapter, so there's leakage between tests.

I can make everything green again if I add Flipper.instance.import(Flipper::Adapters::Memory.new) to the reset (which was the simplest way I could find to delete all the data from the shared adapter)

[rnestler]

I think I hit the same. Maybe it would be better to just disable all feature flags in reset? Ah no, for me it just doesn't work in system specs with the memory adapter. The feature flag just doesn't get set.

[fcheung]

Yes, I hit this. The app thread and spec thread end up using a different flipper instance (fine). The app thread is running for the entire duration of the specs so keeps the same flipper instance.

On the second spec the before each creates a new flipper instance but this only affects the spec thread, the app thread still has the old memory adapter.

To make this work I had to disable the builtin flipper/test_help (with the env var) and then do what I was doing before: create the shared memory adapter once in before(suite) and clear out its contents in before each