Merge pull request #703 from splitrb/check-if-alternatives-have-valid-data

Do not throw error if alternativas have data that can lead to negative numbers for probability calculation

Author: andrehjr

lib/split/experiment.rb

@@ -270,7 +270,16 @@ def load_from_redis
       set_alternatives_and_options(options)
     end
 
+    def can_calculate_winning_alternatives?
+      self.alternatives.all? do |alternative|
+        alternative.participant_count >= 0 &&
+        (alternative.participant_count >= alternative.completed_count)
+      end
+    end
+
     def calc_winning_alternatives
+      return unless can_calculate_winning_alternatives?
+
       # Cache the winning alternatives so we recalculate them once per the specified interval.
       intervals_since_epoch =
         Time.now.utc.to_i / Split.configuration.winning_alternative_recalculation_interval

spec/dashboard_spec.rb

@@ -279,4 +279,16 @@ def link(color)
 
     expect(last_response.body).to include("<small>Unknown</small>")
   end
+
+  it "should be explode with experiments with invalid data" do
+    red_link.participant_count = 1
+    red_link.set_completed_count(10)
+
+    blue_link.participant_count = 3
+    blue_link.set_completed_count(2)
+
+    get "/"
+
+    expect(last_response).to be_ok
+  end
 end

spec/experiment_spec.rb

@@ -576,6 +576,17 @@ def same_but_different_alternative
       expect(p_goal1).not_to be_within(0.04).of(p_goal2)
     end
 
+    it "should not calculate when data is not valid for beta distribution" do
+      experiment = Split::ExperimentCatalog.find_or_create("scientists", "einstein", "bohr")
+
+      experiment.alternatives.each do |alternative|
+        alternative.participant_count = 9
+        alternative.set_completed_count(10)
+      end
+
+      expect { experiment.calc_winning_alternatives }.to_not raise_error
+    end
+
     it "should return nil and not re-calculate probabilities if they have already been calculated today" do
       experiment = Split::ExperimentCatalog.find_or_create({ "link_color3" => ["purchase", "refund"] }, "blue", "red", "green")
       expect(experiment.calc_winning_alternatives).not_to be nil

spec/spec_helper.rb

@@ -11,6 +11,7 @@
 require "split"
 require "ostruct"
 require "yaml"
+require "pry"
 
 Dir["./spec/support/*.rb"].each { |f| require f }