Skip to content

Commit 37c8bc8

Browse files
aliabbasrizvialiabbasrizvi
authored
Updating whitelisting check to precede audience check (#25)
1 parent afde471 commit 37c8bc8

File tree

4 files changed

+131
-2
lines changed

4 files changed

+131
-2
lines changed

‎optimizely/helpers/experiment.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,20 @@ def is_experiment_running(experiment):
1212
"""
1313

1414
return experiment.status in ALLOWED_EXPERIMENT_STATUS
15+
16+
17+
def is_user_in_forced_variation(forced_variations, user_id):
18+
""" Determine if the user is in a forced variation.
19+
20+
Args:
21+
forced_variations: Dict representing forced variations for the experiment.
22+
user_id: User to check for in whitelist.
23+
24+
Returns:
25+
Boolean depending on whether user is in forced variation or not.
26+
"""
27+
28+
if forced_variations and user_id in forced_variations:
29+
return True
30+
31+
return False

‎optimizely/optimizely.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,9 @@ def _validate_preconditions(self, experiment, user_id, attributes):
8080
self.logger.log(enums.LogLevels.INFO, 'Experiment "%s" is not running.' % experiment.key)
8181
return False
8282

83+
if experiment_helper.is_user_in_forced_variation(experiment.forcedVariations, user_id):
84+
return True
85+
8386
if not audience_helper.is_user_in_experiment(self.config, experiment, attributes):
8487
self.logger.log(
8588
enums.LogLevels.INFO,

‎tests/helpers_tests/test_experiment.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,19 @@ def test_is_experiment_running__status_not_running(self):
2020
'42', 'test_experiment', 'Some Status', [], [], {},[])) as mock_get_experiment:
2121
self.assertFalse(experiment.is_experiment_running(self.project_config.get_experiment_from_key('test_experiment')))
2222
mock_get_experiment.assert_called_once_with('test_experiment')
23+
24+
def test_is_user_in_forced_variation__no_forced_variation_dict(self):
25+
""" Test that is_user_in_forced_variation returns False when experiment has no forced variations. """
26+
27+
self.assertFalse(experiment.is_user_in_forced_variation(None, 'test_user'))
28+
self.assertFalse(experiment.is_user_in_forced_variation({}, 'test_user'))
29+
30+
def test_is_user_in_forced_variation__user_not_in_forced_variation(self):
31+
""" Test that is_user_in_forced_variation returns False when user is not in experiment's forced variations. """
32+
33+
self.assertFalse(experiment.is_user_in_forced_variation({'user_1': 'control'}, 'test_user'))
34+
35+
def test_is_user_in_forced_variation__user_in_forced_variation(self):
36+
""" Test that is_user_in_forced_variation returns True when user is in experiment's forced variations. """
37+
38+
self.assertTrue(experiment.is_user_in_forced_variation({'user_1': 'control'}, 'user_1'))

‎tests/test_optimizely.py

Lines changed: 95 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,12 +151,29 @@ def test_activate__with_attributes__invalid_attributes(self):
151151
def test_activate__experiment_not_running(self):
152152
""" Test that activate returns None when experiment is not Running. """
153153

154-
with mock.patch('optimizely.helpers.audience.is_user_in_experiment', return_value=True),\
154+
with mock.patch('optimizely.helpers.audience.is_user_in_experiment', return_value=True) as mock_audience_check,\
155+
mock.patch('optimizely.helpers.experiment.is_user_in_forced_variation',
156+
return_value=True) as mock_whitelist_check,\
155157
mock.patch('optimizely.helpers.experiment.is_experiment_running',
156158
return_value=False) as mock_is_experiment_running:
157159
self.assertIsNone(self.optimizely.activate('test_experiment', 'test_user',
158160
attributes={'test_attribute': 'test_value'}))
159161
mock_is_experiment_running.assert_called_once_with(self.project_config.get_experiment_from_key('test_experiment'))
162+
self.assertEqual(0, mock_whitelist_check.call_count)
163+
self.assertEqual(0, mock_audience_check.call_count)
164+
165+
def test_activate__whitelisting_overrides_audience_check(self):
166+
""" Test that during activate whitelist overrides audience check if user is in the whitelist. """
167+
168+
with mock.patch('optimizely.helpers.audience.is_user_in_experiment', return_value=False) as mock_audience_check,\
169+
mock.patch('optimizely.helpers.experiment.is_user_in_forced_variation',
170+
return_value=True) as mock_whitelist_check,\
171+
mock.patch('optimizely.helpers.experiment.is_experiment_running',
172+
return_value=True) as mock_is_experiment_running:
173+
self.assertEqual('control', self.optimizely.activate('test_experiment', 'user_1'))
174+
mock_is_experiment_running.assert_called_once_with(self.project_config.get_experiment_from_key('test_experiment'))
175+
mock_whitelist_check.assert_called_once_with({'user_1': 'control', 'user_2': 'control'}, 'user_1')
176+
self.assertEqual(0, mock_audience_check.call_count)
160177

161178
def test_activate__bucketer_returns_none(self):
162179
""" Test that activate returns None when user is in no variation. """
@@ -253,12 +270,35 @@ def test_track__experiment_not_running(self):
253270

254271
with mock.patch('optimizely.helpers.experiment.is_experiment_running',
255272
return_value=False) as mock_is_experiment_running,\
273+
mock.patch('optimizely.helpers.experiment.is_user_in_forced_variation',
274+
return_value=True) as mock_whitelist_check,\
275+
mock.patch('optimizely.helpers.audience.is_user_in_experiment', return_value=True) as mock_audience_check,\
256276
mock.patch('time.time', return_value=42),\
257277
mock.patch('optimizely.event_dispatcher.EventDispatcher.dispatch_event') as mock_dispatch_event:
258278
self.optimizely.track('test_event', 'test_user')
259279

260280
mock_is_experiment_running.assert_called_once_with(self.project_config.get_experiment_from_key('test_experiment'))
261281
self.assertEqual(0, mock_dispatch_event.call_count)
282+
self.assertEqual(0, mock_whitelist_check.call_count)
283+
self.assertEqual(0, mock_audience_check.call_count)
284+
285+
def test_track__whitelisted_user_overrides_audience_check(self):
286+
""" Test that track does not check for user in audience when user is in whitelist. """
287+
288+
with mock.patch('optimizely.helpers.experiment.is_experiment_running',
289+
return_value=True) as mock_is_experiment_running,\
290+
mock.patch('optimizely.helpers.experiment.is_user_in_forced_variation',
291+
return_value=True) as mock_whitelist_check,\
292+
mock.patch('optimizely.helpers.audience.is_user_in_experiment',
293+
return_value=False) as mock_audience_check,\
294+
mock.patch('time.time', return_value=42),\
295+
mock.patch('optimizely.event_dispatcher.EventDispatcher.dispatch_event') as mock_dispatch_event:
296+
self.optimizely.track('test_event', 'user_1')
297+
298+
mock_is_experiment_running.assert_called_once_with(self.project_config.get_experiment_from_key('test_experiment'))
299+
mock_whitelist_check.assert_called_once_with({'user_1': 'control', 'user_2': 'control'}, 'user_1')
300+
self.assertEqual(1, mock_dispatch_event.call_count)
301+
self.assertEqual(0, mock_audience_check.call_count)
262302

263303
def test_get_variation__audience_match_and_experiment_running(self):
264304
""" Test that get variation retrieves expected variation
@@ -296,13 +336,31 @@ def test_get_variation__experiment_not_running(self):
296336
return_value=self.project_config.get_variation_from_id(
297337
'test_experiment', '111129'
298338
)) as mock_bucket,\
339+
mock.patch('optimizely.helpers.experiment.is_user_in_forced_variation',
340+
return_value=True) as mock_whitelist_check,\
341+
mock.patch('optimizely.helpers.audience.is_user_in_experiment', return_value=True) as mock_audience_check,\
299342
mock.patch('optimizely.helpers.experiment.is_experiment_running',
300343
return_value=False) as mock_is_experiment_running:
301344
self.assertIsNone(self.optimizely.get_variation('test_experiment', 'test_user',
302345
attributes={'test_attribute': 'test_value'}))
303346

304347
self.assertEqual(0, mock_bucket.call_count)
305348
mock_is_experiment_running.assert_called_once_with(self.project_config.get_experiment_from_key('test_experiment'))
349+
self.assertEqual(0, mock_whitelist_check.call_count)
350+
self.assertEqual(0, mock_audience_check.call_count)
351+
352+
def test_get_variation__whitelisting_overrides_audience_check(self):
353+
""" Test that in get_variation whitelist overrides audience check if user is in the whitelist. """
354+
355+
with mock.patch('optimizely.helpers.audience.is_user_in_experiment', return_value=False) as mock_audience_check,\
356+
mock.patch('optimizely.helpers.experiment.is_user_in_forced_variation',
357+
return_value=True) as mock_whitelist_check,\
358+
mock.patch('optimizely.helpers.experiment.is_experiment_running',
359+
return_value=True) as mock_is_experiment_running:
360+
self.assertEqual('control', self.optimizely.get_variation('test_experiment', 'user_1'))
361+
mock_is_experiment_running.assert_called_once_with(self.project_config.get_experiment_from_key('test_experiment'))
362+
self.assertEqual(1, mock_whitelist_check.call_count)
363+
self.assertEqual(0, mock_audience_check.call_count)
306364

307365
def test_custom_logger(self):
308366
""" Test creating Optimizely object with a custom logger. """
@@ -443,7 +501,9 @@ def test_activate__with_attributes__invalid_attributes(self):
443501
def test_activate__experiment_not_running(self):
444502
""" Test that activate returns None and does not dispatch event when experiment is not Running. """
445503

446-
with mock.patch('optimizely.helpers.audience.is_user_in_experiment', return_value=True),\
504+
with mock.patch('optimizely.helpers.audience.is_user_in_experiment', return_value=True) as mock_audience_check,\
505+
mock.patch('optimizely.helpers.experiment.is_user_in_forced_variation',
506+
return_value=True) as mock_whitelist_check,\
447507
mock.patch('optimizely.helpers.experiment.is_experiment_running',
448508
return_value=False) as mock_is_experiment_running, \
449509
mock.patch('optimizely.bucketer.Bucketer.bucket') as mock_bucket,\
@@ -452,9 +512,24 @@ def test_activate__experiment_not_running(self):
452512
attributes={'test_attribute': 'test_value'}))
453513

454514
mock_is_experiment_running.assert_called_once_with(self.project_config.get_experiment_from_key('test_experiment'))
515+
self.assertEqual(0, mock_audience_check.call_count)
516+
self.assertEqual(0, mock_whitelist_check.call_count)
455517
self.assertEqual(0, mock_bucket.call_count)
456518
self.assertEqual(0, mock_dispatch_event.call_count)
457519

520+
def test_activate__whitelisting_overrides_audience_check(self):
521+
""" Test that during activate whitelist overrides audience check if user is in the whitelist. """
522+
523+
with mock.patch('optimizely.helpers.audience.is_user_in_experiment', return_value=False) as mock_audience_check,\
524+
mock.patch('optimizely.helpers.experiment.is_user_in_forced_variation',
525+
return_value=True) as mock_whitelist_check,\
526+
mock.patch('optimizely.helpers.experiment.is_experiment_running',
527+
return_value=True) as mock_is_experiment_running:
528+
self.assertEqual('control', self.optimizely.activate('test_experiment', 'user_1'))
529+
mock_is_experiment_running.assert_called_once_with(self.project_config.get_experiment_from_key('test_experiment'))
530+
self.assertEqual(1, mock_whitelist_check.call_count)
531+
self.assertEqual(0, mock_audience_check.call_count)
532+
458533
def test_activate__bucketer_returns_none(self):
459534
""" Test that activate returns None and does not dispatch event when user is in no variation. """
460535

@@ -595,6 +670,24 @@ def test_track__experiment_not_running(self):
595670
mock_is_experiment_running.assert_called_once_with(self.project_config.get_experiment_from_key('test_experiment'))
596671
self.assertEqual(0, mock_dispatch_event.call_count)
597672

673+
def test_track__whitelisted_user_overrides_audience_check(self):
674+
""" Test that track does not check for user in audience when user is in whitelist. """
675+
676+
with mock.patch('optimizely.helpers.experiment.is_experiment_running',
677+
return_value=True) as mock_is_experiment_running,\
678+
mock.patch('optimizely.helpers.experiment.is_user_in_forced_variation',
679+
return_value=True) as mock_whitelist_check,\
680+
mock.patch('optimizely.helpers.audience.is_user_in_experiment',
681+
return_value=False) as mock_audience_check,\
682+
mock.patch('time.time', return_value=42),\
683+
mock.patch('optimizely.event_dispatcher.EventDispatcher.dispatch_event') as mock_dispatch_event:
684+
self.optimizely.track('test_event', 'user_1')
685+
686+
mock_is_experiment_running.assert_called_once_with(self.project_config.get_experiment_from_key('test_experiment'))
687+
mock_whitelist_check.assert_called_once_with({'user_1': 'control', 'user_2': 'control'}, 'user_1')
688+
self.assertEqual(1, mock_dispatch_event.call_count)
689+
self.assertEqual(0, mock_audience_check.call_count)
690+
598691

599692
class OptimizelyWithExceptionTest(base.BaseTestV1):
600693

0 commit comments

Comments
 (0)