From 446e74ca83160da4d0aff5efbce8842acc42ea12 Mon Sep 17 00:00:00 2001
From: Jeremy Friesen <jeremy.n.friesen@gmail.com>
Date: Mon, 7 Feb 2022 08:53:30 -0500
Subject: [PATCH] Documenting and addl testing FeatureFlag.enabled? (#16420)

Prior to this commit, we had a single test for `FeatureFlag.enabled?`,
namely that it delegates to `Flipper.enabled?`.  This commit adds some
tests to both verify behavior and communicate to developers the
intentions of the related methods:

- `FeatureFlag.enabled?`
- `FeatureFlag.accessible?`

My hope is the documentation and tests will help address the horrors
mentioned in conversation in #16416:

> I have lived the horrors of having a feature flag accidently
> flipped or incorrectly setup in the first place and half baked
> functionality going live.

Note: I'm not proposing that we test other `Flipper` methods, but
instead to ensure that we're testing and documenting the behavior of
what I believe to be the two primary mechanisms of "putting something
behind a feature flag."

Related to #16406, #16416
---
 app/services/feature_flag.rb       | 19 ++++++++++++++++++-
 spec/services/feature_flag_spec.rb | 26 +++++++++++++++++++++++---
 2 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/app/services/feature_flag.rb b/app/services/feature_flag.rb
index bf4103ee7..0b0826b27 100644
--- a/app/services/feature_flag.rb
+++ b/app/services/feature_flag.rb
@@ -5,7 +5,22 @@ module FeatureFlag
   class << self
     delegate :add, :disable, :enable, :enabled?, :exist?, :remove, to: Flipper
 
-    # Unless the given :feature_flag_name is _explicitly_ disabled,
+    # @!method FeatureFlag.enabled?(feature_flag_name, *args)
+    #
+    #   Answers if the :feature_flag_name has been _explicitly_ **enabled**.
+    #
+    #   @param feature_flag_name [Symbol]
+    #   @param args [Array] passed to Flipper.enabled?
+    #
+    #   @return [TrueClass] the feature is enabled
+    #   @return [FalseClass] the feature is not enabled.
+    #
+    #   @see FeatureFlag.accessible?
+    #
+    #   @see https://rubydoc.info/gems/yard/file/docs/Tags.md#method for details on the @!method
+    #        macro used to compose this documentation.
+
+    # Unless the given :feature_flag_name is _explicitly_ **disabled**,
     # this method returns true.
     #
     # @param feature_flag_name [Symbol]
@@ -19,6 +34,8 @@ module FeatureFlag
     #       enabled, disabled, or has been removed), the feature is
     #       accessible.
     #
+    # @see FeatureFlag.enabled?
+    #
     # @see https://github.com/forem/forem/pull/8149
     #      for further discussion.
     def accessible?(feature_flag_name, *args)
diff --git a/spec/services/feature_flag_spec.rb b/spec/services/feature_flag_spec.rb
index ab28fc9fb..7ca000fca 100644
--- a/spec/services/feature_flag_spec.rb
+++ b/spec/services/feature_flag_spec.rb
@@ -20,12 +20,32 @@ describe FeatureFlag, type: :service do
   end
 
   describe ".enabled?" do
+    subject(:method_call) { described_class.enabled?(flag) }
+
+    let(:flag) { "foo" }
+
     it "calls Flipper's enabled? method" do
-      allow(Flipper).to receive(:enabled?).with("foo")
+      allow(Flipper).to receive(:enabled?).with(flag)
 
-      described_class.enabled?("foo")
+      described_class.enabled?(flag)
 
-      expect(Flipper).to have_received(:enabled?).with("foo")
+      expect(Flipper).to have_received(:enabled?).with(flag)
+    end
+
+    context "when flag explicitly enabled" do
+      before { described_class.enable(flag) }
+
+      it { is_expected.to be_truthy }
+    end
+
+    context "when flag doesn't exist" do
+      it { is_expected.to be_falsey }
+    end
+
+    context "when flag explicitly disabled" do
+      before { described_class.disable(flag) }
+
+      it { is_expected.to be_falsey }
     end
   end