r101356 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r101355‎ | r101356 | r101357 >
Date:13:53, 31 October 2011
Author:hashar
Status:ok (Comments)
Tags:
Comment:
fix mediawiki.test wrong URL

mw.loader.bug30825 test verify we AJAX calls support protocols relative URLS.
The way we forged the target URL was fine when running tests directly from
the file (ie: http://localhost/wiki/tests/qunit/index.html ) but did not
work correctly from Special:JavaScriptTest.

This patch fix the forged URL so it works in both case hopefully.
Modified paths:
  • /branches/JSTesting/tests/qunit/QUnitTestResources.php (modified) (history)
  • /branches/JSTesting/tests/qunit/suites/resources/mediawiki/mediawiki.test.js (modified) (history)

Diff [purge]

Index: branches/JSTesting/tests/qunit/QUnitTestResources.php
——@@ -18,10 +18,7 @@
1919 # jquery.tablesorter.test.js: Broken
2020 #'tests/qunit/suites/resources/jquery/jquery.tablesorter.test.js', // has mw-config def
2121 'tests/qunit/suites/resources/jquery/jquery.textSelection.test.js',
22 - # mediawiki.test.js: Tries to load from /data/ directory, fails when ran from the
23 - # SpecialPage since it uses regex to get the current path and loads from that + /data/
24 - # (index.php/Data in this case..)
25 - #'tests/qunit/suites/resources/mediawiki/mediawiki.test.js',
 22+ 'tests/qunit/suites/resources/mediawiki/mediawiki.test.js',
2623 'tests/qunit/suites/resources/mediawiki/mediawiki.title.test.js', // has mw-config def
2724 'tests/qunit/suites/resources/mediawiki/mediawiki.user.test.js',
2825 'tests/qunit/suites/resources/mediawiki/mediawiki.util.test.js',
Index: branches/JSTesting/tests/qunit/suites/resources/mediawiki/mediawiki.test.js
——@@ -173,13 +173,30 @@
174174 // This bug was actually already fixed in 1.18 and later when discovered in 1.17.
175175 // Test is for regressions!
176176
177 - expect(1);
 177+ expect(2);
178178
 179+ var wgServer = mw.config.get( 'wgServer' ),
 180+ basePath = mw.config.get( 'wgScriptPath' );
 181+
 182+ // From [[Special:JavaScriptTest]] we need to preprend the script path
 183+ // with the actual server (http://localhost/).
 184+ // Running from file tests/qunit/index.html, wgScriptPath is already
 185+ // including the wgServer part
 186+ if( wgServer !== null ) {
 187+ basePath = wgServer + wgScriptPath;
 188+ }
 189+ // Forge an URL to the test callback script
 190+ var target = QUnit.fixurl(
 191+ basePath + '/tests/qunit/data/qunitOkCall.js'
 192+ );
 193+
179194 // Confirm that mw.loader.load() works with protocol-relative URLs
180 - var loc = window.location,
181 - base = ('//' + loc.hostname + loc.pathname).replace(/\/[^\/]*$/, ''),
182 - target = base + '/data/qunitOkCall.js?' + (new Date()).getTime();
 195+ target = target.replace( /https?:/, '' );
183196
 197+ equal( target.substr( 0, 2 ), '//',
 198+ 'URL must be relative to test relative URLs!'
 199+ );
 200+
184201 // Async!
185202 stop();
186203 mw.loader.load( target );

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r96679MFT r87986 -- consolidation of addScript in mediawiki.loader, also fixes bug ...brion18:39, 9 September 2011
r96680Followup r87986: qunit test case for bug 30825...brion18:43, 9 September 2011
r96699Provisional workaround for bug 30825 as it affects things run via $.ajax() us...brion22:03, 9 September 2011
r97404Move the mediawiki.test.bug30825.js helper script out of the /suites/ directo...krinkle04:50, 18 September 2011

Comments

#Comment by Krinkle (talk | contribs)   19:00, 31 October 2011

I can't test this right now but did you account for the fact that mw.config is empty when running from /tests/qunit/index.html (because there is no database/php) ? (which is the reason it was using a regex).

#Comment by Krinkle (talk | contribs)   19:01, 31 October 2011

See also r95638 which sets wgScriptPath for php/db-less static requests via /tests/qunit/index.html.

#Comment by Krinkle (talk | contribs)   19:08, 31 October 2011

Oh now I see it. My bad. So the wgScriptPath isn't entirely clean on index.html, perhaps we can fix that more centrally ?

#Comment by Hashar (talk | contribs)   20:15, 31 October 2011

With index.html on my installation:

Hence the above hack which is merely a workaround.

#Comment by Krinkle (talk | contribs)   20:51, 31 October 2011

I understand the reason for the hack. Does the job well to make it work both statically and on-wiki. I just thought it'd be better to centralize it a tad bit since the other places where we access /tests/qunit/data likely need the same fix. However it appears there are no other places :)

#Comment by Krinkle (talk | contribs)   20:51, 31 October 2011

I understand the reason for the hack. Does the job well to make it work both statically and on-wiki. I just thought it'd be better to centralize it a tad bit since the other places where we access /tests/qunit/data likely need the same fix. However it appears there are no other places :)

Status & tagging log