From 548e94da982ca9f882eb9c42869540456f0c9d9f Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Fri, 23 Aug 2024 05:25:52 +0100 Subject: [PATCH] tests: Adopt private require() for skins.minerva.scripts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added to MediaWiki core last year with I9fca9fdf9b7623b1. This fixes a confusing assertion in page-issues/index.test.js, where for "insertBannersOrNotice()" it was asserting that the HTML contain "⧼skin-minerva-issue-learn-more⧽", where the ⧼ character indicates the message is not found (i.e. an error). The test had to be written this way in order to pass, because the skins.minerva.scripts module was not actually loaded, and thus its templates and messages are not present either. This lack was filled in by index.js for mw.templates, but not mw.messages. By adopting private require(), these workarounds can all be removed. == Motivation == In change I3a4024ccf90e505581, I'm working on improving the testrunner config to enforce uselang=qqx on all tests. This is passing except for GrowthExperiments and Minerva, both of which have the above workarounds in place that caused a message to be undefined, and then kept in the assertion expectation. When using uselang=qqx, values are returned as (key) instead of ⧼key⧽, which exposes these message existence errors. By removing this workaround, the test will simply import the module in the test as normal, thus the messages will exist, and thus it will expect (key), and thus it will continue to pass even after enforcing uselang=qqx. Change-Id: Ib68f45d93a7054ed8bd35fc5644e2852f2f90248 --- resources/skins.minerva.scripts/setup.js | 4 +- skin.json | 99 +++++++++---------- tests/qunit/skins.minerva.scripts/AB.test.js | 2 +- .../skins.minerva.scripts/TitleUtil.test.js | 2 +- .../skins.minerva.scripts/UriUtil.test.js | 2 +- .../downloadPageAction.test.js | 2 +- tests/qunit/skins.minerva.scripts/index.js | 14 --- .../page-issues/index.test.js | 51 +++++----- .../page-issues/parser.test.js | 2 +- .../skins.minerva.scripts/watchstar.test.js | 2 +- 10 files changed, 77 insertions(+), 103 deletions(-) delete mode 100644 tests/qunit/skins.minerva.scripts/index.js diff --git a/resources/skins.minerva.scripts/setup.js b/resources/skins.minerva.scripts/setup.js index 2501ef369..c7c941e45 100644 --- a/resources/skins.minerva.scripts/setup.js +++ b/resources/skins.minerva.scripts/setup.js @@ -46,7 +46,9 @@ function init() { ); } -init(); +if ( !window.QUnit ) { + init(); +} module.exports = { // Version number allows breaking changes to be detected by other extensions diff --git a/skin.json b/skin.json index 1e92096cc..58b118caf 100644 --- a/skin.json +++ b/skin.json @@ -520,6 +520,8 @@ ] }, "skins.minerva.scripts": { + "localBasePath": "resources/skins.minerva.scripts", + "remoteSkinPath": "MinervaNeue/resources/skins.minerva.scripts", "dependencies": [ "skins.minerva.mainMenu.icons", "skins.minerva.mainMenu.styles", @@ -559,40 +561,40 @@ "mobile-frontend-redirected-from" ], "styles": [ - "resources/skins.minerva.scripts/styles.less" + "styles.less" ], "templates": { - "IssueNotice.mustache": "resources/skins.minerva.scripts/page-issues/overlay/IssueNotice.mustache" + "IssueNotice.mustache": "page-issues/overlay/IssueNotice.mustache" }, "packageFiles": [ - "resources/skins.minerva.scripts/setup.js", - "resources/skins.minerva.scripts/reportIfNightModeWasDisabledOnPage.js", - "resources/skins.minerva.scripts/addPortletLink.js", - "resources/skins.minerva.scripts/initMobile.js", - "resources/skins.minerva.scripts/searchSuggestReveal.js", - "resources/skins.minerva.scripts/drawers.js", - "resources/skins.minerva.scripts/ctaDrawers.js", - "resources/skins.minerva.scripts/menu.js", - "resources/skins.minerva.scripts/preInit.js", - "resources/skins.minerva.scripts/downloadPageAction.js", - "resources/skins.minerva.scripts/page-issues/parser.js", - "resources/skins.minerva.scripts/AB.js", - "resources/skins.minerva.scripts/page-issues/overlay/IssueNotice.js", - "resources/skins.minerva.scripts/page-issues/overlay/IssueList.js", - "resources/skins.minerva.scripts/page-issues/overlay/pageIssuesOverlay.js", - "resources/skins.minerva.scripts/page-issues/page/PageIssueLearnMoreLink.js", - "resources/skins.minerva.scripts/page-issues/page/PageIssueLink.js", - "resources/skins.minerva.scripts/page-issues/page/pageIssueFormatter.js", - "resources/skins.minerva.scripts/page-issues/index.js", - "resources/skins.minerva.scripts/UriUtil.js", - "resources/skins.minerva.scripts/TitleUtil.js", - "includes/Skins/ToggleList/ToggleList.js", - "resources/skins.minerva.scripts/TabScroll.js", - "resources/skins.minerva.scripts/Toolbar.js", - "resources/skins.minerva.scripts/mobileRedirect.js", - "resources/skins.minerva.scripts/search.js", - "resources/skins.minerva.scripts/references.js", - "resources/skins.minerva.scripts/watchstar.js" + "setup.js", + "reportIfNightModeWasDisabledOnPage.js", + "addPortletLink.js", + "initMobile.js", + "searchSuggestReveal.js", + "drawers.js", + "ctaDrawers.js", + "menu.js", + "preInit.js", + "downloadPageAction.js", + "page-issues/parser.js", + "AB.js", + "page-issues/overlay/IssueNotice.js", + "page-issues/overlay/IssueList.js", + "page-issues/overlay/pageIssuesOverlay.js", + "page-issues/page/PageIssueLearnMoreLink.js", + "page-issues/page/PageIssueLink.js", + "page-issues/page/pageIssueFormatter.js", + "page-issues/index.js", + "UriUtil.js", + "TitleUtil.js", + "../../includes/Skins/ToggleList/ToggleList.js", + "TabScroll.js", + "Toolbar.js", + "mobileRedirect.js", + "search.js", + "references.js", + "watchstar.js" ] }, "skins.minerva.messageBox.styles": { @@ -618,39 +620,26 @@ } }, "QUnitTestModule": { - "localBasePath": "", - "remoteSkinPath": "MinervaNeue", + "localBasePath": "tests/qunit/skins.minerva.scripts", + "remoteSkinPath": "MinervaNeue/tests/qunit/skins.minerva.scripts", "dependencies": [ "mediawiki.cookie", "skins.minerva.messageBox.styles", + "skins.minerva.scripts", "mobile.startup", "mediawiki.user", "mediawiki.experiments", "mediawiki.Uri" ], - "packageFiles": [ - "tests/qunit/skins.minerva.scripts/index.js", - "tests/qunit/skins.minerva.scripts/integration.test.js", - "resources/skins.minerva.scripts/page-issues/parser.js", - "resources/skins.minerva.scripts/downloadPageAction.js", - "resources/skins.minerva.scripts/AB.js", - "resources/skins.minerva.scripts/page-issues/overlay/IssueNotice.js", - "resources/skins.minerva.scripts/page-issues/overlay/IssueList.js", - "resources/skins.minerva.scripts/page-issues/overlay/pageIssuesOverlay.js", - "resources/skins.minerva.scripts/page-issues/page/PageIssueLearnMoreLink.js", - "resources/skins.minerva.scripts/page-issues/page/PageIssueLink.js", - "resources/skins.minerva.scripts/page-issues/page/pageIssueFormatter.js", - "resources/skins.minerva.scripts/page-issues/index.js", - "resources/skins.minerva.scripts/UriUtil.js", - "resources/skins.minerva.scripts/TitleUtil.js", - "resources/skins.minerva.scripts/watchstar.js", - "tests/qunit/skins.minerva.scripts/downloadPageAction.test.js", - "tests/qunit/skins.minerva.scripts/page-issues/parser.test.js", - "tests/qunit/skins.minerva.scripts/AB.test.js", - "tests/qunit/skins.minerva.scripts/page-issues/index.test.js", - "tests/qunit/skins.minerva.scripts/UriUtil.test.js", - "tests/qunit/skins.minerva.scripts/TitleUtil.test.js", - "tests/qunit/skins.minerva.scripts/watchstar.test.js" + "scripts": [ + "integration.test.js", + "downloadPageAction.test.js", + "page-issues/parser.test.js", + "AB.test.js", + "page-issues/index.test.js", + "UriUtil.test.js", + "TitleUtil.test.js", + "watchstar.test.js" ] }, "ServiceWiringFiles": [ diff --git a/tests/qunit/skins.minerva.scripts/AB.test.js b/tests/qunit/skins.minerva.scripts/AB.test.js index c38511d48..8d35c45a7 100644 --- a/tests/qunit/skins.minerva.scripts/AB.test.js +++ b/tests/qunit/skins.minerva.scripts/AB.test.js @@ -1,6 +1,6 @@ ( function () { - const AB = require( '../../../resources/skins.minerva.scripts/AB.js' ); + const AB = require( 'skins.minerva.scripts/AB.js' ); const defaultConfig = { testName: 'WME.MinervaABTest', samplingRate: 0.5, diff --git a/tests/qunit/skins.minerva.scripts/TitleUtil.test.js b/tests/qunit/skins.minerva.scripts/TitleUtil.test.js index 1d8349113..a808e0dff 100644 --- a/tests/qunit/skins.minerva.scripts/TitleUtil.test.js +++ b/tests/qunit/skins.minerva.scripts/TitleUtil.test.js @@ -1,5 +1,5 @@ ( function () { - const TitleUtil = require( '../../../resources/skins.minerva.scripts/TitleUtil.js' ); + const TitleUtil = require( 'skins.minerva.scripts/TitleUtil.js' ); const mwUriOrg = mw.Uri; QUnit.module( 'Minerva TitleUtil', QUnit.newMwEnvironment( { diff --git a/tests/qunit/skins.minerva.scripts/UriUtil.test.js b/tests/qunit/skins.minerva.scripts/UriUtil.test.js index eb35415d0..56d013383 100644 --- a/tests/qunit/skins.minerva.scripts/UriUtil.test.js +++ b/tests/qunit/skins.minerva.scripts/UriUtil.test.js @@ -1,5 +1,5 @@ ( function () { - const UriUtil = require( '../../../resources/skins.minerva.scripts/UriUtil.js' ); + const UriUtil = require( 'skins.minerva.scripts/UriUtil.js' ); const mwUriOrg = mw.Uri; QUnit.module( 'Minerva UriUtil', { diff --git a/tests/qunit/skins.minerva.scripts/downloadPageAction.test.js b/tests/qunit/skins.minerva.scripts/downloadPageAction.test.js index 42d4fcba7..cdfa15d96 100644 --- a/tests/qunit/skins.minerva.scripts/downloadPageAction.test.js +++ b/tests/qunit/skins.minerva.scripts/downloadPageAction.test.js @@ -7,7 +7,7 @@ const Deferred = $.Deferred; const windowChrome = { chrome: true }; const windowNotChrome = {}; - const downloadAction = require( '../../../resources/skins.minerva.scripts/downloadPageAction.js' ); + const downloadAction = require( 'skins.minerva.scripts/downloadPageAction.js' ); const onClick = downloadAction.test.onClick; const isAvailable = downloadAction.test.isAvailable; diff --git a/tests/qunit/skins.minerva.scripts/index.js b/tests/qunit/skins.minerva.scripts/index.js deleted file mode 100644 index c2430396b..000000000 --- a/tests/qunit/skins.minerva.scripts/index.js +++ /dev/null @@ -1,14 +0,0 @@ -// Since tests.minerva.scripts does -// not pull in the entire module skins.minerva.scripts -// we have to stub certain templates to make it appear like its been loaded. -mw.template.add( 'skins.minerva.scripts', 'IssueNotice.mustache', '' ); -module.exports = [ - require( './integration.test.js' ), - require( './downloadPageAction.test.js' ), - require( './page-issues/parser.test.js' ), - require( './AB.test.js' ), - require( './page-issues/index.test.js' ), - require( './UriUtil.test.js' ), - require( './TitleUtil.test.js' ), - require( './watchstar.test.js' ) -]; diff --git a/tests/qunit/skins.minerva.scripts/page-issues/index.test.js b/tests/qunit/skins.minerva.scripts/page-issues/index.test.js index e6ccc6491..8d0833dbc 100644 --- a/tests/qunit/skins.minerva.scripts/page-issues/index.test.js +++ b/tests/qunit/skins.minerva.scripts/page-issues/index.test.js @@ -1,31 +1,28 @@ -( function () { - const - mobile = require( 'mobile.startup' ), - pageIssues = require( '../../../../resources/skins.minerva.scripts/page-issues/index.js' ), - insertBannersOrNotice = pageIssues.test.insertBannersOrNotice, - PageHTMLParser = mobile.PageHTMLParser, - overlayManager = mobile.getOverlayManager(), - $mockContainer = $( - '
' + - '' + - '' + - '' + - '' + - '
ambox text span
' + - '
' - ), - labelText = 'label text', - inline = true, - SECTION = '0', - processedAmbox = insertBannersOrNotice( - new PageHTMLParser( $mockContainer ), - labelText, SECTION, inline, overlayManager - ).ambox; - - QUnit.module( 'Minerva pageIssues' ); +QUnit.module( 'Minerva pageIssues', () => { + const mobile = require( 'mobile.startup' ); + const pageIssues = require( 'skins.minerva.scripts/page-issues/index.js' ); + const insertBannersOrNotice = pageIssues.test.insertBannersOrNotice; + const PageHTMLParser = mobile.PageHTMLParser; + const overlayManager = mobile.getOverlayManager(); + const $mockContainer = $( + '
' + + '' + + '' + + '' + + '' + + '
ambox text span
' + + '
' + ); + const labelText = 'label text'; + const inline = true; + const SECTION = '0'; + const processedAmbox = insertBannersOrNotice( + new PageHTMLParser( $mockContainer ), + labelText, SECTION, inline, overlayManager + ).ambox; QUnit.test( 'insertBannersOrNotice() should add a "learn more" message', ( assert ) => { - assert.true( /⧼skin-minerva-issue-learn-more⧽/.test( processedAmbox.html() ) ); + assert.true( /(skin-minerva-issue-learn-more)/.test( processedAmbox.html() ) ); } ); QUnit.test( 'insertBannersOrNotice() should add an icon', ( assert ) => { @@ -35,4 +32,4 @@ processedAmbox.click(); assert.strictEqual( window.location.hash, '#/issues/' + SECTION ); } ); -}() ); +} ); diff --git a/tests/qunit/skins.minerva.scripts/page-issues/parser.test.js b/tests/qunit/skins.minerva.scripts/page-issues/parser.test.js index d59ab802b..ed330305f 100644 --- a/tests/qunit/skins.minerva.scripts/page-issues/parser.test.js +++ b/tests/qunit/skins.minerva.scripts/page-issues/parser.test.js @@ -1,6 +1,6 @@ ( function () { const iconElement = document.createElement( 'div' ), - pageIssuesParser = require( '../../../../resources/skins.minerva.scripts/page-issues/parser.js' ), + pageIssuesParser = require( 'skins.minerva.scripts/page-issues/parser.js' ), extractMessage = pageIssuesParser.extract; iconElement.classList.add( 'minerva-icon--issue-generic-defaultColor', 'minerva-ambox-icon' ); diff --git a/tests/qunit/skins.minerva.scripts/watchstar.test.js b/tests/qunit/skins.minerva.scripts/watchstar.test.js index f3aeac8a6..4458e4ebc 100644 --- a/tests/qunit/skins.minerva.scripts/watchstar.test.js +++ b/tests/qunit/skins.minerva.scripts/watchstar.test.js @@ -1,6 +1,6 @@ /* eslint-disable no-jquery/no-class-state */ ( function () { - const watchstar = require( '../../../resources/skins.minerva.scripts/watchstar.js' ); + const watchstar = require( 'skins.minerva.scripts/watchstar.js' ); const toggleClasses = watchstar.test.toggleClasses; const WATCHED_CLASS = watchstar.test.WATCHED_ICON_CLASS; const TEMP_WATCHED_CLASS = watchstar.test.TEMP_WATCHED_ICON_CLASS;