From 672df850cb8004f049e084b7ed159df3515761a7 Mon Sep 17 00:00:00 2001 From: Stephen Niedzielski Date: Thu, 25 Oct 2018 17:10:06 -0700 Subject: [PATCH] Hygiene: revise A/B test terminology Improve the comments and APIs provided by AB.js: - Control becomes unsampled. - A becomes control. - B becomes treatment. This code does not appear to be in use presently, so it's a great time to change it. Change-Id: I31d619f889ee45102a4aed774a6ec41f0d95ba7d --- resources/skins.minerva.scripts/AB.js | 54 ++++++++++---------- tests/qunit/skins.minerva.scripts/AB.test.js | 32 ++++++------ 2 files changed, 43 insertions(+), 43 deletions(-) diff --git a/resources/skins.minerva.scripts/AB.js b/resources/skins.minerva.scripts/AB.js index 2f2f95fbd..62f8eba4e 100644 --- a/resources/skins.minerva.scripts/AB.js +++ b/resources/skins.minerva.scripts/AB.js @@ -1,22 +1,22 @@ /* * Bucketing wrapper for creating AB-tests. * - * Given a test name, sampling rate, and session ID, provides a class that buckets users into - * predefined bucket ("control", "A", "B") and starts an AB-test. + * Given a test name, sampling rate, and session ID, provides a class that buckets a user into + * a predefined bucket ("unsampled", "control", or "treatment") and starts an AB-test. */ ( function ( M, mwExperiments ) { var bucket = { - CONTROL: 'control', - A: 'A', - B: 'B' + UNSAMPLED: 'unsampled', // Old treatment: not sampled and not instrumented. + CONTROL: 'control', // Old treatment: sampled and instrumented. + TREATMENT: 'treatment' // New treatment: sampled and instrumented. }; /** - * Buckets users based on params and exposes an `isEnabled` and `getBucket` method. - * @param {Object} config Configuration object for AB test. - * @param {string} config.testName - * @param {number} config.samplingRate Sampling rate for the AB-test. - * @param {number} config.sessionId Session ID for user bucketing. + * Buckets users based on params and exposes an `isSampled` and `getBucket` method. + * @param {Object} config Configuration object for AB test. + * @param {string} config.testName + * @param {number} config.samplingRate Sampling rate for the AB-test. + * @param {number} config.sessionId Session ID for user bucketing. * @constructor */ function AB( config ) { @@ -28,9 +28,9 @@ name: testName, enabled: !!samplingRate, buckets: { - control: 1 - samplingRate, - A: samplingRate / 2, - B: samplingRate / 2 + unsampled: 1 - samplingRate, + control: samplingRate / 2, + treatment: samplingRate / 2 } }; @@ -39,37 +39,37 @@ * * A boolean instead of an enum is usually a code smell. However, the nature of A/B testing * is to compare an A group's performance to a B group's so a boolean seems natural, even - * in the long term, and preferable to showing bucketing encoding ("A", "B", "control") to - * callers which is necessary if getBucket(). The downside is that now two functions exist - * where one would suffice. + * in the long term, and preferable to showing bucketing encoding ("unsampled", "control", + * "treatment") to callers which is necessary if getBucket(). The downside is that now two + * functions exist where one would suffice. * - * @return {string} AB-test bucket, bucket.CONTROL_BUCKET by default, bucket.A or bucket.B - * buckets otherwise. + * @return {string} AB-test bucket, `bucket.UNSAMPLED` by default, `bucket.CONTROL` or + * `bucket.TREATMENT` buckets otherwise. */ function getBucket() { return mwExperiments.getBucket( test, sessionId ); } - function isA() { - return getBucket() === bucket.A; + function isControl() { + return getBucket() === bucket.CONTROL; } - function isB() { - return getBucket() === bucket.B; + function isTreatment() { + return getBucket() === bucket.TREATMENT; } /** * Checks whether or not a user is in the AB-test, * @return {boolean} */ - function isEnabled() { - return getBucket() !== bucket.CONTROL; // I.e., `isA() || isB()`; + function isSampled() { + return getBucket() !== bucket.UNSAMPLED; // I.e., `isControl() || isTreatment()` } return { - isA: isA, - isB: isB, - isEnabled: isEnabled + isControl: isControl, + isTreatment: isTreatment, + isSampled: isSampled }; } diff --git a/tests/qunit/skins.minerva.scripts/AB.test.js b/tests/qunit/skins.minerva.scripts/AB.test.js index 9f8c4c0d4..d74d4059a 100644 --- a/tests/qunit/skins.minerva.scripts/AB.test.js +++ b/tests/qunit/skins.minerva.scripts/AB.test.js @@ -12,9 +12,9 @@ QUnit.test( 'Bucketing test', function ( assert ) { var userBuckets = { + unsampled: 0, control: 0, - A: 0, - B: 0 + treatment: 0 }, maxUsers = 1000, bucketingTest, @@ -26,31 +26,31 @@ sessionId: mw.user.generateRandomSessionId() } ); bucketingTest = new AB( config ); - if ( bucketingTest.isA() ) { - ++userBuckets.A; - } else if ( bucketingTest.isB() ) { - ++userBuckets.B; - } else if ( !bucketingTest.isEnabled() ) { + if ( bucketingTest.isControl() ) { ++userBuckets.control; + } else if ( bucketingTest.isTreatment() ) { + ++userBuckets.treatment; + } else if ( !bucketingTest.isSampled() ) { + ++userBuckets.unsampled; } else { throw new Error( 'Unknown bucket!' ); } } assert.strictEqual( - ( userBuckets.control / maxUsers > 0.4 ) && - ( userBuckets.control / maxUsers < 0.6 ), - true, 'test control group is about 50% (' + userBuckets.control / 10 + '%)' ); + ( userBuckets.unsampled / maxUsers > 0.4 ) && + ( userBuckets.unsampled / maxUsers < 0.6 ), + true, 'test unsampled group is about 50% (' + userBuckets.unsampled / 10 + '%)' ); assert.strictEqual( - ( userBuckets.A / maxUsers > 0.2 ) && - ( userBuckets.A / maxUsers < 0.3 ), - true, 'test group A is about 25% (' + userBuckets.A / 10 + '%)' ); + ( userBuckets.control / maxUsers > 0.2 ) && + ( userBuckets.control / maxUsers < 0.3 ), + true, 'test control group is about 25% (' + userBuckets.control / 10 + '%)' ); assert.strictEqual( - ( userBuckets.B / maxUsers > 0.2 ) && - ( userBuckets.B / maxUsers < 0.3 ), - true, 'test group B is about 25% (' + userBuckets.B / 10 + '%)' ); + ( userBuckets.treatment / maxUsers > 0.2 ) && + ( userBuckets.treatment / maxUsers < 0.3 ), + true, 'test new treatment group is about 25% (' + userBuckets.treatment / 10 + '%)' ); } ); }( mw.mobileFrontend ) );