jenkins-bot has submitted this change and it was merged. Change subject: tests: Twist the time in comparison tests in a different manner ......................................................................
tests: Twist the time in comparison tests in a different manner The OO.ui.Element#updateThemeClasses is asynchronous and debounced so that multiple state changes during widget initialization, which would normally each require a theme classes update, require only one. However, in comparison tests we need to wait for the updates to complete, or better yet, we want them to be synchronous because that makes life so much easier. Until now we ran the debounced method manually for the root widget, but that obviously only works for the root widget, and would cause the tests to fail in cryptic ways when the root widget included other widgets which needed their theme classes to be updated (I4e8e755e66916abffb90516519fe2b8fc4b48cd7, Ie14a35fac70d62ff7d102caaa56654ebde11d7dd). So let's try something else: make the OO.ui.Element#updateThemeClasses method not be debounced or asynchronous for the tests. It's a bit more evil, and requires a weird check in FlaggedElement (but we already have such workarounds for methods being called before their class is done constructing in various places in the library), but should be a bit more reliable. Change-Id: I4460919cd424a67fed98a06735b86e19f34b1fba --- M src/mixins/FlaggedElement.js M tests/JSPHP.test.karma.js M tests/JSPHP.test.standalone.js 3 files changed, 12 insertions(+), 10 deletions(-) Approvals: Jforrester: Looks good to me, approved jenkins-bot: Verified diff --git a/src/mixins/FlaggedElement.js b/src/mixins/FlaggedElement.js index bd35427..400c5d6 100644 --- a/src/mixins/FlaggedElement.js +++ b/src/mixins/FlaggedElement.js @@ -100,7 +100,8 @@ * @return {boolean} The flag is set */ OO.ui.mixin.FlaggedElement.prototype.hasFlag = function ( flag ) { - return flag in this.flags; + // This may be called before the constructor, thus before this.flags is set + return this.flags && ( flag in this.flags ); }; /** @@ -109,7 +110,8 @@ * @return {string[]} Flag names */ OO.ui.mixin.FlaggedElement.prototype.getFlags = function () { - return Object.keys( this.flags ); + // This may be called before the constructor, thus before this.flags is set + return Object.keys( this.flags || {} ); }; /** diff --git a/tests/JSPHP.test.karma.js b/tests/JSPHP.test.karma.js index 5e181c0..9fe2db1 100644 --- a/tests/JSPHP.test.karma.js +++ b/tests/JSPHP.test.karma.js @@ -31,14 +31,10 @@ $( 'body' ).append( instance.$element, $fromPhp ); - // Updating theme classes is normally debounced, we need to do it immediately - instance.debouncedUpdateThemeClasses(); - testName = JSON.stringify( test.config ); assert.equalDomElement( instance.$element[ 0 ], $fromPhp[ 0 ], testName, true ); infused = OO.ui.infuse( $fromPhp[ 0 ] ); - infused.debouncedUpdateThemeClasses(); assert.equalDomElement( instance.$element[ 0 ], infused.$element[ 0 ], testName + ' (infuse)', true ); instance.$element.add( infused.$element ).detach(); @@ -46,6 +42,10 @@ } ); } + // Updating theme classes is normally debounced, we need to do it immediately + // if we want the tests to be synchronous + OO.ui.Element.prototype.updateThemeClasses = OO.ui.Element.prototype.debouncedUpdateThemeClasses; + /*global testSuiteConfigs, testSuitePHPOutput */ for ( klassName in testSuiteConfigs ) { for ( theme in themes ) { diff --git a/tests/JSPHP.test.standalone.js b/tests/JSPHP.test.standalone.js index 1cbebc3..dcd4560 100644 --- a/tests/JSPHP.test.standalone.js +++ b/tests/JSPHP.test.standalone.js @@ -32,20 +32,20 @@ fromPhp = document.getElementById( id ).firstChild; instance.$element.insertBefore( fromPhp ); - // Updating theme classes is normally debounced, we need to do it immediately - instance.debouncedUpdateThemeClasses(); - testName = JSON.stringify( test.config ); assert.equalDomElement( instance.$element[ 0 ], fromPhp, testName ); infused = OO.ui.infuse( fromPhp ); - infused.debouncedUpdateThemeClasses(); assert.equalDomElement( instance.$element[ 0 ], infused.$element[ 0 ], testName + ' (infuse)' ); } } ); } + // Updating theme classes is normally debounced, we need to do it immediately + // if we want the tests to be synchronous + OO.ui.Element.prototype.updateThemeClasses = OO.ui.Element.prototype.debouncedUpdateThemeClasses; + for ( klassName in OO.ui.JSPHPTestSuite ) { for ( theme in themes ) { makeTest( theme, klassName, OO.ui.JSPHPTestSuite[ klassName ] ); -- To view, visit https://gerrit.wikimedia.org/r/227569 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I4460919cd424a67fed98a06735b86e19f34b1fba Gerrit-PatchSet: 2 Gerrit-Project: oojs/ui Gerrit-Branch: master Gerrit-Owner: Bartosz DziewoĆski <matma....@gmail.com> Gerrit-Reviewer: Catrope <roan.katt...@gmail.com> Gerrit-Reviewer: Jforrester <jforres...@wikimedia.org> Gerrit-Reviewer: Krinkle <krinklem...@gmail.com> Gerrit-Reviewer: Trevor Parscal <tpars...@wikimedia.org> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits