jenkins-bot has submitted this change and it was merged.
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.
* Some documentation clarifications.
Change-Id: I1a625c1fed05bd85031c60f1ec0008b8a5951c89
---
M lib/resources/wikibase.ui.PropertyEditTool.EditableValue.Interface.js
M
lib/tests/qunit/wikibase.ui.PropertyEditTool.EditableValue.SiteIdInterface.tests.js
2 files changed, 106 insertions(+), 70 deletions(-)
Approvals:
Henning Snater: Looks good to me, approved
jenkins-bot: Verified
diff --git
a/lib/resources/wikibase.ui.PropertyEditTool.EditableValue.Interface.js
b/lib/resources/wikibase.ui.PropertyEditTool.EditableValue.Interface.js
index 3faef5e..dd591be 100644
--- a/lib/resources/wikibase.ui.PropertyEditTool.EditableValue.Interface.js
+++ b/lib/resources/wikibase.ui.PropertyEditTool.EditableValue.Interface.js
@@ -392,9 +392,14 @@
/**
* Sets a value.
- * Returns the value really set in the end. This string can be
different from the given value
- * since it will go through some normalization first.
- * Won't change the value and return in case it was invalid.
+ * Returns the current value after setting has been performed. The
returned string can be
+ * different from the given value since the given value will go through
some normalization
+ * first.
+ *
+ * IMPORTANT: If the given value is invalid, no change will happen and
the previous value will
+ * be returned. This might be confusing behavior since, in
the "new UI", setting
+ * invalid values will result into emptying the widget's
value (setting the value
+ * to null)
*
* @param string value
* @return string|null same as value but normalized, null in case the
value was invalid
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..2d61659 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: merged
Gerrit-Change-Id: I1a625c1fed05bd85031c60f1ec0008b8a5951c89
Gerrit-PatchSet: 9
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Daniel Werner <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Henning Snater <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
