Daniel Werner has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/67559


Change subject: Refactoring of SiteIdInterface test
......................................................................

Refactoring of SiteIdInterface test

* Smaller but more tests with improved structure.
* Removed invalid test (added TODO/FIXME) which made false assumptions about
  wb.ui.PropertyEditTool.EditableValue.Interface interface.

Change-Id: I1a625c1fed05bd85031c60f1ec0008b8a5951c89
---
M 
lib/tests/qunit/wikibase.ui.PropertyEditTool.EditableValue.SiteIdInterface.tests.js
1 file changed, 98 insertions(+), 67 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase 
refs/changes/59/67559/1

diff --git 
a/lib/tests/qunit/wikibase.ui.PropertyEditTool.EditableValue.SiteIdInterface.tests.js
 
b/lib/tests/qunit/wikibase.ui.PropertyEditTool.EditableValue.SiteIdInterface.tests.js
index 23a12c9..787e86c 100644
--- 
a/lib/tests/qunit/wikibase.ui.PropertyEditTool.EditableValue.SiteIdInterface.tests.js
+++ 
b/lib/tests/qunit/wikibase.ui.PropertyEditTool.EditableValue.SiteIdInterface.tests.js
@@ -8,6 +8,7 @@
  *
  * @licence GNU GPL v2+
  * @author H. Snater < [email protected] >
+ * @author Daniel Werner < [email protected] >
  */
 
 ( function( mw, wb, $, QUnit, undefined ) {
@@ -26,37 +27,42 @@
                return new 
wb.ui.PropertyEditTool.EditableValue.SiteIdInterface( $node, options );
        };
 
-       var siteIds = ['en', 'de'];
+       var siteIds = ['enwiki', 'dewiki'];
 
        QUnit.module( 
'wikibase.ui.PropertyEditTool.EditableValue.SiteIdInterface', 
QUnit.newWbEnvironment( {
                config: {
                        'wbSiteDetails': {
-                               en: {
+                               enwiki: {
                                        apiUrl: 
'http://en.wikipedia.org/w/api.php',
-                                       id: 'en',
                                        name: 'English Wikipedia',
+                                       shortName: 'English',
                                        pageUrl: 
'http://en.wikipedia.org/wiki/$1',
-                                       shortName: 'English'
+                                       languageCode: 'en',
+                                       id: 'enwiki',
+                                       group: 'whatever'
                                },
-                               de: {
+                               dewiki: {
                                        apiUrl: 
'http://de.wikipedia.org/w/api.php',
-                                       id: 'de',
                                        name: 'Deutsche Wikipedia',
+                                       shortName: 'Deutsch',
                                        pageUrl: 
'http://de.wikipedia.org/wiki/$1',
-                                       shortName: 'Deutsch'
+                                       languageCode: 'de',
+                                       id: 'dewiki',
+                                       group: 'another'
                                }
                        }
                },
                teardown: function() { $( '.wikibase-siteselector-list' 
).remove(); }
        } ) );
 
-       QUnit.test( 'init input', function( assert ) {
-               var $node = $( '<div/>', { id: 'subject' } );
-               var siteIdInterface = newTestSiteIdInterface( $node );
+       QUnit.test( 'initialization and destruction', function( assert ) {
+               var $node = $( '<div/>', { id: 'subject' } ),
+                       siteIdInterface = newTestSiteIdInterface( $node );
 
-               assert.ok(
-                       siteIdInterface._subject[0] === $node[0],
-                       'validated subject'
+               assert.strictEqual(
+                       siteIdInterface.getSubject()[0],
+                       $node[0],
+                       'verified subject'
                );
 
                siteIdInterface.startEditing();
@@ -64,65 +70,14 @@
                assert.equal(
                        siteIdInterface._currentResults.length,
                        2,
-                       'filled result set'
+                       'filled result set after startEditing()'
                );
 
                assert.equal(
                        siteIdInterface.getSelectedSiteId(),
                        null,
-                       'no site id selected'
+                       'no site id selected initially'
                );
-
-               assert.equal(
-                       siteIdInterface.setValue( wb.getSite( 'en' ).getId() ),
-                       wb.getSite( siteIds[0] ).getId(),
-                       'set value to site id'
-               );
-
-               assert.equal(
-                       siteIdInterface.getSelectedSite(),
-                       wb.getSite( siteIds[0] ),
-                       'verified selected site'
-               );
-
-               assert.equal(
-                       siteIdInterface.setValue( wb.getSite( 'de' 
).getShortName() ),
-                       wb.getSite( siteIds[1] ).getId(),
-                       'set value to input option label'
-               );
-
-               assert.equal(
-                       siteIdInterface.getSelectedSite(),
-                       wb.getSite( siteIds[1] ),
-                       'verified selected site'
-               );
-
-               assert.equal(
-                       siteIdInterface.setValue( '' ),
-                       null,
-                       'Set value to an empty string.'
-               );
-
-               var testSite = wb.getSite( siteIds[0] );
-               var testStrings = [
-                       testSite.getId(),
-                       testSite.getShortName(),
-                       testSite.getName() + ' (' + testSite.getId() + ')'
-               ];
-
-               $.each( testStrings, function( index, val ) {
-                       assert.equal(
-                               siteIdInterface.setValue( val ),
-                               testSite.getId(),
-                               'Set value to "' + val + '"'
-                       );
-
-                       assert.equal(
-                               siteIdInterface.getSelectedSiteId(),
-                               testSite.getId(),
-                               'get result-set match from string "' + val + '"'
-                       );
-               } );
 
                siteIdInterface.destroy();
 
@@ -131,7 +86,83 @@
                        0,
                        'no input element'
                );
+       } );
 
+       QUnit.test( 'valid input resulting into some site being selected', 
function( assert ) {
+
+
+               $.each( siteIds, function( i, siteId ) {
+                       var testSite = wb.getSite( siteId );
+                       var inputStrings = {
+                               'site\'s ID': testSite.getId()
+                               // TODO/FIXME: the following input strings have 
been used in the test before but
+                               //  the assertions only succeeded because the 
first defined string was a valid one
+                               //  and because all strings were tested on the 
same Interface instance. So the later
+                               //  input strings were not valid, but 
setValue() would return the current value
+                               //  which is the same as the expected one since 
all those strings represent the
+                               //  same site link.
+                               //  It is not clear what the definition for 
valid input is and on which level the
+                               //  normalization happens (perhaps this test 
should be moved to EditableSiteLink
+                               //  level.
+
+                               //'site\'s language code': 
testSite.getLanguageCode(),
+                               //'site\'s short name': testSite.getShortName(),
+                               //'site\'s name & language code': 
testSite.getName() + ' (' + testSite.getLanguageCode() + ')'
+                       };
+
+                       $.each( inputStrings, function( description, inputVal ) 
{
+                               var $node = $( '<div/>', { id: 'subject' } ),
+                                       siteIdInterface = 
newTestSiteIdInterface( $node );
+
+                               assert.strictEqual(
+                                       siteIdInterface.setValue( inputVal ),
+                                       testSite.getId(),
+                                       'set value to ' + description + ' "' + 
inputVal + '"'
+                               );
+
+                               assert.strictEqual(
+                                       siteIdInterface.getSelectedSite(),
+                                       testSite,
+                                       'site (id "' + testSite.getId() + '") 
selected from input string "' + inputVal + '"'
+                               );
+                       } );
+               } );
+       } );
+
+       QUnit.test( 'invalid input', function( assert ) {
+               var $node = $( '<div/>', { id: 'subject' } ),
+                       siteIdInterface = newTestSiteIdInterface( $node ),
+                       lastValidSiteId = 'dewiki';
+
+               assert.equal(
+                       siteIdInterface.setValue( lastValidSiteId ),
+                       lastValidSiteId,
+                       'set value to valid value (not null) first'
+               );
+
+               var lastValidSite = siteIdInterface.getSelectedSite();
+
+               // NOTE: the tested behavior is whether when entering an 
invalid value, the previous value
+               //       will still be set and returned by setValue(). This is 
very confusing behavior but
+               //       the behavior as specified by 
wb.ui.PropertyEditTool.EditableValue.Interface
+
+               assert.equal(
+                       siteIdInterface.setValue( '' ),
+                       lastValidSiteId,
+                       'set value to an empty string, invalid, previous valid 
value will be returned'
+               );
+
+               assert.strictEqual(
+                       siteIdInterface.getSelectedSite(),
+                       lastValidSite,
+                       'getSelectedSite() returns same site as before'
+               );
+
+               assert.strictEqual(
+                       siteIdInterface.getSelectedSiteId(),
+                       lastValidSiteId,
+                       'getSelectedSiteId() returns previously valid value'
+               );
        } );
 
        QUnit.test( 'init input with blacklist', function( assert ) {
@@ -151,7 +182,7 @@
                assert.equal(
                        siteIdInterface.getSelectedSiteId(),
                        null,
-                       'no site id slected'
+                       'no site id selected'
                );
 
                // set this to valid value

-- 
To view, visit https://gerrit.wikimedia.org/r/67559
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I1a625c1fed05bd85031c60f1ec0008b8a5951c89
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Daniel Werner <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to