Skip to content

Commit

Permalink
Fix combined experiments (#502)
Browse files Browse the repository at this point in the history
1. always pass all alternatives to prevent unintentional version bumps
2. return the chosen alternative from `ab_combined_test`

Fixes #500
  • Loading branch information
semanticart authored and andrew committed Sep 20, 2017
1 parent 13e11b6 commit d69048a
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 4 deletions.
9 changes: 8 additions & 1 deletion lib/split/combined_experiments_helper.rb
Expand Up @@ -6,6 +6,7 @@ def ab_combined_test(metric_descriptor, control = nil, *alternatives)
raise(Split::InvalidExperimentsFormatError, 'Unable to find experiment #{metric_descriptor} in configuration') if experiment[:combined_experiments].nil?

alternative = nil
weighted_alternatives = nil
experiment[:combined_experiments].each do |combined_experiment|
if alternative.nil?
if control
Expand All @@ -15,9 +16,15 @@ def ab_combined_test(metric_descriptor, control = nil, *alternatives)
alternative = ab_test(combined_experiment, normalized_alternatives[0], *normalized_alternatives[1])
end
else
ab_test(combined_experiment, [{alternative => 1}])
weighted_alternatives ||= experiment[:alternatives].each_with_object({}) do |alt, memo|
alt = Alternative.new(alt, experiment[:name]).name
memo[alt] = (alt == alternative ? 1 : 0)
end

ab_test(combined_experiment, [weighted_alternatives])
end
end
alternative
end

def find_combined_experiment(metric_descriptor)
Expand Down
6 changes: 3 additions & 3 deletions spec/combined_experiments_helper_spec.rb
Expand Up @@ -46,12 +46,12 @@
end
end

it "uses same alternatives for all sub experiments " do
it "uses same alternative for all sub experiments and returns the alternative" 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}] )
expect(self).to receive(:ab_test).with(:exp_1_scroll, [{"control" => 0, "test-alt" => 1}])

ab_combined_test('combined_exp_1')
expect(ab_combined_test('combined_exp_1')).to eq('test-alt')
end
end
end

0 comments on commit d69048a

Please sign in to comment.