-
Notifications
You must be signed in to change notification settings - Fork 0
Bucketing Feedback - Do not merge #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| public function setUp() | ||
| { | ||
| $this->testBucketingIdControl = 'testBucketingIdControl!'; // generates bucketing number 3741 | ||
| $this->testBucketingIdVariation = '123456789'; // generates bucketing number 4567 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't get how these Ids help in testing. The variables, at most times are passed in tests where we already mock bucket values by TestBucketer so these are ignored. At other times, where these ids do have an impact, we can not really predict the bucketing number because these values are concatenated with group id or experiment id (in a given test) before being passed to murmur hash method.
| ) | ||
| ); | ||
|
|
||
| // check that the null bucketing ID defaults to bucketing with the userId |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bucketer does not know that bucketing ID = user ID if bucketing ID is null. It's role is simply to bucket based on a given bucketing id. It's the decision service::getVariation which holds the logic for bucketing ID = user ID if bucketing ID is null.
This test isn't reporting an issue, because a null is considered as an empty string in php and when concatenated with the experiment id, it produces some number by chance which gives us our expected variation.
See Logs:
Optimizely.DEBUG: Assigned bucket 3218 to user "testUserId" with bucketing ID "testBucketingIdControl!" and with bucketing key "testBucketingIdControl!7716830082".
[2017-10-09 11:17:22] Optimizely.INFO: User "testUserId" is in variation control of experiment test_experiment.
[2017-10-09 11:17:22] Optimizely.DEBUG: Assigned bucket 7428 to user "testUserId" with bucketing ID "" and with bucketing key "7716830082".
[2017-10-09 11:17:22] Optimizely.INFO: User "testUserId" is in variation variation of experiment test_experiment.
| $variationKey = $optlyObject->getVariation($this->experimentKey, $this->testUserIdWhitelisted, $userAttributesWithBucketingId); | ||
| $this->assertEquals( $this->variationKeyControl, $variationKey); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think all of the new tests until this point, can go to OptimizelyTests or ProjectConfigTests files for better organization and maintenance.
No description provided.