r109698 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r109697‎ | r109698 | r109699 >
Date:22:26, 21 January 2012
Author:krinkle
Status:resolved (Comments)
Tags:core 
Comment:
[Unit testing] Add unit tests for Xml::namespaceSelector
* Making Language::namespaceNames explicitly public. It already was but since is being used as such outside the class
Modified paths:
  • /trunk/phase3/languages/Language.php (modified) (history)
  • /trunk/phase3/tests/phpunit/includes/XmlTest.php (modified) (history)
  • /trunk/phase3/tests/qunit/suites/resources/mediawiki/mediawiki.util.test.js (modified) (history)

Diff [purge]

Index: trunk/phase3/tests/qunit/suites/resources/mediawiki/mediawiki.util.test.js
——@@ -108,18 +108,18 @@
109109 test( 'getParamValue', function() {
110110 expect(5);
111111
112 - var url1 = 'http://mediawiki.org/?foo=wrong&foo=right#&foo=bad';
 112+ var url1 = 'http://example.org/?foo=wrong&foo=right#&foo=bad';
113113
114114 equal( mw.util.getParamValue( 'foo', url1 ), 'right', 'Use latest one, ignore hash' );
115115 strictEqual( mw.util.getParamValue( 'bar', url1 ), null, 'Return null when not found' );
116116
117 - var url2 = 'http://mediawiki.org/#&foo=bad';
 117+ var url2 = 'http://example.org/#&foo=bad';
118118 strictEqual( mw.util.getParamValue( 'foo', url2 ), null, 'Ignore hash if param is not in querystring but in hash (bug 27427)' );
119119
120 - var url3 = 'example.com?' + $.param({ 'TEST': 'a b+c' });
 120+ var url3 = 'example.org?' + $.param({ 'TEST': 'a b+c' });
121121 strictEqual( mw.util.getParamValue( 'TEST', url3 ), 'a b+c', 'Bug 30441: getParamValue must understand "+" encoding of space' );
122122
123 - var url4 = 'example.com?' + $.param({ 'TEST': 'a b+c d' }); // check for sloppy code from r95332 :)
 123+ var url4 = 'example.org?' + $.param({ 'TEST': 'a b+c d' }); // check for sloppy code from r95332 :)
124124 strictEqual( mw.util.getParamValue( 'TEST', url4 ), 'a b+c d', 'Bug 30441: getParamValue must understand "+" encoding of space (multiple spaces)' );
125125 });
126126
——@@ -161,12 +161,12 @@
162162 $mwPanel = $(mwPanel).appendTo( 'body' ),
163163 $vectorTabs = $(vectorTabs).appendTo( 'body' );
164164
165 - var tbRL = mw.util.addPortletLink( 'p-test-tb', 'http://mediawiki.org/wiki/ResourceLoader',
 165+ var tbRL = mw.util.addPortletLink( 'p-test-tb', '//mediawiki.org/wiki/ResourceLoader',
166166 'ResourceLoader', 't-rl', 'More info about ResourceLoader on MediaWiki.org ', 'l' );
167167
168168 ok( $.isDomElement( tbRL ), 'addPortletLink returns a valid DOM Element according to $.isDomElement' );
169169
170 - var tbMW = mw.util.addPortletLink( 'p-test-tb', 'http://mediawiki.org/',
 170+ var tbMW = mw.util.addPortletLink( 'p-test-tb', '//mediawiki.org/',
171171 'MediaWiki.org', 't-mworg', 'Go to MediaWiki.org ', 'm', tbRL ),
172172 $tbMW = $( tbMW );
173173
——@@ -175,7 +175,7 @@
176176 equal( $tbMW.closest( '.portlet' ).attr( 'id' ), 'p-test-tb', 'Link was inserted within correct portlet' );
177177 equal( $tbMW.next().attr( 'id' ), 't-rl', 'Link is in the correct position (by passing nextnode)' );
178178
179 - var tbRLDM = mw.util.addPortletLink( 'p-test-tb', 'http://mediawiki.org/wiki/RL/DM',
 179+ var tbRLDM = mw.util.addPortletLink( 'p-test-tb', '//mediawiki.org/wiki/RL/DM',
180180 'Default modules', 't-rldm', 'List of all default modules ', 'd', '#t-rl' );
181181
182182 equal( $( tbRLDM ).next().attr( 'id' ), 't-rl', 'Link is in the correct position (by passing CSS selector)' );
Index: trunk/phase3/tests/phpunit/includes/XmlTest.php
——@@ -2,17 +2,42 @@
33
44 class XmlTest extends MediaWikiTestCase {
55 private static $oldLang;
 6+ private static $oldNamespaces;
67
78 public function setUp() {
8 - global $wgLang;
 9+ global $wgLang, $wgContLang;
910
1011 self::$oldLang = $wgLang;
1112 $wgLang = Language::factory( 'en' );
 13+
 14+ // Hardcode namespaces during test runs,
 15+ // so that html output based on existing namespaces
 16+ // can be properly evaluated.
 17+ self::$oldNamespaces = $wgContLang->namespaceNames;
 18+ $wgContLang->namespaceNames = array(
 19+ -2 => 'Media',
 20+ -1 => 'Special',
 21+ 0 => '',
 22+ 1 => 'Talk',
 23+ 2 => 'User',
 24+ 3 => 'User_talk',
 25+ 4 => 'MyWiki',
 26+ 5 => 'MyWiki_Talk',
 27+ 6 => 'File',
 28+ 7 => 'File_talk',
 29+ 8 => 'MediaWiki',
 30+ 9 => 'MediaWiki_talk',
 31+ 10 => 'Template',
 32+ 11 => 'Template_talk',
 33+ 100 => 'Custom',
 34+ 101 => 'Custom_talk',
 35+ );
1236 }
1337
1438 public function tearDown() {
15 - global $wgLang;
 39+ global $wgLang, $wgContLang;
1640 $wgLang = self::$oldLang;
 41+ $wgContLang->namespaceNames = self::$oldNamespaces;
1742 }
1843
1944 public function testExpandAttributes() {
——@@ -167,6 +192,51 @@
168193 );
169194 }
170195
 196+ function testNamespaceSelector() {
 197+ $this->assertEquals(
 198+ '<select id="namespace" name="namespace" class="namespaceselector">
 199+<option value="0">(Main)</option>
 200+<option value="1">Talk</option>
 201+<option value="2">User</option>
 202+<option value="3">User talk</option>
 203+<option value="4">MyWiki</option>
 204+<option value="5">MyWiki Talk</option>
 205+<option value="6">File</option>
 206+<option value="7">File talk</option>
 207+<option value="8">MediaWiki</option>
 208+<option value="9">MediaWiki talk</option>
 209+<option value="10">Template</option>
 210+<option value="11">Template talk</option>
 211+<option value="100">Custom</option>
 212+<option value="101">Custom talk</option>
 213+</select>',
 214+ Xml::namespaceSelector(),
 215+ 'Basic namespace selector without custom options'
 216+ );
 217+ $this->assertEquals(
 218+ '<label for="myname">Select a namespace:</label>&#160;<select id="namespace" name="myname" class="namespaceselector">
 219+<option value="all">all</option>
 220+<option value="0">(Main)</option>
 221+<option value="1">Talk</option>
 222+<option value="2" selected="">User</option>
 223+<option value="3">User talk</option>
 224+<option value="4">MyWiki</option>
 225+<option value="5">MyWiki Talk</option>
 226+<option value="6">File</option>
 227+<option value="7">File talk</option>
 228+<option value="8">MediaWiki</option>
 229+<option value="9">MediaWiki talk</option>
 230+<option value="10">Template</option>
 231+<option value="11">Template talk</option>
 232+<option value="100">Custom</option>
 233+<option value="101">Custom talk</option>
 234+</select>',
 235+ Xml::namespaceSelector( $selected = '2', $all = 'all', $element_name = 'myname', $label = 'Select a namespace:' ),
 236+ 'Basic namespace selector with custom values'
 237+ );
 238+ }
 239+
 240+
171241 #
172242 # textarea
173243 #
Index: trunk/phase3/languages/Language.php
——@@ -63,10 +63,12 @@
6464 var $mMagicExtensions = array(), $mMagicHookDone = false;
6565 private $mHtmlCode = null;
6666
67 - var $mNamespaceIds, $namespaceNames, $namespaceAliases;
 67+ var $mNamespaceIds, $namespaceAliases;
6868 var $dateFormatStrings = array();
6969 var $mExtendedSpecialPageAliases;
7070
 71+ public $namespaceNames;
 72+
7173 /**
7274 * ReplacementArray object caches
7375 */

Follow-up revisions

RevisionCommit summaryAuthorDate
r109974[Unit testing] Re-order attribs to a-z to make testing more reliable...krinkle00:52, 25 January 2012
r109990[Xml/Html] new method Html::namespaceSelector...krinkle03:01, 25 January 2012
r109993[Html] Unit test + bugfix Html::namespaceSelector...krinkle03:25, 25 January 2012
r110118Fixup r109698, add setter for $namespaceNames and use proper accessors in the...demon13:00, 27 January 2012

Comments

#Comment by Nikerabbit (talk | contribs)   10:55, 23 January 2012

Should there be a comment that it is only public for testing, and should not be used otherwise?

#Comment by 😂 (talk | contribs)   21:14, 24 January 2012

It won't do any good. If you make it public, people will use it and complain when you take it away later.

#Comment by 😂 (talk | contribs)   15:43, 26 January 2012

Do not explicitly make this public. I know there's no functional difference but there's a practical one. If you need a member var outside of a class you should write accessors.

#Comment by 😂 (talk | contribs)   21:27, 26 January 2012

In fact, before this revision not a single usage of $namespaceNames was outside of this class (core or extensions). That's even more reason to make it protected and slap accessors on it rather than making it public.

Status & tagging log