Merge pull request #377 from spheric/master

only choose override if it exists as valid alternative

lib/split/trial.rb

@@ -53,7 +53,7 @@ def choose!(context = nil)
       # Only run the process once
       return alternative if @alternative_choosen
 
-      if @options[:override]
+      if override_is_alternative?
         self.alternative = @options[:override]
       elsif @options[:disabled] || Split.configuration.disabled?
         self.alternative = @experiment.control
@@ -88,6 +88,10 @@ def run_callback(context, callback_name)
       context.send(callback_name, self) if callback_name && context.respond_to?(callback_name, true)
     end
 
+    def override_is_alternative?
+      @experiment.alternatives.map(&:name).include?(@options[:override])
+    end
+
     def should_store_alternative?
       if @options[:override] || @options[:disabled]
         Split.configuration.store_override

spec/trial_spec.rb

@@ -4,8 +4,9 @@
 
 describe Split::Trial do
   let(:user) { mock_user }
+  let(:alternatives) { ['basket', 'cart'] }
   let(:experiment) do
-    Split::Experiment.new('basket_text', :alternatives => ['basket', 'cart']).save
+    Split::Experiment.new('basket_text', :alternatives => alternatives).save
   end
 
   it "should be initializeable" do
@@ -35,7 +36,6 @@
   end
 
   describe "metadata" do
-    let(:alternatives) { ['basket', 'cart'] }
     let(:metadata) { Hash[alternatives.map { |k| [k, "Metadata for #{k}"] }] }
     let(:experiment) do
       Split::Experiment.new('basket_text', :alternatives => alternatives, :metadata => metadata).save
@@ -83,22 +83,29 @@
     def expect_alternative(trial, alternative_name)
       3.times do
         trial.choose! context
-        expect(trial.alternative.name).to eq(alternative_name)
+        expect(alternative_name).to include(trial.alternative.name)
       end
     end
 
     context "when override is present" do
+      let(:override) { 'cart' }
       let(:trial) do
-        Split::Trial.new(:user => user, :experiment => experiment, :override => 'cart')
+        Split::Trial.new(:user => user, :experiment => experiment, :override => override)
       end
 
       it_behaves_like 'a trial with callbacks'
 
       it "picks the override" do
-        trial = Split::Trial.new(:user => user, :experiment => experiment, :override => 'cart')
         expect(experiment).to_not receive(:next_alternative)
+        expect_alternative(trial, override)
+      end
 
-        expect_alternative(trial, 'cart')
+      context "when alternative doesn't exist" do
+        let(:override) { nil }
+        it 'falls back on next_alternative' do
+          expect(experiment).to receive(:next_alternative).and_call_original
+          expect_alternative(trial, alternatives)
+        end
       end
     end