Krinkle has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/394441 )
Change subject: Factor out getRandom() and inSample()
......................................................................
Factor out getRandom() and inSample()
* Centralise the Math.floor() logic and add tests for it.
* Make getRandom() easier to change.
* Document why we duplicated mw.eventLog.inSample().
* Remove use of needless jQuery isNumeric overhead in favour
of a stricter type check.
Partially extracted from I0908bad313a8566c.
Change-Id: I5a3170b9dd762801c87f7dd9a72e7596da11ba0d
---
M modules/ext.navigationTiming.js
M tests/ext.navigationTiming.test.js
2 files changed, 51 insertions(+), 19 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/NavigationTiming
refs/changes/41/394441/1
diff --git a/modules/ext.navigationTiming.js b/modules/ext.navigationTiming.js
index 84aa956..ca84954 100644
--- a/modules/ext.navigationTiming.js
+++ b/modules/ext.navigationTiming.js
@@ -13,12 +13,29 @@
visibilityChanged = false,
TYPE_NAVIGATE = 0;
- function inSample() {
- var factor = mw.config.get( 'wgNavigationTimingSamplingFactor'
);
- if ( !$.isNumeric( factor ) || factor < 1 ) {
+ /**
+ * Get random number between 0 (inclusive) and 1 (exclusive).
+ *
+ * @return {number}
+ */
+ function getRandom() {
+ return Math.random();
+ }
+
+ /**
+ * Determine result of random sampling.
+ *
+ * Based on ext.eventLogging.subscriber's mw.eventLog.inSample
+ * Duplicated here because we need it without/before dependencies.
+ *
+ * @param {number} factor One in how many should be included.
(0=nobody, 1=all, 2=50%)
+ * @return {boolean}
+ */
+ function inSample( factor ) {
+ if ( typeof factor !== 'number' || factor < 1 ) {
return false;
}
- return Math.floor( Math.random() * factor ) === 0;
+ return Math.floor( getRandom() * factor ) === 0;
}
function getDevicePixelRatio() {
@@ -251,14 +268,13 @@
}
function inAsiaSample() {
- var factor,
- // From
https://dev.maxmind.com/geoip/legacy/codes/country_continent/
- asianCountries = [ 'AE', 'AF', 'AM', 'AP', 'AZ', 'BD',
'BH', 'BN',
- 'BT', 'CC', 'CN', 'CX', 'CY', 'GE', 'HK', 'ID',
'IL', 'IN', 'IO',
- 'IQ', 'IR', 'JO', 'JP', 'KG', 'KH', 'KP', 'KR',
'KW', 'KZ', 'LA',
- 'LB', 'LK', 'MM', 'MN', 'MO', 'MV', 'MY', 'NP',
'OM', 'PH', 'PK',
- 'PS', 'QA', 'SA', 'SG', 'SY', 'TH', 'TJ', 'TL',
'TM', 'TW', 'UZ',
- 'VN', 'YE' ];
+ // From
https://dev.maxmind.com/geoip/legacy/codes/country_continent/
+ var asianCountries = [ 'AE', 'AF', 'AM', 'AP', 'AZ', 'BD',
'BH', 'BN',
+ 'BT', 'CC', 'CN', 'CX', 'CY', 'GE', 'HK', 'ID', 'IL',
'IN', 'IO',
+ 'IQ', 'IR', 'JO', 'JP', 'KG', 'KH', 'KP', 'KR', 'KW',
'KZ', 'LA',
+ 'LB', 'LK', 'MM', 'MN', 'MO', 'MV', 'MY', 'NP', 'OM',
'PH', 'PK',
+ 'PS', 'QA', 'SA', 'SG', 'SY', 'TH', 'TJ', 'TL', 'TM',
'TW', 'UZ',
+ 'VN', 'YE' ];
// Only regular page views, ignore any non-compliant data
if ( !navigation || navigation.type !== TYPE_NAVIGATE ||
!isCompliant() ) {
@@ -277,12 +293,7 @@
// Sampled (unlike main sampling factor, this is applied within
and
// after the above filters).
- factor = mw.config.get(
'wgNavigationTimingFirstPaintAsiaSamplingFactor' );
- if ( !$.isNumeric( factor ) || factor < 1 ) {
- return false;
- }
-
- return Math.floor( Math.random() * factor ) === 0;
+ return inSample( mw.config.get(
'wgNavigationTimingFirstPaintAsiaSamplingFactor' ) );
}
function emitAsiaFirstPaint() {
@@ -358,7 +369,7 @@
// Only load EventLogging when page load is in the sampling
// Use a conditional block instead of early return since module.exports
// must happen unconditionally for unit tests.
- isInSample = inSample();
+ isInSample = inSample( mw.config.get(
'wgNavigationTimingSamplingFactor', 0 ) );
if ( isInSample ) {
// Preload EventLogging and schema modules
loadEL = mw.loader.using( [ 'schema.NavigationTiming',
'schema.SaveTiming' ] );
@@ -385,6 +396,7 @@
* @private
*/
module.exports = {
+ inSample: inSample,
emitNavTiming: emitNavigationTiming,
emitAsiaFirstPaint: emitAsiaFirstPaint,
inAsiaSample: inAsiaSample,
diff --git a/tests/ext.navigationTiming.test.js
b/tests/ext.navigationTiming.test.js
index 8a99ad1..2dd23dd 100644
--- a/tests/ext.navigationTiming.test.js
+++ b/tests/ext.navigationTiming.test.js
@@ -20,6 +20,26 @@
}
} ) );
+QUnit.test( 'inSample', function ( assert ) {
+ var rand, navTiming = require( 'ext.navigationTiming' );
+
+ this.sandbox.stub( Math, 'random', function () {
+ return rand;
+ } );
+
+ rand = 0.99;
+ assert.strictEqual( navTiming.inSample( 0 ), false, '0 is never' );
+ assert.strictEqual( navTiming.inSample( 0.9 ), false, '0.9 is never' );
+ assert.strictEqual( navTiming.inSample( 1 ), true, '1 is always' );
+ assert.strictEqual( navTiming.inSample( '1' ), false, 'non-number is
never' );
+ assert.strictEqual( navTiming.inSample( 2 ), false, '2 not this time' );
+
+ rand = 0.01;
+ assert.strictEqual( navTiming.inSample( 0 ), false, '0 is never' );
+ assert.strictEqual( navTiming.inSample( 1 ), true, '1 is always' );
+ assert.strictEqual( navTiming.inSample( 2 ), true, '2 this time' );
+} );
+
// Basic test will ensure no exceptions are thrown and various
// of the core properties are set as expected.
QUnit.test( 'Basic', function ( assert ) {
--
To view, visit https://gerrit.wikimedia.org/r/394441
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5a3170b9dd762801c87f7dd9a72e7596da11ba0d
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/NavigationTiming
Gerrit-Branch: master
Gerrit-Owner: Krinkle <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
