Skip to content

Commit ad1e98a

Browse files
msohailhussainmikeproeng37
authored andcommitted
Rashid/bucket variation bug (#107)
1 parent c0123f6 commit ad1e98a

File tree

2 files changed

+15
-23
lines changed

2 files changed

+15
-23
lines changed

src/Optimizely/Bucketer.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<?php
22
/**
3-
* Copyright 2016-2017, Optimizely
3+
* Copyright 2016-2018, Optimizely
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
66
* you may not use this file except in compliance with the License.
@@ -127,21 +127,21 @@ private function findBucket($bucketingId, $userId, $parentId, $trafficAllocation
127127
public function bucket(ProjectConfig $config, Experiment $experiment, $bucketingId, $userId)
128128
{
129129
if (is_null($experiment->getKey())) {
130-
return new Variation();
130+
return null;
131131
}
132132

133133
// Determine if experiment is in a mutually exclusive group.
134134
if ($experiment->isInMutexGroup()) {
135135
$group = $config->getGroup($experiment->getGroupId());
136136

137137
if (is_null($group->getId())) {
138-
return new Variation();
138+
return null;
139139
}
140140

141141
$userExperimentId = $this->findBucket($bucketingId, $userId, $group->getId(), $group->getTrafficAllocation());
142142
if (empty($userExperimentId)) {
143143
$this->_logger->log(Logger::INFO, sprintf('User "%s" is in no experiment.', $userId));
144-
return new Variation();
144+
return null;
145145
}
146146

147147
if ($userExperimentId != $experiment->getId()) {
@@ -154,7 +154,7 @@ public function bucket(ProjectConfig $config, Experiment $experiment, $bucketing
154154
$experiment->getGroupId()
155155
)
156156
);
157-
return new Variation();
157+
return null;
158158
}
159159

160160
$this->_logger->log(
@@ -186,6 +186,6 @@ public function bucket(ProjectConfig $config, Experiment $experiment, $bucketing
186186
}
187187

188188
$this->_logger->log(Logger::INFO, sprintf('User "%s" is in no variation.', $userId));
189-
return new Variation();
189+
return null;
190190
}
191191
}

tests/BucketerTest.php

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<?php
22
/**
3-
* Copyright 2016-2017, Optimizely
3+
* Copyright 2016-2018, Optimizely
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
66
* you may not use this file except in compliance with the License.
@@ -107,8 +107,7 @@ public function testBucketValidExperimentNotInGroup()
107107
->method('log')
108108
->with(Logger::INFO, 'User "testUserId" is in no variation.');
109109

110-
$this->assertEquals(
111-
new Variation(),
110+
$this->assertNull(
112111
$bucketer->bucket(
113112
$this->config,
114113
$this->config->getExperimentFromKey('test_experiment'),
@@ -167,8 +166,7 @@ public function testBucketValidExperimentNotInGroup()
167166
->method('log')
168167
->with(Logger::INFO, 'User "testUserId" is in no variation.');
169168

170-
$this->assertEquals(
171-
new Variation(),
169+
$this->assertNull(
172170
$bucketer->bucket(
173171
$this->config,
174172
$this->config->getExperimentFromKey('test_experiment'),
@@ -271,8 +269,7 @@ public function testBucketValidExperimentInGroup()
271269
->method('log')
272270
->with(Logger::INFO, 'User "testUserId" is not in experiment group_experiment_1 of group 7722400015.');
273271

274-
$this->assertEquals(
275-
new Variation(),
272+
$this->assertNull(
276273
$bucketer->bucket(
277274
$this->config,
278275
$this->config->getExperimentFromKey('group_experiment_1'),
@@ -290,8 +287,7 @@ public function testBucketValidExperimentInGroup()
290287
->method('log')
291288
->with(Logger::INFO, 'User "testUserId" is in no experiment.');
292289

293-
$this->assertEquals(
294-
new Variation(),
290+
$this->assertNull(
295291
$bucketer->bucket(
296292
$this->config,
297293
$this->config->getExperimentFromKey('group_experiment_1'),
@@ -308,8 +304,7 @@ public function testBucketValidExperimentInGroup()
308304
$this->loggerMock->expects($this->at(1))
309305
->method('log')
310306
->with(Logger::INFO, 'User "testUserId" is in no experiment.');
311-
$this->assertEquals(
312-
new Variation(),
307+
$this->assertNull(
313308
$bucketer->bucket(
314309
$this->config,
315310
$this->config->getExperimentFromKey('group_experiment_1'),
@@ -325,8 +320,7 @@ public function testBucketInvalidExperiment()
325320
$this->loggerMock->expects($this->never())
326321
->method('log');
327322

328-
$this->assertEquals(
329-
new Variation(),
323+
$this->assertNull(
330324
$bucketer->bucket($this->config, new Experiment(), $this->testBucketingIdControl, $this->testUserId)
331325
);
332326
}
@@ -356,8 +350,7 @@ public function testBucketVariationInvalidExperimentsWithBucketingId()
356350
$bucketer = new TestBucketer($this->loggerMock);
357351
$bucketer->setBucketValues([1000, 3000, 7000, 9000]);
358352

359-
$this->assertEquals(
360-
new Variation(),
353+
$this->assertNull(
361354
$bucketer->bucket(
362355
$this->config,
363356
$this->config->getExperimentFromKey('invalid_experiment'),
@@ -430,8 +423,7 @@ public function testBucketWithRolloutRule()
430423
)
431424
);
432425

433-
$this->assertEquals(
434-
new Variation(),
426+
$this->assertNull(
435427
$bucketer->bucket(
436428
$this->config,
437429
$rollout_rule,

0 commit comments

Comments
 (0)