Merge pull request #2994 from DataDog/fix-en-var

DataDog · Jul 25, 2023 · 177c1c3 · 177c1c3
2 parents ba913d4 + 97025c6
commit 177c1c3
Show file tree

Hide file tree

Showing 3 changed files with 75 additions and 89 deletions.
diff --git a/lib/datadog/core/configuration/options.rb b/lib/datadog/core/configuration/options.rb
@@ -68,18 +68,15 @@ def options
           end
 
           def set_option(name, value, precedence: Configuration::Option::Precedence::PROGRAMMATIC)
-            add_option(name) unless options.key?(name)
-            options[name].set(value, precedence: precedence)
+            resolve_option(name).set(value, precedence: precedence)
           end
 
           def unset_option(name, precedence: Configuration::Option::Precedence::PROGRAMMATIC)
-            add_option(name) unless options.key?(name)
-            options[name].unset(precedence)
+            resolve_option(name).unset(precedence)
           end
 
           def get_option(name)
-            add_option(name) unless options.key?(name)
-            options[name].get
+            resolve_option(name).get
           end
 
           def reset_option(name)
@@ -91,10 +88,10 @@ def option_defined?(name)
             self.class.options.key?(name)
           end
 
-          def using_default?(option)
-            return options[option].default_precedence? if options[option]
-
-            true
+          # Is this option's value the default fallback value?
+          def using_default?(name)
+            get_option(name) # Resolve value check if environment variable overwrote the default
+            options[name].default_precedence?
           end
 
           def options_hash
@@ -109,12 +106,14 @@ def reset_options!
 
           private
 
-          def add_option(name)
+          # Ensure option DSL is loaded
+          def resolve_option(name)
+            option = options[name]
+            return option if option
+
             assert_valid_option!(name)
             definition = self.class.options[name]
-            definition.build(self).tap do |option|
-              options[name] = option
-            end
+            options[name] = definition.build(self)
           end
 
           def assert_valid_option!(name)

diff --git a/spec/datadog/core/configuration/base_spec.rb b/spec/datadog/core/configuration/base_spec.rb
@@ -142,81 +142,6 @@
         end
       end
 
-      describe '#using_default?' do
-        let(:configuration) do
-          Class.new do
-            include Datadog::Core::Configuration::Base
-
-            settings :fake_test do
-              option :enabled, default: true
-              option :without_default
-            end
-          end.new
-        end
-
-        context 'with default option' do
-          context 'when not set or accessed' do
-            it 'returns true' do
-              expect(configuration.fake_test.using_default?(:enabled)).to be(true)
-            end
-          end
-
-          context 'when not set but accessed' do
-            it 'returns true' do
-              configuration.fake_test.enabled
-              expect(configuration.fake_test.using_default?(:enabled)).to be(true)
-            end
-          end
-
-          context 'when set' do
-            it 'returns false' do
-              configuration.fake_test.enabled = false
-              expect(configuration.fake_test.using_default?(:enabled)).to be(false)
-            end
-          end
-
-          context 'when set and reset' do
-            it 'returns false' do
-              configuration.fake_test.enabled = false
-              expect(configuration.fake_test.using_default?(:enabled)).to be(false)
-              configuration.fake_test.reset!
-              expect(configuration.fake_test.using_default?(:enabled)).to be(true)
-            end
-          end
-        end
-
-        context 'without default option' do
-          context 'when not set or accessed' do
-            it 'returns true' do
-              expect(configuration.fake_test.using_default?(:without_default)).to be(true)
-            end
-          end
-
-          context 'when not set but accessed' do
-            it 'returns true' do
-              configuration.fake_test.without_default
-              expect(configuration.fake_test.using_default?(:without_default)).to be(true)
-            end
-          end
-
-          context 'when set' do
-            it 'returns false' do
-              configuration.fake_test.without_default = false
-              expect(configuration.fake_test.using_default?(:without_default)).to be(false)
-            end
-          end
-
-          context 'when set and reset' do
-            it 'returns false' do
-              configuration.fake_test.without_default = false
-              expect(configuration.fake_test.using_default?(:without_default)).to be(false)
-              configuration.fake_test.reset!
-              expect(configuration.fake_test.using_default?(:without_default)).to be(true)
-            end
-          end
-        end
-      end
-
       describe '#reset!' do
         subject(:reset!) { base_object.reset! }
 

diff --git a/spec/datadog/core/configuration/options_spec.rb b/spec/datadog/core/configuration/options_spec.rb
@@ -264,6 +264,68 @@
         end
       end
 
+      describe '#using_default?' do
+        subject(:using_default?) { options_object.using_default?(name) }
+
+        let(:name) { :foo }
+
+        context 'when the option is defined' do
+          before { options_class.send(:option, name, meta) }
+
+          let(:meta) { {} }
+
+          context 'and a value is set' do
+            before { options_object.set_option(name, 'something') }
+
+            it { is_expected.to be(false) }
+          end
+
+          context 'and a value is not set' do
+            context 'and no default value is configured' do
+              it { is_expected.to be(true) }
+            end
+
+            context 'and a default value is configured' do
+              let(:meta) { { default: 'anything' } }
+
+              it { is_expected.to be(true) }
+
+              context 'and an environment variable is configured' do
+                let(:meta) { { default: 'anything', env: 'TEST_ENV_VAR' } }
+
+                context 'and an environmet variable is set' do
+                  around do |example|
+                    ClimateControl.modify('TEST_ENV_VAR' => 'anything') { example.run }
+                  end
+
+                  it { is_expected.to be(false) }
+                end
+
+                context 'and an environment variable is not set' do
+                  it { is_expected.to be(true) }
+                end
+              end
+            end
+
+            context 'an environment variable is configured' do
+              let(:meta) { { env: 'TEST_ENV_VAR' } }
+
+              context 'and an environmet variable is set' do
+                around do |example|
+                  ClimateControl.modify('TEST_ENV_VAR' => 'anything') { example.run }
+                end
+
+                it { is_expected.to be(false) }
+              end
+
+              context 'and an environment variable is not set' do
+                it { is_expected.to be(true) }
+              end
+            end
+          end
+        end
+      end
+
       describe '#option_defined?' do
         subject(:option_defined?) { options_object.option_defined?(name) }