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

Flipper::Adapters::Strict::NotFound raised when checking non-existant feature flag during rspec test #849

Closed
dannyy83 opened this issue Feb 19, 2024 · 9 comments

Comments

@dannyy83
Copy link

dannyy83 commented Feb 19, 2024

The following error is being raised when a class under test is checking if a feature toggle exists:

Flipper::Adapters::Strict::NotFound: Could not find feature "some_feature". Call `Flipper.add("some_feature")` to create it.

I believe this is being caused by a bug in the flipper_configure method within test_help.rb:

def flipper_configure
  # Use a shared Memory adapter for all tests. This is instantiated outside of the
  # `configure` block so the same instance is returned in new threads.
  adapter = Flipper::Adapters::Memory.new

  Flipper.configure do |config|
    config.adapter { adapter }
    config.default { Flipper.new(config.adapter) }
  end
end

The code above should not be calling config.adapter when initializing Flipper. Instead, it should be:

def flipper_configure
  # Use a shared Memory adapter for all tests. This is instantiated outside of the
  # `configure` block so the same instance is returned in new threads.
  adapter = Flipper::Adapters::Memory.new

  Flipper.configure do |config|
    config.adapter { adapter }
    config.default { Flipper.new(adapter) }
  end
end

Is there any work around to prevent test_help.rb from being loaded, so that I can configure flipper myself, to avoid this? This is preventing me from upgrading to flipper v1.2

@bkeepers
Copy link
Collaborator

Hi @dannyy83, you're actually running into an issue with the strict adapter, introduced in #763. I'm guessing you're running Flipper 1.1? In 1.2 we disabled the strict adapter in tests because people were running into this issue. So you can either upgrade to 1.2 or manually disable it per the directions in the 1.1 release notes (see https://github.com/flippercloud/flipper/releases).

Let me know if that doesn't fix it for you.

@dannyy83
Copy link
Author

Hi @bkeepers, I'm seeing this with the following environment:
flipper v1.2.2
flipper-redis v1.2.2
rails v7.1.3
rspec v3.13.0
ruby v3.3.0

My configuration for flipper is:

require 'flipper'
require 'flipper/adapters/redis'

Flipper.configure do |config|
  config.default do
    client = Redis.new(url: 'some_url')
    adapter = Flipper::Adapters::Redis.new(client)
    Flipper.new(adapter)
  end
end

I have no explicit flipper configuration in rspec, except for what's being automatically pulled in from https://github.com/flippercloud/flipper/blob/main/lib/flipper/test_help.rb

@dannyy83
Copy link
Author

@bkeepers I've also tried to configure flipper as follows (as here https://www.flippercloud.io/docs/adapters/redis)

require 'flipper/adapters/redis'

And I still have the same issue.

@dannyy83
Copy link
Author

I've also tried to disable the strict adapter as suggested:

Rails.application.configure do
  config.flipper.strict = false
end

And this has no effect.

@dannyy83
Copy link
Author

Also, here is the stack trace of the error being raised:

     # /usr/local/bundle/gems/flipper-1.2.2/lib/flipper/adapters/strict.rb:44:in `assert_feature_exists'
     # /usr/local/bundle/gems/flipper-1.2.2/lib/flipper/adapters/strict.rb:24:in `get'
     # /usr/local/bundle/gems/flipper-1.2.2/lib/flipper/adapters/memoizable.rb:58:in `get'
     # /usr/local/bundle/gems/flipper-1.2.2/lib/flipper/feature.rb:279:in `gate_values'
     # /usr/local/bundle/gems/flipper-1.2.2/lib/flipper/feature.rb:110:in `block in enabled?'
     # /usr/local/bundle/gems/flipper-1.2.2/lib/flipper/feature.rb:436:in `block in instrument'
     # /usr/local/bundle/gems/flipper-1.2.2/lib/flipper/instrumenters/noop.rb:5:in `instrument'
     # /usr/local/bundle/gems/flipper-1.2.2/lib/flipper/feature.rb:433:in `instrument'
     # /usr/local/bundle/gems/flipper-1.2.2/lib/flipper/feature.rb:107:in `enabled?'
     # /usr/local/bundle/gems/flipper-1.2.2/lib/flipper/dsl.rb:36:in `enabled?'

@dannyy83
Copy link
Author

dannyy83 commented Feb 21, 2024

Hi @bkeepers. I've done some more investigation on this, and tracked down the root cause of this problem to https://github.com/flippercloud/flipper/blob/main/lib/flipper/engine.rb#L71

I'm not sure why, but the call to flipper.strict returns the ActiveSupport::OrderedOptions instance, instead of the value of the :strict key (which is false). So, the Strict adapter is being used.

To be more explicit, If I set a breakpoint on the above line, you can see:

> flipper.strict
=> {:env_key=>"flipper", :memoize=>true, :preload=>true, :instrumenter=>ActiveSupport::Notifications, :log=>true, :cloud_path=>"_flipper", :strict=>false, :test_help=>true}

> flipper.strict.class
=> ActiveSupport::OrderedOptions

# But, interestingly
> flipper.dig(:strict)
=> false

@dannyy83
Copy link
Author

I've finally tracked down this problem...
In our test env, we also use the json_expressions gem (https://github.com/chancancode/json_expressions).
This gem adds a strict method to Hash and Array which returns a clone and unfortunately this method is called when flipper is initializing in our test env, which is truthy, causing strict mode to be enabled.

@bkeepers
Copy link
Collaborator

@dannyy83 wow, that's a fun one. I'm glad you figured it out! I'm not sure there's much we can do in flipper to work around that, but I'm open to suggestions if you have ideas.

@dannyy83
Copy link
Author

dannyy83 commented Feb 21, 2024

Hi @bkeepers, I agree there's not much we can do for flipper to work around this issue. I don't have any suggestions.

For others that may see this problem, I worked around this issue by setting require: false in the Gemfile for json_expressions:

group :test do
  gem 'json_expressions', require: false
end

And instead manually load it in the rails_helper.rb AFTER rails is loaded:

require 'json_expressions/rspec'

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

No branches or pull requests

2 participants