Merge pull request #622 from splitrb/consistent-return-metadata-when-disabled

ab_test must return metadata on error or if split is disabled/excluded user

Author: andrehjr

lib/split/helper.rb

@@ -32,8 +32,8 @@ def ab_test(metric_descriptor, control = nil, *alternatives)
       end
 
       if block_given?
-        metadata = trial ? trial.metadata : {}
-        yield(alternative, metadata)
+        metadata = experiment.metadata[alternative] if experiment.metadata
+        yield(alternative, metadata || {})
       else
         alternative
       end

spec/encapsulated_helper_spec.rb

@@ -21,7 +21,7 @@ def params
     end
 
     it "calls the block with selected alternative" do
-      expect{|block| ab_test('link_color', 'red', 'red', &block) }.to yield_with_args('red', nil)
+      expect{|block| ab_test('link_color', 'red', 'red', &block) }.to yield_with_args('red', {})
     end
 
     context "inside a view" do

spec/helper_spec.rb

@@ -277,33 +277,63 @@
   end
 
   describe 'metadata' do
-    before do
-      Split.configuration.experiments = {
-        :my_experiment => {
-          :alternatives => ["one", "two"],
-          :resettable => false,
-          :metadata => { 'one' => 'Meta1', 'two' => 'Meta2' }
+    context 'is defined' do
+      before do
+        Split.configuration.experiments = {
+          :my_experiment => {
+            :alternatives => ["one", "two"],
+            :resettable => false,
+            :metadata => { 'one' => 'Meta1', 'two' => 'Meta2' }
+          }
         }
-      }
-    end
+      end
+
+      it 'should be passed to helper block' do
+        @params = { 'ab_test' => { 'my_experiment' => 'two' } }
+        expect(ab_test('my_experiment')).to eq 'two'
+        expect(ab_test('my_experiment') do |alternative, meta|
+          meta
+        end).to eq('Meta2')
+      end
+
+      it 'should pass control metadata helper block if library disabled' do
+        Split.configure do |config|
+          config.enabled = false
+        end
 
-    it 'should be passed to helper block' do
-      @params = { 'ab_test' => { 'my_experiment' => 'one' } }
-      expect(ab_test('my_experiment')).to eq 'one'
-      expect(ab_test('my_experiment') do |alternative, meta|
-        meta
-      end).to eq('Meta1')
+        expect(ab_test('my_experiment')).to eq 'one'
+        expect(ab_test('my_experiment') do |_, meta|
+          meta
+        end).to eq('Meta1')
+      end
     end
 
-    it 'should pass empty hash to helper block if library disabled' do
-      Split.configure do |config|
-        config.enabled = false
+    context 'is not defined' do
+      before do
+        Split.configuration.experiments = {
+          :my_experiment => {
+            :alternatives => ["one", "two"],
+            :resettable => false,
+            :metadata => nil
+          }
+        }
       end
 
-      expect(ab_test('my_experiment')).to eq 'one'
-      expect(ab_test('my_experiment') do |_, meta|
-        meta
-      end).to eq({})
+      it 'should be passed to helper block' do
+        expect(ab_test('my_experiment') do |alternative, meta|
+          meta
+        end).to eq({})
+      end
+
+      it 'should pass control metadata helper block if library disabled' do
+        Split.configure do |config|
+          config.enabled = false
+        end
+
+        expect(ab_test('my_experiment') do |_, meta|
+          meta
+        end).to eq({})
+      end
     end
   end