diff --git a/resources/dist/index.js b/resources/dist/index.js index 53007caa9..6e21c941a 100644 Binary files a/resources/dist/index.js and b/resources/dist/index.js differ diff --git a/resources/dist/index.js.map b/resources/dist/index.js.map index 39a0fdfc6..e22692104 100644 Binary files a/resources/dist/index.js.map and b/resources/dist/index.js.map differ diff --git a/src/experiments.js b/src/experiments.js new file mode 100644 index 000000000..e902b8a7c --- /dev/null +++ b/src/experiments.js @@ -0,0 +1,55 @@ +/** + * @module experiments + */ + +/** + * @interface Experiments + * + * @global + */ + +/** + * Creates a helper wrapper for the MediaWiki-provided + * `mw.experiments#getBucket` bucketing function. + * + * @param {mw.experiments} mwExperiments The `mw.experiments` singleton instance + * @return {Experiments} + */ +module.exports = function createExperiments( mwExperiments ) { + return { + + /** + * Gets whether something is true given a name and a token. + * + * @example + * const experiments = require( './src/experiments' )( mw.experiments ); + * const isFooEnabled = experiments.weightedBoolean( + * 'foo', + * 10 / 100, // 10% of all unique tokens should have foo enabled. + * token + * ); + * + * @function + * @name Experiments#weightedBoolean + * @param {String} name The name of the thing. Since this is used as the + * name of the underlying experiment it should be unique to reduce the + * likelihood of collisions with other enabled experiments + * @param {Number} trueWeight A number between 0 and 1, representing the + * probability of the thing being true + * @param {String} token A token associated with the user for the duration + * of the experiment + * @return {Boolean} + */ + weightedBoolean: function ( name, trueWeight, token ) { + return mwExperiments.getBucket( { + enabled: true, + + name: name, + buckets: { + 'true': trueWeight, + 'false': 1 - trueWeight + } + }, token ) === 'true'; + } + }; +}; diff --git a/src/index.js b/src/index.js index b011658a9..59892f8e5 100644 --- a/src/index.js +++ b/src/index.js @@ -17,6 +17,7 @@ var mw = mediaWiki, createIsEnabled = require( './isEnabled' ), title = require( './title' ), renderer = require( './renderer' ), + createExperiments = require( './experiments' ), statsvInstrumentation = require( './statsvInstrumentation' ), changeListeners = require( './changeListeners' ), @@ -116,6 +117,7 @@ mw.requestIdleCallback( function () { gateway = createGateway( mw.config ), userSettings, settingsDialog, + experiments, statsvTracker, isEnabled, schema, @@ -123,7 +125,8 @@ mw.requestIdleCallback( function () { userSettings = createUserSettings( mw.storage ); settingsDialog = createSettingsDialogRenderer(); - statsvTracker = getStatsvTracker( mw.user, mw.config, mw.experiments ); + experiments = createExperiments( mw.experiments ); + statsvTracker = getStatsvTracker( mw.user, mw.config, experiments ); isEnabled = createIsEnabled( mw.user, userSettings, mw.config, mw.experiments ); diff --git a/src/statsvInstrumentation.js b/src/statsvInstrumentation.js index 52bea2ffe..747ff78f4 100644 --- a/src/statsvInstrumentation.js +++ b/src/statsvInstrumentation.js @@ -4,26 +4,22 @@ /** * Gets whether Graphite logging (via [the statsv HTTP endpoint][0]) is enabled - * for duration of the browser session. The sampling rate is controlled by + * for the duration of the user's session. The bucketing rate is controlled by * `wgPopupsStatsvSamplingRate`. * * [0]: https://wikitech.wikimedia.org/wiki/Graphite#statsv * * @param {mw.user} user The `mw.user` singleton instance * @param {mw.Map} config The `mw.config` singleton instance - * @param {mw.experiments} experiments The `mw.experiments` singleton instance + * @param {Experiments} experiments * @returns {Boolean} */ exports.isEnabled = function isEnabled( user, config, experiments ) { - var samplingRate = config.get( 'wgPopupsStatsvSamplingRate', 0 ), - bucket = experiments.getBucket( { - name: 'ext.Popups.statsv', - enabled: true, - buckets: { - control: 1 - samplingRate, - A: samplingRate - } - }, user.sessionId() ); + var bucketingRate = config.get( 'wgPopupsStatsvSamplingRate', 0 ); - return bucket === 'A'; + return experiments.weightedBoolean( + 'ext.Popups.statsv', + bucketingRate, + user.sessionId() + ); }; diff --git a/tests/node-qunit/experiments.test.js b/tests/node-qunit/experiments.test.js new file mode 100644 index 000000000..f64586f44 --- /dev/null +++ b/tests/node-qunit/experiments.test.js @@ -0,0 +1,39 @@ +var createExperiments = require( '../../src/experiments' ); + +QUnit.module( 'ext.popups/experiments#weightedBoolean' ); + +QUnit.test( 'it should call mw.experiments#getBucket', function ( assert ) { + var getBucketStub = this.sandbox.stub(), + stubMWExperiments = { + getBucket: getBucketStub + }, + experiments = createExperiments( stubMWExperiments ); + + experiments.weightedBoolean( 'foo', 0.2, 'barbaz' ); + + assert.ok( getBucketStub.calledOnce ); + assert.deepEqual( + getBucketStub.getCall( 0 ).args, + [ + { + enabled: true, + + name: 'foo', + buckets: { + 'true': 0.2, + 'false': 0.8 // 1 - 0.2 + } + }, + 'barbaz' + ] + ); + + // --- + + getBucketStub.returns( 'true' ); + + assert.ok( + experiments.weightedBoolean( 'foo', 0.2, 'barbaz' ), + 'It should return true if the bucket is "true".' + ); +} ); diff --git a/tests/node-qunit/statsvInstrumentation.test.js b/tests/node-qunit/statsvInstrumentation.test.js index e140f54e2..bbeb6044b 100644 --- a/tests/node-qunit/statsvInstrumentation.test.js +++ b/tests/node-qunit/statsvInstrumentation.test.js @@ -1,27 +1,39 @@ var stubs = require( './stubs' ), statsv = require( '../../src/statsvInstrumentation' ); -QUnit.module( 'ext.popups/statsvInstrumentation', { - beforeEach: function () { - this.user = stubs.createStubUser(); - this.config = stubs.createStubMap(); - } -} ); +QUnit.module( 'ext.popups/statsvInstrumentation' ); -QUnit.test( 'isEnabled', function ( assert ) { - var experiments = stubs.createStubExperiments( true ); +QUnit.test( '#isEnabled', function ( assert ) { + var user = stubs.createStubUser(), + config = stubs.createStubMap(), + weightedBooleanStub = this.sandbox.stub(), + experiments = { + weightedBoolean: weightedBooleanStub + }; - assert.expect( 2 ); + config.set( 'wgPopupsStatsvSamplingRate', 0.3141 ); - assert.ok( - statsv.isEnabled( this.user, this.config, experiments ), - 'Logging is enabled when the user is in the sample.' + statsv.isEnabled( user, config, experiments ); + + assert.ok( weightedBooleanStub.calledOnce ); + assert.deepEqual( + weightedBooleanStub.getCall( 0 ).args, + [ + 'ext.Popups.statsv', + config.get( 'wgPopupsStatsvSamplingRate' ), + user.sessionId() + ] ); - experiments = stubs.createStubExperiments( false ); + // --- - assert.notOk( - statsv.isEnabled( this.user, this.config, experiments ), - 'Logging is disabled when the user is not in the sample.' + config.delete( 'wgPopupsStatsvSamplingRate' ); + + statsv.isEnabled( user, config, experiments ); + + assert.deepEqual( + weightedBooleanStub.getCall( 1 ).args[ 1 ], + 0, + 'The bucketing rate should be 0 by default.' ); } );