Feature - Bucketing IDs #68
Conversation
* 🖊 Non - testing changes done * ✏️ Fixed broken unit tests due to method def change * ✏️ : Added debug messages. Will remove later * ✏️ Changes in previous unit tests * ✏️ Bucket Id unit tests * ✏️ : More development * 🖊 refactoring * 🖊 : final minor ticks
# Conflicts: # lib/optimizely/event_builder.rb
|
Can one of the admins verify this patch? |
lib/optimizely/bucketer.rb
Outdated
| # Determines ID of variation to be shown for a given experiment key and user ID. | ||
| # | ||
| # experiment - Experiment for which visitor is to be bucketed. | ||
| # bucketing_id - String A customer-assigned value used to generate bucketing key |
There was a problem hiding this comment.
nit: the bucketing key
| expect(bucketer.bucket(experiment,'123456789', 'test_user')).to be(expected_variation) | ||
| end | ||
| end | ||
|
|
There was a problem hiding this comment.
Please add the cases when the bucketing key fallbacks to the user ID. This happens when the bucketing key is null. So, to clarify:
• Bucketing with bucketing ID
-Non null bucketing ID
-Null bucketing ID
• Bucketing with invalid experiment key and bucketing ID
-Non null bucketing ID
-Null bucketing ID
• Bucketing with grouped experiments and bucketing ID
-Non null bucketing ID
-Null bucketing ID
I've updated the doc with these corner cases.
You can look at the PHP example:
https://github.com/optimizely/php-sdk/blob/master/tests/BucketerTest.php#L298
There was a problem hiding this comment.
@alda-optimizely I have a question regarding this. Please see my comments here:
msohailhussain/php-sdk#6
There was a problem hiding this comment.
@oakbani Yea you're right. I don't remember why I did that...maybe it was some legacy implementation that I forgot to update? The main goal for the test is to make sure that the bucketing ID is being used and not the userID for the bucketing. So, userID should bucket to the non expected variation. I've updated my PHP tests.
| @@ -75,9 +75,9 @@ def get_bucketing_id(user_id, entity_id=nil) | |||
| expect(bucketer).to receive(:generate_bucket_value).once.and_return(3000) | |||
|
|
|||
| experiment = config.get_experiment_from_key('group1_exp2') | |||
| expect(bucketer.bucket(experiment, 'test_user')).to be_nil | |||
| expect(bucketer.bucket(experiment,'bucket_id_ignored', 'test_user')).to be_nil | |||
There was a problem hiding this comment.
what does the bucketing key: bucketing_id_ignored do?
how is it ignored?
There was a problem hiding this comment.
I have used this string in all those cases where we mock the method generate_bucket_value to test traffic allocations as in above
expect(bucketer).to receive(:generate_bucket_value).twice.and_return(3000)
This makes it more readable to the developer that the string passed as bucketing id does not have an impact
spec/decision_service_spec.rb
Outdated
| @@ -162,6 +195,31 @@ | |||
| .with(Logger::INFO, "Saved variation ID 111128 of experiment ID 111127 for user 'test_user'.") | |||
| end | |||
|
|
|||
| it 'should look up the UserProfile, bucket normally (using Bucketing ID attrbiute), and save the result if no saved profile is found' do | |||
|
@alda-optimizely Updated with required changes. Let me know if there is anything else |
🖊 Please note that since data file in Ruby SDK is different from PHP-SDK, the resulting experiment/variation values and the flow of resulting log statements due to bucketing may differ in the unit tests.