From 075aadd979c706fa0619d9615cf388fcbd1bc0a3 Mon Sep 17 00:00:00 2001
From: David Dening <ddening@xogrp.com>
Date: Sat, 24 Jun 2017 20:57:09 -0400
Subject: [PATCH 1/3] Support for combined experiments (see README)

---
 README.md           | 24 +++++++++++++++++-
 lib/split/helper.rb | 26 +++++++++++++++++++
 spec/helper_spec.rb | 62 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 111 insertions(+), 1 deletion(-)

diff --git a/README.md b/README.md
index 8f1cad94..65b7450a 100644
--- a/README.md
+++ b/README.md
@@ -1,4 +1,4 @@
-# [Split](http://libraries.io/rubygems/split)
+# [Split](http://libraries.io/rubygems/split) 
 
 [![Gem Version](https://badge.fury.io/rb/split.svg)](http://badge.fury.io/rb/split)
 [![Build Status](https://secure.travis-ci.org/splitrb/split.svg?branch=master)](http://travis-ci.org/splitrb/split)
@@ -623,6 +623,28 @@ Once you finish one of the goals, the test is considered to be completed, and fi
 
 **Bad Example**: Test both how button color affects signup *and* how it affects login, at the same time. THIS WILL NOT WORK.
 
+#### Combined Experiments
+If you want to test how how button color affects signup *and* how it affects login, at the same time. Use combined tests
+Configure like so 
+```ruby
+  Split.configuration.experiments = {
+        :button_color_experiment => {
+          :alternatives => ["blue", "green"],
+          :combined_experiments => ["button_color_on_signup", "button_color_on_login"]
+        }
+      }
+```
+
+Starting the combined test starts all combined experiments
+```ruby
+ ab_combined_test(:button_color_experiment)
+```
+Finish each combined test as normal
+
+```ruby
+   ab_finished(:button_color_on_login)
+   ab_finished(:button_color_on_signup)
+```
 ### DB failover solution
 
 Due to the fact that Redis has no automatic failover mechanism, it's
diff --git a/lib/split/helper.rb b/lib/split/helper.rb
index b3f2667f..e3bc9128 100644
--- a/lib/split/helper.rb
+++ b/lib/split/helper.rb
@@ -10,6 +10,7 @@ def ab_test(metric_descriptor, control = nil, *alternatives)
         experiment = ExperimentCatalog.find_or_initialize(metric_descriptor, control, *alternatives)
         alternative = if Split.configuration.enabled
           experiment.save
+          raise(Split::InvalidExperimentsFormatError) unless Split::configuration.experiments&.dig(experiment.name.to_sym,:combined_experiments).nil?
           trial = Trial.new(:user => ab_user, :experiment => experiment,
               :override => override_alternative(experiment.name), :exclude => exclude_visitor?,
               :disabled => split_generically_disabled?)
@@ -38,6 +39,31 @@ def ab_test(metric_descriptor, control = nil, *alternatives)
       end
     end
 
+     def ab_combined_test(metric_descriptor, control = nil, *alternatives)
+      raise(Split::InvalidExperimentsFormatError) unless metric_descriptor.class == String || metric_descriptor.class == Symbol
+      raise(Split::InvalidExperimentsFormatError) unless Split.configuration.enabled
+      raise(Split::InvalidExperimentsFormatError) unless Split.configuration.allow_multiple_experiments
+      experiment = Split::configuration.experiments[metric_descriptor.to_sym]
+      nil if experiment.nil?
+      raise(Split::InvalidExperimentsFormatError) if experiment[:combined_experiments].nil?
+
+      alternative = nil
+      experiment[:combined_experiments].each do |combined_experiment|
+        if alternative.nil?
+          if control
+            alternative = ab_test(combined_experiment, control, alternatives)
+          else
+            normalized_alternatives = Split::Configuration.new.normalize_alternatives(experiment[:alternatives])
+            alternative = ab_test(combined_experiment, normalized_alternatives[0], *normalized_alternatives[1])
+            # alternative = ab_test(combined_experiment, control, *alternatives)
+          end
+        else
+          ab_test(combined_experiment, [{alternative => 1}])
+        end
+      end
+    end
+
+
     def reset!(experiment)
       ab_user.delete(experiment.key)
     end
diff --git a/spec/helper_spec.rb b/spec/helper_spec.rb
index bb79ca8b..54959591 100755
--- a/spec/helper_spec.rb
+++ b/spec/helper_spec.rb
@@ -35,6 +35,18 @@
       expect(lambda { ab_test({'link_color' => "purchase"}, 'blue', 'red') }).not_to raise_error
     end
 
+    it "raises an appropriate error when processing combined expirements" do
+      Split.configuration.experiments = {
+        :combined_exp_1 => {
+          :alternatives => [  { name: "control", percent: 50 },  { name: "test-alt", percent: 50 } ],
+          :metric => :my_metric,
+          :combined_experiments => [:combined_exp_1_sub_1]
+        }
+      }
+      Split::ExperimentCatalog.find_or_create('combined_exp_1')
+      expect(lambda { ab_test('combined_exp_1')}).to raise_error(Split::InvalidExperimentsFormatError )
+    end
+
     it "should assign a random alternative to a new user when there are an equal number of alternatives assigned" do
       ab_test('link_color', 'blue', 'red')
       expect(['red', 'blue']).to include(ab_user['link_color'])
@@ -253,6 +265,56 @@
     end
   end
 
+  describe 'ab_combined_test' do
+    let!(:config_enabled) { true }
+    let!(:combined_experiments) { [:exp_1_click, :exp_1_scroll ]}
+    let!(:allow_multiple_experiments) { true }
+
+    before do
+      Split.configuration.experiments = {
+        :combined_exp_1 => {
+          :alternatives => [ {"control"=> 0.5}, {"test-alt"=> 0.5} ],
+          :metric => :my_metric,
+          :combined_experiments => combined_experiments
+        }
+      }
+      Split.configuration.enabled = config_enabled
+      Split.configuration.allow_multiple_experiments = allow_multiple_experiments
+    end
+
+    context 'without config enabled' do
+      let!(:config_enabled) { false }
+
+      it "raises an error" do
+        expect(lambda { ab_combined_test :combined_exp_1 }).to raise_error(Split::InvalidExperimentsFormatError )
+      end
+    end
+
+    context 'multiple experiments disabled' do
+      let!(:allow_multiple_experiments) { false }
+
+      it "raises an error if multiple experiments is disabled" do
+        expect(lambda { ab_combined_test :combined_exp_1 }).to raise_error(Split::InvalidExperimentsFormatError)
+      end
+    end
+
+    context 'without combined experiments' do
+      let!(:combined_experiments) { nil }
+
+      it "raises an error" do
+        expect(lambda { ab_combined_test :combined_exp_1 }).to raise_error(Split::InvalidExperimentsFormatError )
+      end
+    end
+
+    it "uses same alternatives for all sub experiments " do
+      allow(self).to receive(:get_alternative) { "test-alt" }
+      expect(self).to receive(:ab_test).with(:exp_1_click, {"control"=>0.5}, {"test-alt"=>0.5}) { "test-alt" }
+      expect(self).to receive(:ab_test).with(:exp_1_scroll, [{"test-alt" => 1}] )
+
+      ab_combined_test('combined_exp_1')
+    end
+  end
+
   describe 'metadata' do
     before do
       Split.configuration.experiments = {

From 2535e65f6681a10c6fa7eecb57a409ab2535db40 Mon Sep 17 00:00:00 2001
From: David Dening <ddening@xogrp.com>
Date: Mon, 3 Jul 2017 11:42:57 -0400
Subject: [PATCH 2/3] Refactor combined experiments into its own helper

---
 lib/split.rb                             |  1 +
 lib/split/combined_experiments_helper.rb | 30 +++++++++++++
 lib/split/engine.rb                      |  2 +
 lib/split/helper.rb                      | 25 -----------
 spec/combined_experiments_helper_spec.rb | 57 ++++++++++++++++++++++++
 spec/helper_spec.rb                      | 50 ---------------------
 6 files changed, 90 insertions(+), 75 deletions(-)
 create mode 100644 lib/split/combined_experiments_helper.rb
 create mode 100644 spec/combined_experiments_helper_spec.rb

diff --git a/lib/split.rb b/lib/split.rb
index 58b8e9f6..83fbc44b 100755
--- a/lib/split.rb
+++ b/lib/split.rb
@@ -13,6 +13,7 @@
 require 'split/extensions/string'
 require 'split/goals_collection'
 require 'split/helper'
+require 'split/combined_experiments_helper'
 require 'split/metric'
 require 'split/persistence'
 require 'split/redis_interface'
diff --git a/lib/split/combined_experiments_helper.rb b/lib/split/combined_experiments_helper.rb
new file mode 100644
index 00000000..d824615f
--- /dev/null
+++ b/lib/split/combined_experiments_helper.rb
@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+module Split
+  module CombinedExperimentsHelper
+    def ab_combined_test(metric_descriptor, control = nil, *alternatives)
+      return nil unless experiment = find_combined_experiment(metric_descriptor)
+      raise(Split::InvalidExperimentsFormatError) if experiment[:combined_experiments].nil?
+
+      alternative = nil
+      experiment[:combined_experiments].each do |combined_experiment|
+        if alternative.nil?
+          if control
+            alternative = ab_test(combined_experiment, control, alternatives)
+          else
+            normalized_alternatives = Split::Configuration.new.normalize_alternatives(experiment[:alternatives])
+            alternative = ab_test(combined_experiment, normalized_alternatives[0], *normalized_alternatives[1])
+          end
+        else
+          ab_test(combined_experiment, [{alternative => 1}])
+        end
+      end
+    end
+
+    def find_combined_experiment(metric_descriptor)
+      raise(Split::InvalidExperimentsFormatError) unless metric_descriptor.class == String || metric_descriptor.class == Symbol
+      raise(Split::InvalidExperimentsFormatError) unless Split.configuration.enabled
+      raise(Split::InvalidExperimentsFormatError) unless Split.configuration.allow_multiple_experiments
+      experiment = Split::configuration.experiments[metric_descriptor.to_sym]
+    end
+  end
+end
diff --git a/lib/split/engine.rb b/lib/split/engine.rb
index 579dd2d9..9dc3ece3 100644
--- a/lib/split/engine.rb
+++ b/lib/split/engine.rb
@@ -5,6 +5,8 @@ class Engine < ::Rails::Engine
       if Split.configuration.include_rails_helper
         ActionController::Base.send :include, Split::Helper
         ActionController::Base.helper Split::Helper
+        ActionController::Base.send :include, Split::CombinedExperimentsHelper
+        ActionController::Base.helper Split::CombinedExperimentsHelper
       end
     end
   end
diff --git a/lib/split/helper.rb b/lib/split/helper.rb
index e3bc9128..434042c8 100644
--- a/lib/split/helper.rb
+++ b/lib/split/helper.rb
@@ -39,31 +39,6 @@ def ab_test(metric_descriptor, control = nil, *alternatives)
       end
     end
 
-     def ab_combined_test(metric_descriptor, control = nil, *alternatives)
-      raise(Split::InvalidExperimentsFormatError) unless metric_descriptor.class == String || metric_descriptor.class == Symbol
-      raise(Split::InvalidExperimentsFormatError) unless Split.configuration.enabled
-      raise(Split::InvalidExperimentsFormatError) unless Split.configuration.allow_multiple_experiments
-      experiment = Split::configuration.experiments[metric_descriptor.to_sym]
-      nil if experiment.nil?
-      raise(Split::InvalidExperimentsFormatError) if experiment[:combined_experiments].nil?
-
-      alternative = nil
-      experiment[:combined_experiments].each do |combined_experiment|
-        if alternative.nil?
-          if control
-            alternative = ab_test(combined_experiment, control, alternatives)
-          else
-            normalized_alternatives = Split::Configuration.new.normalize_alternatives(experiment[:alternatives])
-            alternative = ab_test(combined_experiment, normalized_alternatives[0], *normalized_alternatives[1])
-            # alternative = ab_test(combined_experiment, control, *alternatives)
-          end
-        else
-          ab_test(combined_experiment, [{alternative => 1}])
-        end
-      end
-    end
-
-
     def reset!(experiment)
       ab_user.delete(experiment.key)
     end
diff --git a/spec/combined_experiments_helper_spec.rb b/spec/combined_experiments_helper_spec.rb
new file mode 100644
index 00000000..46369f11
--- /dev/null
+++ b/spec/combined_experiments_helper_spec.rb
@@ -0,0 +1,57 @@
+# frozen_string_literal: true
+require 'spec_helper'
+require 'split/combined_experiments_helper'
+
+describe Split::CombinedExperimentsHelper do
+  include Split::CombinedExperimentsHelper
+
+  describe 'ab_combined_test' do
+    let!(:config_enabled) { true }
+    let!(:combined_experiments) { [:exp_1_click, :exp_1_scroll ]}
+    let!(:allow_multiple_experiments) { true }
+
+    before do
+      Split.configuration.experiments = {
+        :combined_exp_1 => {
+          :alternatives => [ {"control"=> 0.5}, {"test-alt"=> 0.5} ],
+          :metric => :my_metric,
+          :combined_experiments => combined_experiments
+        }
+      }
+      Split.configuration.enabled = config_enabled
+      Split.configuration.allow_multiple_experiments = allow_multiple_experiments
+    end
+
+    context 'without config enabled' do
+      let!(:config_enabled) { false }
+
+      it "raises an error" do
+        expect(lambda { ab_combined_test :combined_exp_1 }).to raise_error(Split::InvalidExperimentsFormatError )
+      end
+    end
+
+    context 'multiple experiments disabled' do
+      let!(:allow_multiple_experiments) { false }
+
+      it "raises an error if multiple experiments is disabled" do
+        expect(lambda { ab_combined_test :combined_exp_1 }).to raise_error(Split::InvalidExperimentsFormatError)
+      end
+    end
+
+    context 'without combined experiments' do
+      let!(:combined_experiments) { nil }
+
+      it "raises an error" do
+        expect(lambda { ab_combined_test :combined_exp_1 }).to raise_error(Split::InvalidExperimentsFormatError )
+      end
+    end
+
+    it "uses same alternatives for all sub experiments " do
+      allow(self).to receive(:get_alternative) { "test-alt" }
+      expect(self).to receive(:ab_test).with(:exp_1_click, {"control"=>0.5}, {"test-alt"=>0.5}) { "test-alt" }
+      expect(self).to receive(:ab_test).with(:exp_1_scroll, [{"test-alt" => 1}] )
+
+      ab_combined_test('combined_exp_1')
+    end
+  end
+end
diff --git a/spec/helper_spec.rb b/spec/helper_spec.rb
index 54959591..ff434556 100755
--- a/spec/helper_spec.rb
+++ b/spec/helper_spec.rb
@@ -265,56 +265,6 @@
     end
   end
 
-  describe 'ab_combined_test' do
-    let!(:config_enabled) { true }
-    let!(:combined_experiments) { [:exp_1_click, :exp_1_scroll ]}
-    let!(:allow_multiple_experiments) { true }
-
-    before do
-      Split.configuration.experiments = {
-        :combined_exp_1 => {
-          :alternatives => [ {"control"=> 0.5}, {"test-alt"=> 0.5} ],
-          :metric => :my_metric,
-          :combined_experiments => combined_experiments
-        }
-      }
-      Split.configuration.enabled = config_enabled
-      Split.configuration.allow_multiple_experiments = allow_multiple_experiments
-    end
-
-    context 'without config enabled' do
-      let!(:config_enabled) { false }
-
-      it "raises an error" do
-        expect(lambda { ab_combined_test :combined_exp_1 }).to raise_error(Split::InvalidExperimentsFormatError )
-      end
-    end
-
-    context 'multiple experiments disabled' do
-      let!(:allow_multiple_experiments) { false }
-
-      it "raises an error if multiple experiments is disabled" do
-        expect(lambda { ab_combined_test :combined_exp_1 }).to raise_error(Split::InvalidExperimentsFormatError)
-      end
-    end
-
-    context 'without combined experiments' do
-      let!(:combined_experiments) { nil }
-
-      it "raises an error" do
-        expect(lambda { ab_combined_test :combined_exp_1 }).to raise_error(Split::InvalidExperimentsFormatError )
-      end
-    end
-
-    it "uses same alternatives for all sub experiments " do
-      allow(self).to receive(:get_alternative) { "test-alt" }
-      expect(self).to receive(:ab_test).with(:exp_1_click, {"control"=>0.5}, {"test-alt"=>0.5}) { "test-alt" }
-      expect(self).to receive(:ab_test).with(:exp_1_scroll, [{"test-alt" => 1}] )
-
-      ab_combined_test('combined_exp_1')
-    end
-  end
-
   describe 'metadata' do
     before do
       Split.configuration.experiments = {

From b0d5a9df61c83d0a97b76f2526ae0d4553835e1f Mon Sep 17 00:00:00 2001
From: David Dening <ddening@xogrp.com>
Date: Tue, 4 Jul 2017 14:02:53 -0400
Subject: [PATCH 3/3] Update README, add descriptive error messages

---
 README.md                                | 7 +++++++
 lib/split/combined_experiments_helper.rb | 8 ++++----
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/README.md b/README.md
index 65b7450a..815d9658 100644
--- a/README.md
+++ b/README.md
@@ -645,6 +645,13 @@ Finish each combined test as normal
    ab_finished(:button_color_on_login)
    ab_finished(:button_color_on_signup)
 ```
+
+**Additional Configuration**: 
+* Be sure to enable `allow_multiple_experiments` 
+* In Sinatra include the CombinedExperimentsHelper
+  ```
+    helpers Split::CombinedExperimentsHelper
+  ```
 ### DB failover solution
 
 Due to the fact that Redis has no automatic failover mechanism, it's
diff --git a/lib/split/combined_experiments_helper.rb b/lib/split/combined_experiments_helper.rb
index d824615f..8fc9e88e 100644
--- a/lib/split/combined_experiments_helper.rb
+++ b/lib/split/combined_experiments_helper.rb
@@ -3,7 +3,7 @@ module Split
   module CombinedExperimentsHelper
     def ab_combined_test(metric_descriptor, control = nil, *alternatives)
       return nil unless experiment = find_combined_experiment(metric_descriptor)
-      raise(Split::InvalidExperimentsFormatError) if experiment[:combined_experiments].nil?
+      raise(Split::InvalidExperimentsFormatError, 'Unable to find experiment #{metric_descriptor} in configuration') if experiment[:combined_experiments].nil?
 
       alternative = nil
       experiment[:combined_experiments].each do |combined_experiment|
@@ -21,9 +21,9 @@ def ab_combined_test(metric_descriptor, control = nil, *alternatives)
     end
 
     def find_combined_experiment(metric_descriptor)
-      raise(Split::InvalidExperimentsFormatError) unless metric_descriptor.class == String || metric_descriptor.class == Symbol
-      raise(Split::InvalidExperimentsFormatError) unless Split.configuration.enabled
-      raise(Split::InvalidExperimentsFormatError) unless Split.configuration.allow_multiple_experiments
+      raise(Split::InvalidExperimentsFormatError, 'Invalid descriptor class (String or Symbol required)') unless metric_descriptor.class == String || metric_descriptor.class == Symbol
+      raise(Split::InvalidExperimentsFormatError, 'Enable configuration') unless Split.configuration.enabled
+      raise(Split::InvalidExperimentsFormatError, 'Enable `allow_multiple_experiments`') unless Split.configuration.allow_multiple_experiments
       experiment = Split::configuration.experiments[metric_descriptor.to_sym]
     end
   end