Skip to content

Commit 6b8c85f

Browse files
raju-optiraju-opti
committed
bucketer fix
1 parent 1474e15 commit 6b8c85f

File tree

3 files changed

+40
-70
lines changed

3 files changed

+40
-70
lines changed

‎lib/core/bucketer/index.tests.js

Lines changed: 39 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -92,20 +92,13 @@ describe('lib/core/bucketer', function () {
9292
var decisionResponse = bucketer.bucket(bucketerParamsTest1);
9393
expect(decisionResponse.result).to.equal('111128');
9494

95-
var bucketedUser_log1 = buildLogMessageFromArgs(createdLogger.log.args[0]);
96-
expect(bucketedUser_log1).to.equal(
97-
sprintf(USER_ASSIGNED_TO_EXPERIMENT_BUCKET, 'BUCKETER', '50', 'ppid1')
98-
);
95+
expect(createdLogger.debug.args[0]).to.deep.equal([USER_ASSIGNED_TO_EXPERIMENT_BUCKET, 50, 'ppid1']);
9996

10097
var bucketerParamsTest2 = cloneDeep(bucketerParams);
10198
bucketerParamsTest2.userId = 'ppid2';
10299
expect(bucketer.bucket(bucketerParamsTest2).result).to.equal(null);
103100

104-
var notBucketedUser_log1 = buildLogMessageFromArgs(createdLogger.log.args[1]);
105-
106-
expect(notBucketedUser_log1).to.equal(
107-
sprintf(USER_ASSIGNED_TO_EXPERIMENT_BUCKET, 'BUCKETER', '50000', 'ppid2')
108-
);
101+
expect(createdLogger.debug.args[1]).to.deep.equal([USER_ASSIGNED_TO_EXPERIMENT_BUCKET, 50000, 'ppid2']);
109102
});
110103
});
111104

@@ -152,28 +145,14 @@ describe('lib/core/bucketer', function () {
152145
expect(decisionResponse.result).to.equal('551');
153146

154147
sinon.assert.calledTwice(bucketerStub);
155-
sinon.assert.callCount(createdLogger.log, 3);
156-
157-
var log1 = buildLogMessageFromArgs(createdLogger.log.args[0]);
158-
expect(log1).to.equal(
159-
sprintf(USER_ASSIGNED_TO_EXPERIMENT_BUCKET, 'BUCKETER', '50', 'testUser')
160-
);
161-
162-
var log2 = buildLogMessageFromArgs(createdLogger.log.args[1]);
163-
expect(log2).to.equal(
164-
sprintf(
165-
USER_BUCKETED_INTO_EXPERIMENT_IN_GROUP,
166-
'BUCKETER',
167-
'testUser',
168-
'groupExperiment1',
169-
'666'
170-
)
171-
);
172-
173-
var log3 = buildLogMessageFromArgs(createdLogger.log.args[2]);
174-
expect(log3).to.equal(
175-
sprintf(USER_ASSIGNED_TO_EXPERIMENT_BUCKET, 'BUCKETER', '50', 'testUser')
176-
);
148+
sinon.assert.callCount(createdLogger.debug, 2);
149+
sinon.assert.callCount(createdLogger.info, 1);
150+
151+
expect(createdLogger.debug.args[0]).to.deep.equal([USER_ASSIGNED_TO_EXPERIMENT_BUCKET, 50, 'testUser']);
152+
153+
expect(createdLogger.info.args[0]).to.deep.equal([USER_BUCKETED_INTO_EXPERIMENT_IN_GROUP, 'testUser', 'groupExperiment1', '666']);
154+
155+
expect(createdLogger.debug.args[1]).to.deep.equal([USER_ASSIGNED_TO_EXPERIMENT_BUCKET, 50, 'testUser']);
177156
});
178157

179158
it('should return decision response with variation null when a user is bucketed into a different grouped experiment than the one speicfied', function () {
@@ -183,22 +162,12 @@ describe('lib/core/bucketer', function () {
183162
expect(decisionResponse.result).to.equal(null);
184163

185164
sinon.assert.calledOnce(bucketerStub);
186-
sinon.assert.calledTwice(createdLogger.log);
187-
188-
var log1 = buildLogMessageFromArgs(createdLogger.log.args[0]);
189-
expect(log1).to.equal(
190-
sprintf(USER_ASSIGNED_TO_EXPERIMENT_BUCKET, 'BUCKETER', '5000', 'testUser')
191-
);
192-
var log2 = buildLogMessageFromArgs(createdLogger.log.args[1]);
193-
expect(log2).to.equal(
194-
sprintf(
195-
USER_NOT_BUCKETED_INTO_EXPERIMENT_IN_GROUP,
196-
'BUCKETER',
197-
'testUser',
198-
'groupExperiment1',
199-
'666'
200-
)
201-
);
165+
sinon.assert.calledOnce(createdLogger.debug);
166+
sinon.assert.calledOnce(createdLogger.info);
167+
168+
expect(createdLogger.debug.args[0]).to.deep.equal([USER_ASSIGNED_TO_EXPERIMENT_BUCKET, 5000, 'testUser']);
169+
170+
expect(createdLogger.info.args[0]).to.deep.equal([USER_NOT_BUCKETED_INTO_EXPERIMENT_IN_GROUP, 'testUser', 'groupExperiment1', '666']);
202171
});
203172

204173
it('should return decision response with variation null when a user is not bucketed into any experiments in the random group', function () {
@@ -208,14 +177,12 @@ describe('lib/core/bucketer', function () {
208177
expect(decisionResponse.result).to.equal(null);
209178

210179
sinon.assert.calledOnce(bucketerStub);
211-
sinon.assert.calledTwice(createdLogger.log);
212-
213-
var log1 = buildLogMessageFromArgs(createdLogger.log.args[0]);
214-
expect(log1).to.equal(
215-
sprintf(USER_ASSIGNED_TO_EXPERIMENT_BUCKET, 'BUCKETER', '50000', 'testUser')
216-
);
217-
var log2 = buildLogMessageFromArgs(createdLogger.log.args[1]);
218-
expect(log2).to.equal(sprintf(USER_NOT_IN_ANY_EXPERIMENT, 'BUCKETER', 'testUser', '666'));
180+
sinon.assert.calledOnce(createdLogger.debug);
181+
sinon.assert.calledOnce(createdLogger.info);
182+
183+
expect(createdLogger.debug.args[0]).to.deep.equal([USER_ASSIGNED_TO_EXPERIMENT_BUCKET, 50000, 'testUser']);
184+
185+
expect(createdLogger.info.args[0]).to.deep.equal([USER_NOT_IN_ANY_EXPERIMENT, 'testUser', '666']);
219186
});
220187

221188
it('should return decision response with variation null when a user is bucketed into traffic space of deleted experiment within a random group', function () {
@@ -225,14 +192,12 @@ describe('lib/core/bucketer', function () {
225192
expect(decisionResponse.result).to.equal(null);
226193

227194
sinon.assert.calledOnce(bucketerStub);
228-
sinon.assert.calledTwice(createdLogger.log);
229-
230-
var log1 = buildLogMessageFromArgs(createdLogger.log.args[0]);
231-
expect(log1).to.equal(
232-
sprintf(USER_ASSIGNED_TO_EXPERIMENT_BUCKET, 'BUCKETER', '9000', 'testUser')
233-
);
234-
var log2 = buildLogMessageFromArgs(createdLogger.log.args[1]);
235-
expect(log2).to.equal(sprintf(USER_NOT_IN_ANY_EXPERIMENT, 'BUCKETER', 'testUser', '666'));
195+
sinon.assert.calledOnce(createdLogger.debug);
196+
sinon.assert.calledOnce(createdLogger.info);
197+
198+
expect(createdLogger.debug.args[0]).to.deep.equal([USER_ASSIGNED_TO_EXPERIMENT_BUCKET, 9000, 'testUser']);
199+
200+
expect(createdLogger.info.args[0]).to.deep.equal([USER_NOT_IN_ANY_EXPERIMENT, 'testUser', '666']);
236201
});
237202

238203
it('should throw an error if group ID is not in the datafile', function () {
@@ -267,10 +232,9 @@ describe('lib/core/bucketer', function () {
267232
expect(decisionResponse.result).to.equal('553');
268233

269234
sinon.assert.calledOnce(bucketerStub);
270-
sinon.assert.calledOnce(createdLogger.log);
235+
sinon.assert.calledOnce(createdLogger.debug);
271236

272-
var log1 = buildLogMessageFromArgs(createdLogger.log.args[0]);
273-
expect(log1).to.equal(sprintf(USER_ASSIGNED_TO_EXPERIMENT_BUCKET, 'BUCKETER', '0', 'testUser'));
237+
expect(createdLogger.debug.args[0]).to.deep.equal([USER_ASSIGNED_TO_EXPERIMENT_BUCKET, 0, 'testUser']);
274238
});
275239

276240
it('should return decision response with variation null when a user does not fall into an experiment within an overlapping group', function () {
@@ -314,8 +278,15 @@ describe('lib/core/bucketer', function () {
314278

315279
it('should not log an invalid variation ID warning', function () {
316280
bucketer.bucket(bucketerParams)
317-
const foundInvalidVariationWarning = createdLogger.log.getCalls().some((call) => {
318-
const message = call.args[1];
281+
const calls = [
282+
...createdLogger.debug.getCalls(),
283+
...createdLogger.info.getCalls(),
284+
...createdLogger.warn.getCalls(),
285+
...createdLogger.error.getCalls(),
286+
];
287+
288+
const foundInvalidVariationWarning = calls.some((call) => {
289+
const message = call.args[0];
319290
return message.includes('Bucketed into an invalid variation ID')
320291
});
321292
expect(foundInvalidVariationWarning).to.equal(false);

‎lib/core/bucketer/index.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,6 @@ export const bucket = function(bucketerParams: BucketerParams): DecisionResponse
8080
if (bucketedExperimentId === null) {
8181
bucketerParams.logger?.info(
8282
USER_NOT_IN_ANY_EXPERIMENT,
83-
MODULE_NAME,
8483
bucketerParams.userId,
8584
groupId,
8685
);

‎package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@
7474
"clean:win": "(if exist dist rd /s/q dist)",
7575
"lint": "tsc --noEmit && eslint 'lib/**/*.js' 'lib/**/*.ts'",
7676
"test-vitest": "tsc --noEmit --p tsconfig.spec.json && vitest run",
77-
"test-mocha": "TS_NODE_COMPILER_OPTIONS='{\"module\": \"commonjs\" }' mocha -r ts-node/register -r lib/tests/exit_on_unhandled_rejection.js 'lib/**/*.tests.ts' 'lib/**/event_tag_utils/*.tests.js'",
77+
"test-mocha": "TS_NODE_COMPILER_OPTIONS='{\"module\": \"commonjs\" }' mocha -r ts-node/register -r lib/tests/exit_on_unhandled_rejection.js 'lib/**/*.tests.ts' 'lib/**/bucketer/*.tests.js'",
7878
"test": "npm run test-mocha && npm run test-vitest",
7979
"posttest": "npm run lint",
8080
"test-ci": "npm run test-xbrowser && npm run test-umdbrowser",

0 commit comments

Comments
 (0)