diff --git a/lib/ab_test.rb b/lib/ab_test.rb index 3e7a7a84326..abee04e0a13 100644 --- a/lib/ab_test.rb +++ b/lib/ab_test.rb @@ -16,9 +16,9 @@ def initialize(queries: [], **) end end.freeze - # @param [Proc,Regexp,string,Boolean,nil] should_log Controls whether bucket data for this - # A/B test is logged with specific - # events. + # @param [Regexp,#include?,nil] should_log A list of analytics event names for which the A/B test + # bucket assignment should be logged, or a regular expression pattern which is tested against an + # analytics event name when an event is being logged. # @yieldparam [ActionDispatch::Request] request # @yieldparam [String,nil] service_provider Issuer string for the service provider associated with # the current session. @@ -79,7 +79,7 @@ def include_in_analytics_event?(event_name) elsif !should_log.nil? raise 'Unexpected value used for should_log' else - true + false end end diff --git a/spec/lib/ab_test_spec.rb b/spec/lib/ab_test_spec.rb index 06b08a77ab6..172e2ffe7a7 100644 --- a/spec/lib/ab_test_spec.rb +++ b/spec/lib/ab_test_spec.rb @@ -171,23 +171,20 @@ subject(:return_value) { ab_test.include_in_analytics_event?(event_name) } context 'when should_log is nil' do - it 'returns true' do - expect(return_value).to eql(true) - end + it { is_expected.to eql(false) } end context 'when Regexp is used' do context 'and it matches' do let(:should_log) { /cool/ } - it 'returns true' do - expect(return_value).to eql(true) - end + + it { is_expected.to eql(true) } end + context 'and it does not match' do let(:should_log) { /not cool/ } - it 'returns false' do - expect(return_value).to eql(false) - end + + it { is_expected.to eql(false) } end end diff --git a/spec/services/analytics_spec.rb b/spec/services/analytics_spec.rb index 872ae64c5ee..8ed85ac60da 100644 --- a/spec/services/analytics_spec.rb +++ b/spec/services/analytics_spec.rb @@ -154,9 +154,7 @@ bucket_b: 50, }, should_log:, - ) do |user:, **| - user.id - end, + ), } end @@ -166,28 +164,25 @@ allow(AbTests).to receive(:all).and_return(ab_tests) end - it 'includes ab_tests in logged event' do - expect(ahoy).to receive(:track).with( - 'Trackable Event', - analytics_attributes.merge( - ab_tests: { - foo_test: { - bucket: anything, - }, - }, - ), - ) + it 'does not include ab_tests in logged event' do + expect(ahoy).to receive(:track).with('Trackable Event', analytics_attributes) analytics.track_event('Trackable Event') end - context 'when should_log says not to' do - let(:should_log) { /some other event/ } + context 'for an included test' do + let(:should_log) { /Trackable/ } - it 'does not include ab_test in logged event' do + it 'includes ab_test bucket detail in logged event' do expect(ahoy).to receive(:track).with( 'Trackable Event', - analytics_attributes, + analytics_attributes.merge( + ab_tests: { + foo_test: { + bucket: be_in([:bucket_a, :bucket_b]), + }, + }, + ), ) analytics.track_event('Trackable Event')