Tobias Gritschacher has submitted this change and it was merged.
Change subject: Added test for valueview experts in general and StringValue
expert specifically
......................................................................
Added test for valueview experts in general and StringValue expert specifically
Introduces jQuery.valueview.tests.testExpert which will test a Expert
constructor on basis of a
given test definition. Added first expert test using this new function on the
StringValue expert.
Fixes some minor issues and updates documentation of jQuery.valueview.Expert
itself to pass the
new tests.
Change-Id: Ia681d6a8a31771cc000d5f1f423de25a77f6a6c2
---
M ValueView/ValueView.tests.qunit.php
M ValueView/resources/jquery.valueview/valueview.Expert.js
A
ValueView/tests/qunit/jquery.valueview/valueview.experts/experts.StringValue.tests.js
A ValueView/tests/qunit/jquery.valueview/valueview.tests.testExpert.js
4 files changed, 437 insertions(+), 8 deletions(-)
Approvals:
Tobias Gritschacher: Looks good to me, approved
jenkins-bot: Verified
diff --git a/ValueView/ValueView.tests.qunit.php
b/ValueView/ValueView.tests.qunit.php
index 81ebd1a..25606d3 100644
--- a/ValueView/ValueView.tests.qunit.php
+++ b/ValueView/ValueView.tests.qunit.php
@@ -82,6 +82,25 @@
'qunit.parameterize',
),
),
+
+ 'jquery.valueview.tests.testExpert.js' => array(
+ 'scripts' => array(
+
"$bp/jquery.valueview/valueview.tests.testExpert.js",
+ ),
+ 'dependencies' => array(
+ 'jquery.valueview.experts',
+ 'qunit.parameterize',
+ ),
+ ),
+
+ 'jquery.valueview.experts.stringvalue.tests' => array(
+ 'scripts' => array(
+
"$bp/jquery.valueview/valueview.experts/experts.StringValue.tests.js",
+ ),
+ 'dependencies' => array(
+ 'jquery.valueview.experts.stringvalue',
+ ),
+ ),
);
} );
diff --git a/ValueView/resources/jquery.valueview/valueview.Expert.js
b/ValueView/resources/jquery.valueview/valueview.Expert.js
index 4aed814..4579fc6 100644
--- a/ValueView/resources/jquery.valueview/valueview.Expert.js
+++ b/ValueView/resources/jquery.valueview/valueview.Expert.js
@@ -62,7 +62,8 @@
* @param {dv.util.Notifier} [valueViewNotifier] Required so the expert
can notify the valueview
* about certain events. The following notification keys can be
used:
* - change: will be sent when raw value displayed by the expert
changes. Either by a
- * user action or by calling the rawValue() method.
+ * user action or by calling the rawValue() method.
First parameter is a
+ * reference to the Expert itself.
* @param {Object} [options={}]
*
* TODO: think about whether there should be a function to add multiple
notifiers for widget
@@ -167,7 +168,7 @@
* @since 0.1
* @abstract
*
- * @return valueParsers.Parser
+ * @return valueParsers.ValueParser
*/
parser: function() {
return new vp.NullParser()
@@ -194,7 +195,8 @@
* value, so basically, the expert itself will do the job of
parsing itself somehow and does
* not require an asynchronous job for doing so.
* If the first parameter is set, then the function will set
the value instead of returning
- * it. An incompatible value will be recognized as empty (same
as null).
+ * it. An incompatible value will be recognized as empty (same
as null). If the given value
+ * is different from the current one, "change" will be notified
to the change notifier.
*
* @since 0.1
*
@@ -203,13 +205,24 @@
* Returns undefined if used as setter.
*/
rawValue: function( rawValue ) {
+ var currentRawValue = this._getRawValue();
+
if( rawValue === undefined ) { // GETTER:
- return this._getRawValue();
+ return currentRawValue;
}
- // SETTER:
- this._setRawValue( rawValue );
- this.draw();
- this._viewNotifier.notify( 'change' );
+
+ // Only change value if different from current value:
+ if( !this.rawValueCompare( currentRawValue, rawValue )
) { // SETTER:
+ this._setRawValue( rawValue );
+
+ // rawValue might be a unknown value different
from null which will end as null
+ // nonetheless. If that is the case the value
was null already, then this is not
+ // a real update.
+ if( currentRawValue !== null ||
this._getRawValue() !== null ) {
+ this.draw();
+ this._viewNotifier.notify( 'change', [
this ] );
+ }
+ }
},
/**
diff --git
a/ValueView/tests/qunit/jquery.valueview/valueview.experts/experts.StringValue.tests.js
b/ValueView/tests/qunit/jquery.valueview/valueview.experts/experts.StringValue.tests.js
new file mode 100644
index 0000000..1a8aa9f
--- /dev/null
+++
b/ValueView/tests/qunit/jquery.valueview/valueview.experts/experts.StringValue.tests.js
@@ -0,0 +1,25 @@
+/**
+ * @since 0.1
+ * @ingroup ValueView
+ *
+ * @licence GNU GPL v2+
+ * @author Daniel Werner < daniel.wer...@wikimedia.de >
+ */
+ ( function( $, QUnit, valueview, StringParser ) {
+
+ 'use strict';
+
+ var testExpert = valueview.tests.testExpert;
+
+ QUnit.module( 'jquery.valueview.experts.StringValue' );
+
+ testExpert( {
+ expertConstructor: valueview.experts.StringValue,
+ rawValues: {
+ valid: [ 'foo bar', '42', '*(&#$@#*$' ],
+ unknown:
testExpert.basicTestDefinition.rawValues.unknown.concat( [ 42 ] )
+ },
+ relatedValueParser: StringParser
+ } );
+
+}( jQuery, QUnit, jQuery.valueview, valueParsers.StringParser ) );
diff --git
a/ValueView/tests/qunit/jquery.valueview/valueview.tests.testExpert.js
b/ValueView/tests/qunit/jquery.valueview/valueview.tests.testExpert.js
new file mode 100644
index 0000000..32d1ef3
--- /dev/null
+++ b/ValueView/tests/qunit/jquery.valueview/valueview.tests.testExpert.js
@@ -0,0 +1,372 @@
+/**
+ * @since 0.1
+ * @ingroup ValueView
+ *
+ * @licence GNU GPL v2+
+ * @author Daniel Werner < daniel.wer...@wikimedia.de >
+ */
+ jQuery.valueview.tests = jQuery.valueview.tests || {};
+ jQuery.valueview.tests.testExpert = ( function( $, QUnit, valueview,
Notifier, ValueParser ) {
+
+'use strict';
+
+ /**
+ * Helper which returns a string describing some value in more detail. The
string will hold a
+ * quoted representation of the value and a note of what type the value is.
+ *
+ * @param {*} value
+ * @return string
+ */
+function valueDescription( value ) {
+ return '"' + value + '" (' + Object.prototype.toString.call( value ) +
')';
+}
+
+ /**
+ * Tests different aspects of a valueview expert.
+ * @since 0.1
+ *
+ * TODO: Test error cases.
+ *
+ * @param {Object} testDefinition See testExpert.basicTestDefinition for
documentation.
+ */
+function testExpert( testDefinition ) {
+ // Throw error if something is wrong with given test definition:
+ testExpert.verifyTestDefinition( testDefinition );
+
+ var Expert = testDefinition.expertConstructor,
+ validRawValues = testDefinition.rawValues.valid,
+ validRawValue = validRawValues[0],
+ unknownRawValues = testDefinition.rawValues.unknown,
+ unknownRawValue = unknownRawValues[0];
+
+ // Add null (empty) to list of valid values:
+ if( $.inArray( null, validRawValues ) > 0 ) {
+ throw new Error( 'null should not be part of the list of valid
values since it will be ' +
+ 'added by default' );
+ }
+ validRawValues.push( null );
+
+ // Used as source for expertProvider().
+ var expertDefinitions = [
+ {
+ title: 'instance without notifier',
+ args: [
+ $( '<span/>' ),
+ new valueview.MockViewState()
+ ]
+ }, {
+ title: 'instance with notifier',
+ args: [
+ $( '<div/>' ),
+ new valueview.MockViewState(),
+ new Notifier()
+ ]
+ }, {
+ title: 'instance with ViewState of disabled view',
+ args: [
+ $( '<div/>' ),
+ new valueview.MockViewState( { isDisabled: true
} ),
+ new Notifier()
+ ]
+ }
+ ];
+
+ /**
+ * Returns an array of Objects holding the following fields:
+ * - title: Description of the set.
+ * - expert: A valueview Expert instance.
+ * - constructorArgs: Arguments used to construct the expert given in
the "expert" field.
+ *
+ * @returns {Array}
+ */
+ function expertProvider() {
+ var experts = [];
+
+ $.each( expertDefinitions, function( i, definition ) {
+ var $viewPort = definition.args[0],
+ viewState = definition.args[1],
+ notifier = definition.args[2];
+
+ var expert = new Expert( $viewPort, viewState, notifier
);
+
+ experts.push( {
+ title: definition.title,
+ expert: expert,
+ constructorArgs: definition.args
+ } );
+ } );
+
+ return experts;
+ }
+
+ var expertCases = QUnit.cases( expertProvider() );
+
+ expertCases.test( 'constructor', function( args, assert ) {
+ assert.ok(
+ args.expert instanceof Expert,
+ 'expert successfully constructed'
+ );
+ assert.ok(
+ args.expert instanceof valueview.Expert,
+ 'expert instance of jQuery.valueview.Expert'
+ );
+ } );
+
+ expertCases.test( 'parser', function( args, assert ) {
+ var valueParser = args.expert.parser();
+
+ assert.ok(
+ valueParser instanceof ValueParser,
+ 'parser() returns a value parser instance'
+ );
+ assert.ok(
+ valueParser instanceof
testDefinition.relatedValueParser,
+ 'parser() returns instance of the expected value parser
constructor'
+ );
+ } );
+
+ expertCases.test( 'viewState', function( args, assert ) {
+ var viewState = args.expert.viewState();
+ assert.ok(
+ viewState instanceof valueview.ViewState,
+ 'viewState() returns a jQuery.valueview.ViewState
instance'
+ );
+ } );
+
+ expertCases.test( 'rawValueCompare: Test of different raw values',
function( args, assert ) {
+ $.each( validRawValues, function( i, testValue ) {
+ $.each( validRawValues, function( j, otherValue ) {
+ var successExpected = i === j;
+
+ assert.ok(
+ args.expert.rawValueCompare( testValue,
otherValue ) === successExpected,
+ 'Raw value "' + valueDescription(
testValue ) + ' does ' +
+ ( successExpected ? '' : 'not '
) + 'equal raw value "' +
+ valueDescription( otherValue )
+ );
+ } );
+ } );
+ } );
+
+ expertCases.test( 'rawValueCompare: Works with 2nd parameter omitted',
function( args, assert ) {
+ var expert = args.expert;
+
+ assert.ok(
+ expert.rawValueCompare( null ),
+ '"rawValueCompare( null )" is true for newly
initialized expert'
+ );
+
+ expert.rawValue( validRawValue );
+
+ assert.ok(
+ expert.rawValueCompare( validRawValue ),
+ '"rawValueCompare( value )" is true after "rawValue(
value )"'
+ );
+ } );
+
+ expertCases.test( 'rawValue: setting and getting raw value', function(
args, assert ) {
+ var expert = args.expert;
+
+ $.each( validRawValues, function( i, testValue ) {
+ expert.rawValue( testValue );
+
+ assert.ok(
+ true,
+ 'Changed value via "rawValue( value )". "value"
is ' + valueDescription( testValue )
+ );
+ assert.ok(
+ expert.rawValueCompare( expert.rawValue(),
testValue ),
+ 'The new value has been received via
"rawValue()"'
+ );
+ } );
+ } );
+
+ expertCases.test( 'rawValue: setting value to unknown value', function(
args, assert ) {
+ var expert = args.expert;
+
+ $.each( unknownRawValues, function( i, testValue ) {
+ expert.rawValue( testValue );
+
+ assert.ok(
+ true,
+ 'Changed value via "rawValue( value )". "value"
is ' + valueDescription( testValue )
+ );
+ assert.ok(
+ expert.rawValueCompare( expert.rawValue(), null
),
+ 'The new value returned by "rawValue()" is null
(empty value)'
+ );
+ } );
+ } );
+
+ var expertCasesMemberCallTest = function( memberName ) {
+ expertCases.test( memberName, function( args, assert ) {
+ args.expert[ memberName ]();
+ assert.ok(
+ true,
+ memberName + '() has been called'
+ );
+ } );
+ };
+ expertCasesMemberCallTest( 'draw' );
+ expertCasesMemberCallTest( 'focus' );
+ expertCasesMemberCallTest( 'blur' );
+
+ // Separate test for change notification:
+ QUnit.test( 'Expert change notification', 10, function( assert ) {
+ // Helper flags for tests:
+ var notified = false,
+ newValue;
+
+ // Notifier with callback to set flag above after "change"
notification.
+ var notifier = new Notifier( {
+ change: function( expert, arg2 ) {
+ notified = true;
+ assert.ok(
+ arguments.length === 1 && expert
instanceof Expert,
+ '"change" notification received, first
argument is expert'
+ );
+ assert.ok(
+ expert.rawValueCompare( newValue,
expert.rawValue() ),
+ 'Value has been changed to expected
value'
+ );
+ }
+ } );
+
+ var expert = new Expert(
+ $( '<div/>' ),
+ new valueview.MockViewState(),
+ notifier
+ );
+
+ assert.ok( // +1
+ !notified,
+ 'Change notification has not been triggered initially'
+ );
+
+ function testChangeRawValue( rawValue, changeExptected,
testDescription ) {
+ notified = false;
+ newValue = rawValue;
+ expert.rawValue( rawValue );
+
+ var msg = changeExptected
+ ? 'Changing the expert\'s raw value has
triggered a "change" notification after '
+ : 'No "change" notification has been triggered
since the expert\'s value did not ' +
+ 'change after ';
+ msg += testDescription;
+
+ assert.ok( changeExptected === notified, msg );
+ }
+
+ testChangeRawValue( null, false, 'changing value to empty after
initialization ' +
+ '(value should be empty already)' ); // +1
+ testChangeRawValue( validRawValue, true, 'changing value to
valid value' ); // +3 assertions
+ testChangeRawValue( validRawValue, false, 'attempt to change
value to current value' ); // +1
+ testChangeRawValue( null, true, 'change value to empty value
and not' ); // +3
+ testChangeRawValue( unknownRawValue, false, 'changing value to
unknown value which will ' +
+ 'be interpreted as empty value' ); // +1
+ } );
+}
+
+ /**
+ * Object holding all fields required by testExpert's first argument's object.
+ * @since 0.1
+ * @type Object
+ */
+testExpert.basicTestDefinition = {
+ /**
+ * A jQuery.valueview.Expert implementation's constructor to be tested.
+ * @type Function
+ */
+ expertConstructor: valueview.experts.StringValue,
+ /**
+ * Definition of different raw values. Holds different fields for
different kinds of raw values.
+ * The keys "valid" and "unknown" should each hold at least one value
in their array of values
+ * and the values must not have any duplicates.
+ * @type Object
+ */
+ rawValues: {
+ /**
+ * Array of valid raw values.
+ * @type {*[]}
+ */
+ valid: [],
+ /**
+ * Array of unknown raw values. These values are expected to be
interpreted as "null" by the
+ * Expert. null must not be part of the list.
+ */
+ unknown: [
+ [], // array
+ {}, // plain object
+ new function NotSoPlainObject() {}(), // not-so-plain
object
+ /regex/, // regex
+ $.noop, // function
+ Number['NaN'] // NaN
+ ]
+ },
+ /**
+ * Defines what kind of value parser the Expert's parse() function is
exptected to return.
+ * @type Function Constructor implementation of
valueParsers.ValueParser.
+ */
+ relatedValueParser: null
+};
+
+ /**
+ * Will validate a test definition for the "testExpert". Throws an error if
something is wrong with
+ * the given test definition.
+ *
+ * @param {Object} testDefinition
+ */
+testExpert.verifyTestDefinition = function( testDefinition ) {
+ function verifyTestDefinitionRawValues( rawValueDefinitions ) {
+ var allValues = [],
+ value, i;
+
+ if( !$.isPlainObject( rawValueDefinitions )
+ || !rawValueDefinitions.unknown
+ || !rawValueDefinitions.valid
+ ) {
+ throw new Error( 'Test definition\'s raw value
definitions require on set of values for ' +
+ '"valid" and one set of values for "unknown"
values' );
+ }
+
+ $.each( rawValueDefinitions, function( valuesType, setOfValues
) {
+ if( setOfValues.length < 1 ) {
+ throw new Error( 'Test definition\'s set of
values "' + valuesType + '" has to be an ' +
+ 'array of at least one value
representing the type of value' );
+ }
+ allValues = allValues.concat( setOfValues );
+ } );
+
+ for( i = allValues.length - 1; i >= 0; i-- ) {
+ value = allValues[i];
+ if( value === undefined ) {
+ throw new Error( 'Expert test definition\'s
sets of values given in the rawValue ' +
+ 'field must not contain undefined as a
value' );
+ }
+ if(
+ $.inArray( value, allValues ) < i
+ && (
+ typeof value !== 'number'
+ || !isNaN( value ) // NaN might be a
unknown value, but NaN !== NaN, $.inArray returns -1
+ )
+ ) {
+ throw new Error( 'Expert test definition\'s
sets of values given in the rawValue ' +
+ 'field must be sets of unique values.
E.g. a unknown value can not be a valid ' +
+ 'value at the same time. Value ' +
valueDescription( value ) + ' is a duplicate' );
+ }
+ }
+ }
+
+ verifyTestDefinitionRawValues( testDefinition.rawValues );
+
+ if( !testDefinition.expertConstructor
+ || !( testDefinition.expertConstructor.prototype instanceof
valueview.Expert )
+ ) {
+ throw new Error( 'Test definition\'s "expertConstructor" field
has to hold a constructor ' +
+ 'implementing jQuery.valueview.Expert' );
+ }
+};
+
+return testExpert; // expose
+
+}( jQuery, QUnit, jQuery.valueview, dataValues.util.Notifier,
valueParsers.ValueParser ) );
--
To view, visit https://gerrit.wikimedia.org/r/62765
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia681d6a8a31771cc000d5f1f423de25a77f6a6c2
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/extensions/DataValues
Gerrit-Branch: master
Gerrit-Owner: Daniel Werner <daniel.wer...@wikimedia.de>
Gerrit-Reviewer: Tobias Gritschacher <tobias.gritschac...@wikimedia.de>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
