Skip to content

Commit c6c2b48

Browse files
aliabbasrizvialiabbasrizvi
authored
Merge branch 'master' into oakbani/decision-is-feat-enabled
2 parents 638de4a + 1f34b2a commit c6c2b48

File tree

4 files changed

+229
-14
lines changed

4 files changed

+229
-14
lines changed

‎optimizely/optimizely.py

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -208,12 +208,25 @@ def _get_feature_variable_for_type(self, feature_key, variable_key, variable_typ
208208
)
209209
return None
210210

211+
variable_value = variable.defaultValue
212+
211213
decision = self.decision_service.get_variation_for_feature(feature_flag, user_id, attributes)
212214
if decision.variation:
213-
variable_value = self.config.get_variable_value_for_variation(variable, decision.variation)
214215

216+
feature_enabled = decision.variation.featureEnabled
217+
if feature_enabled:
218+
variable_value = self.config.get_variable_value_for_variation(variable, decision.variation)
219+
self.logger.info(
220+
'Got variable value "%s" for variable "%s" of feature flag "%s".' % (
221+
variable_value, variable_key, feature_key
222+
)
223+
)
224+
else:
225+
self.logger.info(
226+
'Feature "%s" for variation "%s" is not enabled. '
227+
'Returning the default variable value "%s".' % (feature_key, decision.variation.key, variable_value)
228+
)
215229
else:
216-
variable_value = variable.defaultValue
217230
self.logger.info(
218231
'User "%s" is not in any variation or rollout rule. '
219232
'Returning default value for variable "%s" of feature flag "%s".' % (user_id, variable_key, feature_key)

‎tests/base.py

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# Copyright 2016-2018, Optimizely
1+
# Copyright 2016-2019, Optimizely
22
# Licensed under the Apache License, Version 2.0 (the "License");
33
# you may not use this file except in compliance with the License.
44
# You may obtain a copy of the License at
@@ -307,7 +307,29 @@ def setUp(self, config_dict='config_dict'):
307307
'variations': [{
308308
'key': '211129',
309309
'id': '211129',
310-
'featureEnabled': True
310+
'featureEnabled': True,
311+
'variables': [{
312+
'id': '132', 'value': 'true'
313+
}, {
314+
'id': '135', 'value': '395'
315+
}, {
316+
'id': '134', 'value': '39.99'
317+
}, {
318+
'id': '133', 'value': 'Hello audience'
319+
}]
320+
}, {
321+
'key': '211229',
322+
'id': '211229',
323+
'featureEnabled': False,
324+
'variables': [{
325+
'id': '132', 'value': 'true'
326+
}, {
327+
'id': '135', 'value': '395'
328+
}, {
329+
'id': '134', 'value': '39.99'
330+
}, {
331+
'id': '133', 'value': 'Hello audience'
332+
}]
311333
}]
312334
}, {
313335
'id': '211137',
@@ -379,7 +401,27 @@ def setUp(self, config_dict='config_dict'):
379401
'key': 'test_feature_in_rollout',
380402
'experimentIds': [],
381403
'rolloutId': '211111',
382-
'variables': [],
404+
'variables': [{
405+
'id': '132',
406+
'key': 'is_running',
407+
'defaultValue': 'false',
408+
'type': 'boolean'
409+
}, {
410+
'id': '133',
411+
'key': 'message',
412+
'defaultValue': 'Hello',
413+
'type': 'string'
414+
}, {
415+
'id': '134',
416+
'key': 'price',
417+
'defaultValue': '99.99',
418+
'type': 'double'
419+
}, {
420+
'id': '135',
421+
'key': 'count',
422+
'defaultValue': '999',
423+
'type': 'integer'
424+
}]
383425
}, {
384426
'id': '91113',
385427
'key': 'test_feature_in_group',

‎tests/test_config.py

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# Copyright 2016-2018, Optimizely
1+
# Copyright 2016-2019, Optimizely
22
# Licensed under the Apache License, Version 2.0 (the "License");
33
# you may not use this file except in compliance with the License.
44
# You may obtain a copy of the License at
@@ -885,7 +885,19 @@ def test_get_feature_from_key__valid_feature_key(self):
885885
opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
886886
project_config = opt_obj.config
887887

888-
expected_feature = entities.FeatureFlag('91112', 'test_feature_in_rollout', [], '211111', {})
888+
expected_feature = entities.FeatureFlag(
889+
'91112',
890+
'test_feature_in_rollout',
891+
[],
892+
'211111',
893+
{
894+
'is_running': entities.Variable('132', 'is_running', 'boolean', 'false'),
895+
'message': entities.Variable('133', 'message', 'string', 'Hello'),
896+
'price': entities.Variable('134', 'price', 'double', '99.99'),
897+
'count': entities.Variable('135', 'count', 'integer', '999')
898+
}
899+
)
900+
889901
self.assertEqual(expected_feature, project_config.get_feature_from_key('test_feature_in_rollout'))
890902

891903
def test_get_feature_from_key__invalid_feature_key(self):
@@ -916,7 +928,29 @@ def test_get_rollout_from_id__valid_rollout_id(self):
916928
'variations': [{
917929
'key': '211129',
918930
'id': '211129',
919-
'featureEnabled': True
931+
'featureEnabled': True,
932+
'variables': [{
933+
'id': '132', 'value': 'true'
934+
}, {
935+
'id': '135', 'value': '395'
936+
}, {
937+
'id': '134', 'value': '39.99'
938+
}, {
939+
'id': '133', 'value': 'Hello audience'
940+
}]
941+
}, {
942+
'key': '211229',
943+
'id': '211229',
944+
'featureEnabled': False,
945+
'variables': [{
946+
'id': '132', 'value': 'true'
947+
}, {
948+
'id': '135', 'value': '395'
949+
}, {
950+
'id': '134', 'value': '39.99'
951+
}, {
952+
'id': '133', 'value': 'Hello audience'
953+
}]
920954
}]
921955
}, {
922956
'id': '211137',

‎tests/test_optimizely.py

Lines changed: 132 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2397,6 +2397,122 @@ def test_get_feature_variable__returns_none_if_invalid_variable_key(self):
23972397
mock.call('Variable with key "invalid_variable" not found in the datafile.')
23982398
])
23992399

2400+
def test_get_feature_variable__returns_default_value_if_feature_not_enabled(self):
2401+
""" Test that get_feature_variable_* returns default value if feature is not enabled for the user. """
2402+
2403+
opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
2404+
mock_experiment = opt_obj.config.get_experiment_from_key('test_experiment')
2405+
mock_variation = opt_obj.config.get_variation_from_id('test_experiment', '111128')
2406+
2407+
# Boolean
2408+
with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature',
2409+
return_value=decision_service.Decision(mock_experiment, mock_variation,
2410+
decision_service.DECISION_SOURCE_EXPERIMENT)), \
2411+
mock.patch.object(opt_obj, 'logger') as mock_client_logger:
2412+
2413+
self.assertTrue(opt_obj.get_feature_variable_boolean('test_feature_in_experiment', 'is_working', 'test_user'))
2414+
2415+
mock_client_logger.info.assert_called_once_with(
2416+
'Feature "test_feature_in_experiment" for variation "control" is not enabled. '
2417+
'Returning the default variable value "true".'
2418+
)
2419+
2420+
# Double
2421+
with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature',
2422+
return_value=decision_service.Decision(mock_experiment, mock_variation,
2423+
decision_service.DECISION_SOURCE_EXPERIMENT)), \
2424+
mock.patch.object(opt_obj, 'logger') as mock_client_logger:
2425+
self.assertEqual(10.99,
2426+
opt_obj.get_feature_variable_double('test_feature_in_experiment', 'cost', 'test_user'))
2427+
2428+
mock_client_logger.info.assert_called_once_with(
2429+
'Feature "test_feature_in_experiment" for variation "control" is not enabled. '
2430+
'Returning the default variable value "10.99".'
2431+
)
2432+
2433+
# Integer
2434+
with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature',
2435+
return_value=decision_service.Decision(mock_experiment, mock_variation,
2436+
decision_service.DECISION_SOURCE_EXPERIMENT)), \
2437+
mock.patch.object(opt_obj, 'logger') as mock_client_logger:
2438+
self.assertEqual(999,
2439+
opt_obj.get_feature_variable_integer('test_feature_in_experiment', 'count', 'test_user'))
2440+
2441+
mock_client_logger.info.assert_called_once_with(
2442+
'Feature "test_feature_in_experiment" for variation "control" is not enabled. '
2443+
'Returning the default variable value "999".'
2444+
)
2445+
2446+
# String
2447+
with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature',
2448+
return_value=decision_service.Decision(mock_experiment, mock_variation,
2449+
decision_service.DECISION_SOURCE_EXPERIMENT)), \
2450+
mock.patch.object(opt_obj, 'logger') as mock_client_logger:
2451+
self.assertEqual('devel',
2452+
opt_obj.get_feature_variable_string('test_feature_in_experiment', 'environment', 'test_user'))
2453+
2454+
mock_client_logger.info.assert_called_once_with(
2455+
'Feature "test_feature_in_experiment" for variation "control" is not enabled. '
2456+
'Returning the default variable value "devel".'
2457+
)
2458+
2459+
def test_get_feature_variable__returns_default_value_if_feature_not_enabled_in_rollout(self):
2460+
""" Test that get_feature_variable_* returns default value if feature is not enabled for the user. """
2461+
2462+
opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
2463+
mock_experiment = opt_obj.config.get_experiment_from_key('211127')
2464+
mock_variation = opt_obj.config.get_variation_from_id('211127', '211229')
2465+
2466+
# Boolean
2467+
with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature',
2468+
return_value=decision_service.Decision(mock_experiment, mock_variation,
2469+
decision_service.DECISION_SOURCE_ROLLOUT)), \
2470+
mock.patch.object(opt_obj, 'logger') as mock_client_logger:
2471+
self.assertFalse(opt_obj.get_feature_variable_boolean('test_feature_in_rollout', 'is_running', 'test_user'))
2472+
2473+
mock_client_logger.info.assert_called_once_with(
2474+
'Feature "test_feature_in_rollout" for variation "211229" is not enabled. '
2475+
'Returning the default variable value "false".'
2476+
)
2477+
2478+
# Double
2479+
with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature',
2480+
return_value=decision_service.Decision(mock_experiment, mock_variation,
2481+
decision_service.DECISION_SOURCE_ROLLOUT)), \
2482+
mock.patch.object(opt_obj, 'logger') as mock_client_logger:
2483+
self.assertEqual(99.99,
2484+
opt_obj.get_feature_variable_double('test_feature_in_rollout', 'price', 'test_user'))
2485+
2486+
mock_client_logger.info.assert_called_once_with(
2487+
'Feature "test_feature_in_rollout" for variation "211229" is not enabled. '
2488+
'Returning the default variable value "99.99".'
2489+
)
2490+
2491+
# Integer
2492+
with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature',
2493+
return_value=decision_service.Decision(mock_experiment, mock_variation,
2494+
decision_service.DECISION_SOURCE_ROLLOUT)), \
2495+
mock.patch.object(opt_obj, 'logger') as mock_client_logger:
2496+
self.assertEqual(999,
2497+
opt_obj.get_feature_variable_integer('test_feature_in_rollout', 'count', 'test_user'))
2498+
2499+
mock_client_logger.info.assert_called_once_with(
2500+
'Feature "test_feature_in_rollout" for variation "211229" is not enabled. '
2501+
'Returning the default variable value "999".'
2502+
)
2503+
2504+
# String
2505+
with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature',
2506+
return_value=decision_service.Decision(mock_experiment, mock_variation,
2507+
decision_service.DECISION_SOURCE_ROLLOUT)), \
2508+
mock.patch.object(opt_obj, 'logger') as mock_client_logger:
2509+
self.assertEqual('Hello',
2510+
opt_obj.get_feature_variable_string('test_feature_in_rollout', 'message', 'test_user'))
2511+
mock_client_logger.info.assert_called_once_with(
2512+
'Feature "test_feature_in_rollout" for variation "211229" is not enabled. '
2513+
'Returning the default variable value "Hello".'
2514+
)
2515+
24002516
def test_get_feature_variable__returns_none_if_type_mismatch(self):
24012517
""" Test that get_feature_variable_* returns None if type mismatch. """
24022518

@@ -2439,15 +2555,25 @@ def test_get_feature_variable_returns__variable_value__typed_audience_match(self
24392555
opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_typed_audiences))
24402556

24412557
# Should be included in the feature test via greater-than match audience with id '3468206647'
2442-
self.assertEqual(
2443-
'xyz',
2444-
opt_obj.get_feature_variable_string('feat_with_var', 'x', 'user1', {'lasers': 71})
2558+
with mock.patch.object(opt_obj, 'logger') as mock_client_logger:
2559+
self.assertEqual(
2560+
'xyz',
2561+
opt_obj.get_feature_variable_string('feat_with_var', 'x', 'user1', {'lasers': 71})
2562+
)
2563+
2564+
mock_client_logger.info.assert_called_once_with(
2565+
'Got variable value "xyz" for variable "x" of feature flag "feat_with_var".'
24452566
)
24462567

24472568
# Should be included in the feature test via exact match boolean audience with id '3468206643'
2448-
self.assertEqual(
2449-
'xyz',
2450-
opt_obj.get_feature_variable_string('feat_with_var', 'x', 'user1', {'should_do_it': True})
2569+
with mock.patch.object(opt_obj, 'logger') as mock_client_logger:
2570+
self.assertEqual(
2571+
'xyz',
2572+
opt_obj.get_feature_variable_string('feat_with_var', 'x', 'user1', {'should_do_it': True})
2573+
)
2574+
2575+
mock_client_logger.info.assert_called_once_with(
2576+
'Got variable value "xyz" for variable "x" of feature flag "feat_with_var".'
24512577
)
24522578

24532579
def test_get_feature_variable_returns__default_value__typed_audience_match(self):

0 commit comments

Comments
 (0)