[MediaWiki-commits] [Gerrit] mediawiki...MultimediaViewer[master]: Fix mmv.logging.PerformanceLogger qunit tests
Matthias Mullie has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/353890 )
Change subject: Fix mmv.logging.PerformanceLogger qunit tests
..
Fix mmv.logging.PerformanceLogger qunit tests
Bug: T164473
Change-Id: I6ae5c0170bf12fb076ddc505299ceeb7b9c292e8
---
M resources/mmv/logging/mmv.logging.PerformanceLogger.js
M tests/qunit/mmv/logging/mmv.logging.PerformanceLogger.test.js
2 files changed, 79 insertions(+), 61 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/MultimediaViewer
refs/changes/90/353890/1
diff --git a/resources/mmv/logging/mmv.logging.PerformanceLogger.js
b/resources/mmv/logging/mmv.logging.PerformanceLogger.js
index fdbfd65..639e907 100644
--- a/resources/mmv/logging/mmv.logging.PerformanceLogger.js
+++ b/resources/mmv/logging/mmv.logging.PerformanceLogger.js
@@ -116,6 +116,7 @@
* @param {string} url URL of that was measured
* @param {XMLHttpRequest} request HTTP request that just completed
* @param {jQuery.Deferred.} [extraStatsDeferred] A promise
which resolves to extra stats to be included.
+* @return {jQuery.Promise}
*/
PL.recordEntry = function ( type, total, url, request,
extraStatsDeferred ) {
var matches,
@@ -133,7 +134,7 @@
if ( url && url.length ) {
// There is no need to measure the same url more than
once
if ( url in this.performanceChecked ) {
- return;
+ return $.Deferred().reject();
}
this.performanceChecked[ url ] = true;
@@ -166,7 +167,7 @@
}
}
- ( extraStatsDeferred || $.Deferred().reject() ).done( function
( extraStats ) {
+ return ( extraStatsDeferred || $.Deferred().reject() ).done(
function ( extraStats ) {
stats = $.extend( stats, extraStats );
} ).always( function () {
logger.log( stats );
diff --git a/tests/qunit/mmv/logging/mmv.logging.PerformanceLogger.test.js
b/tests/qunit/mmv/logging/mmv.logging.PerformanceLogger.test.js
index 006f392..49a4d60 100644
--- a/tests/qunit/mmv/logging/mmv.logging.PerformanceLogger.test.js
+++ b/tests/qunit/mmv/logging/mmv.logging.PerformanceLogger.test.js
@@ -40,7 +40,9 @@
var performance = new mw.mmv.logging.PerformanceLogger(),
fakeEventLog = { logEvent: this.sandbox.stub() },
type = 'gender',
- total = 100;
+ total = 100,
+ // we'll be waiting for 4 promises to complete
+ asyncs = [assert.async(), assert.async(),
assert.async(), assert.async()];
this.sandbox.stub( performance, 'loadDependencies' ).returns(
$.Deferred().resolve() );
this.sandbox.stub( performance, 'isInSample' );
@@ -48,27 +50,30 @@
performance.isInSample.returns( false );
- performance.recordEntry( type, total );
-
- assert.strictEqual( fakeEventLog.logEvent.callCount, 0, 'No
stats should be logged if not in sample' );
+ performance.recordEntry( type, total ).then( null, function () {
+ assert.strictEqual( fakeEventLog.logEvent.callCount, 0,
'No stats should be logged if not in sample' );
+ asyncs.pop()();
+ } );
performance.isInSample.returns( true );
- performance.recordEntry( type, total );
+ performance.recordEntry( type, total ).then( null, function () {
+ assert.strictEqual( fakeEventLog.logEvent.getCall( 0
).args[ 0 ], 'MultimediaViewerNetworkPerformance', 'EventLogging schema is
correct' );
+ assert.strictEqual( fakeEventLog.logEvent.getCall( 0
).args[ 1 ].type, type, 'type is correct' );
+ assert.strictEqual( fakeEventLog.logEvent.getCall( 0
).args[ 1 ].total, total, 'total is correct' );
+ assert.strictEqual( fakeEventLog.logEvent.callCount, 1,
'Stats should be logged' );
+ asyncs.pop()();
+ } );
- assert.strictEqual( fakeEventLog.logEvent.getCall( 0 ).args[ 0
], 'MultimediaViewerNetworkPerformance', 'EventLogging schema is correct' );
- assert.strictEqual( fakeEventLog.logEvent.getCall( 0 ).args[ 1
].type, type, 'type is correct' );
- assert.strictEqual( fakeEventLog.logEvent.getCall( 0 ).args[ 1
].total, total, 'total is correct' );
+ performance.recordEntry( type, total, 'URL' ).then( null,
function () {
+ assert.strictEqual( fakeEventLog.logEvent.callCount, 2,
'Stats should be logged' );
+
[MediaWiki-commits] [Gerrit] mediawiki...MultimediaViewer[master]: Fix mmv.logging.PerformanceLogger qunit tests
Jforrester has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/353890 )
Change subject: Fix mmv.logging.PerformanceLogger qunit tests
..
Fix mmv.logging.PerformanceLogger qunit tests
Bug: T164473
Change-Id: I6ae5c0170bf12fb076ddc505299ceeb7b9c292e8
---
M resources/mmv/logging/mmv.logging.PerformanceLogger.js
M tests/qunit/mmv/logging/mmv.logging.PerformanceLogger.test.js
2 files changed, 79 insertions(+), 61 deletions(-)
Approvals:
Jforrester: Verified; Looks good to me, approved
diff --git a/resources/mmv/logging/mmv.logging.PerformanceLogger.js
b/resources/mmv/logging/mmv.logging.PerformanceLogger.js
index fdbfd65..639e907 100644
--- a/resources/mmv/logging/mmv.logging.PerformanceLogger.js
+++ b/resources/mmv/logging/mmv.logging.PerformanceLogger.js
@@ -116,6 +116,7 @@
* @param {string} url URL of that was measured
* @param {XMLHttpRequest} request HTTP request that just completed
* @param {jQuery.Deferred.} [extraStatsDeferred] A promise
which resolves to extra stats to be included.
+* @return {jQuery.Promise}
*/
PL.recordEntry = function ( type, total, url, request,
extraStatsDeferred ) {
var matches,
@@ -133,7 +134,7 @@
if ( url && url.length ) {
// There is no need to measure the same url more than
once
if ( url in this.performanceChecked ) {
- return;
+ return $.Deferred().reject();
}
this.performanceChecked[ url ] = true;
@@ -166,7 +167,7 @@
}
}
- ( extraStatsDeferred || $.Deferred().reject() ).done( function
( extraStats ) {
+ return ( extraStatsDeferred || $.Deferred().reject() ).done(
function ( extraStats ) {
stats = $.extend( stats, extraStats );
} ).always( function () {
logger.log( stats );
diff --git a/tests/qunit/mmv/logging/mmv.logging.PerformanceLogger.test.js
b/tests/qunit/mmv/logging/mmv.logging.PerformanceLogger.test.js
index 006f392..777f129 100644
--- a/tests/qunit/mmv/logging/mmv.logging.PerformanceLogger.test.js
+++ b/tests/qunit/mmv/logging/mmv.logging.PerformanceLogger.test.js
@@ -40,7 +40,9 @@
var performance = new mw.mmv.logging.PerformanceLogger(),
fakeEventLog = { logEvent: this.sandbox.stub() },
type = 'gender',
- total = 100;
+ total = 100,
+ // we'll be waiting for 4 promises to complete
+ asyncs = [ assert.async(), assert.async(),
assert.async(), assert.async() ];
this.sandbox.stub( performance, 'loadDependencies' ).returns(
$.Deferred().resolve() );
this.sandbox.stub( performance, 'isInSample' );
@@ -48,27 +50,30 @@
performance.isInSample.returns( false );
- performance.recordEntry( type, total );
-
- assert.strictEqual( fakeEventLog.logEvent.callCount, 0, 'No
stats should be logged if not in sample' );
+ performance.recordEntry( type, total ).then( null, function () {
+ assert.strictEqual( fakeEventLog.logEvent.callCount, 0,
'No stats should be logged if not in sample' );
+ asyncs.pop()();
+ } );
performance.isInSample.returns( true );
- performance.recordEntry( type, total );
+ performance.recordEntry( type, total ).then( null, function () {
+ assert.strictEqual( fakeEventLog.logEvent.getCall( 0
).args[ 0 ], 'MultimediaViewerNetworkPerformance', 'EventLogging schema is
correct' );
+ assert.strictEqual( fakeEventLog.logEvent.getCall( 0
).args[ 1 ].type, type, 'type is correct' );
+ assert.strictEqual( fakeEventLog.logEvent.getCall( 0
).args[ 1 ].total, total, 'total is correct' );
+ assert.strictEqual( fakeEventLog.logEvent.callCount, 1,
'Stats should be logged' );
+ asyncs.pop()();
+ } );
- assert.strictEqual( fakeEventLog.logEvent.getCall( 0 ).args[ 0
], 'MultimediaViewerNetworkPerformance', 'EventLogging schema is correct' );
- assert.strictEqual( fakeEventLog.logEvent.getCall( 0 ).args[ 1
].type, type, 'type is correct' );
- assert.strictEqual( fakeEventLog.logEvent.getCall( 0 ).args[ 1
].total, total, 'total is correct' );
+ performance.recordEntry( type, total, 'URL' ).then( null,
function () {
+ assert.strictEqual( fakeEventLog.logEvent.callCount, 2,
'Stats should be logged' );
+ asyncs.pop()();
+
