Skip to content
This repository has been archived by the owner on Feb 22, 2018. It is now read-only.

Commit

Permalink
fitest(ngModel): Do not use input selection*
Browse files Browse the repository at this point in the history
IDL attributes for input type=number tests

Fix#588

Closes#589
  • Loading branch information
chalin authored and mhevery committed Feb 20, 2014
1 parent 70d83c5 commit c09745b
Showing 1 changed file with 24 additions and 32 deletions.
56 changes: 24 additions & 32 deletions test/directive/ng_model_spec.dart
Original file line number Diff line number Diff line change
Expand Up @@ -111,17 +111,15 @@ describe('ng-model', () {
}));
});

/* An input of type number can only have assigned to its [value] field
* a string that represents a valid number. Any attempts to assign
* any other string will have no effect on the [value] field.
*
* This function simulates typing the given string value into the input
* field regardless of whether it represents a valid number or not. It
* has the side-effect of setting the window focus to the input.
/* This function simulates typing the given text into the input
* field. The text will be added wherever the insertion point
* happens to be. This method has as side-effect to set the
* focus on the input (without setting the focus the text
* dispatch may not work).
*/
void setNumberInputValue(InputElement input, String value) {
void simulateTypingText(InputElement input, String text) {
input..focus()
..dispatchEvent(new TextEvent('textInput', data: value));
..dispatchEvent(new TextEvent('textInput', data: text));
}

describe('type="number" like', () {
Expand All @@ -131,43 +129,37 @@ describe('ng-model', () {
var element = _.compile('<input type="number" ng-model="$modelFieldName">');
dom.querySelector('body').append(element);

// Subcase: text not representing a valid number.
var piX = '3.141x';
setNumberInputValue(element, piX);
// This test will progressively enter the text '1e1'
// '1' is a valid number.
// '1e' is not a valid number.
// '1e1' is again a valid number (with an exponent)

simulateTypingText(element, '1');
_.triggerEvent(element, 'change');
expect(element.value).toEqual('1');
expect(_.rootScope.context[modelFieldName]).toEqual(1);

simulateTypingText(element, 'e');
// Because the text is not a valid number, the element value is empty.
expect(element.value).toEqual('');
// But the selection can confirm that the text is there:
element.selectionStart = 0;
element.selectionEnd = piX.length;
expect(dom.window.getSelection().toString()).toEqual(piX);
// When the input is invalid, the model is [double.NAN]:
_.triggerEvent(element, 'change');
expect(_.rootScope.context[modelFieldName].isNaN).toBeTruthy();

// Subcase: text representing a valid number (as if the user had erased
// the trailing 'x').
num pi = 3.14159;
setNumberInputValue(element, pi.toString());
simulateTypingText(element, '1');
_.triggerEvent(element, 'change');
expect(element.value).toEqual(pi.toString());
expect(_.rootScope.context[modelFieldName]).toEqual(pi);
expect(element.value).toEqual('1e1');
expect(_.rootScope.context[modelFieldName]).toEqual(10);
}));

it('should not reformat user input to equivalent numeric representation', inject((Injector i) {
var modelFieldName = 'modelForNumFromInvalid2';
var element = _.compile('<input type="number" ng-model="$modelFieldName">');
dom.querySelector('body').append(element);

var ten = '1e1';
setNumberInputValue(element, ten);
_.triggerEvent(element, 'change');
expect(_.rootScope.context[modelFieldName]).toEqual(10);
// Ensure that the input text is literally the same
element.selectionStart = 0;
// Set the selectionEnd to one beyond ten.length in
// case angular added some extra text.
element.selectionEnd = ten.length + 1;
expect(dom.window.getSelection().toString()).toEqual(ten);
simulateTypingText(element, '1e-1');
expect(element.value).toEqual('1e-1');
expect(_.rootScope.context[modelFieldName]).toEqual(0.1);
}));

it('should update input value from model', inject(() {
Expand Down

6 comments on commit c09745b

@chalin
Copy link
Contributor Author

@chalin chalin commented on c09745b Feb 20, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mhevery for some reason #588 and #589 are not showing up as closed on my side. Is there something special you or I needed to do or is this meant to be automatic. If the latter, then it may be because of the lack of space characters in the commit message just before the issue number: namely "Closes#589" rather than "Closes #589". Maybe just a minor bookkeeping issue but I figured I would point it out in case it affects something (e.g., automated change log generation scripts).

@chalin
Copy link
Contributor Author

@chalin chalin commented on c09745b Feb 20, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mhevery. One more question about the commit title: is "fitest" a label new category, maybe meant to be "fixtest"? Either way, if so, I can update the CONTRIBUTING.md to reflect this new category; and while I am at it, I would suggest broadening the "test" category: it currently reads:

 test: Adding missing tests

Is "adding missing tests" the only reason "test" should be used? What if tests are refactored or fixed or improved?

@bgourlie
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a "refactor" category, so I don't think it's necessary to broaden the "tests" category.

@chalin
Copy link
Contributor Author

@chalin chalin commented on c09745b Feb 20, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bgourlie: possibly, but the categories/types "feat", "fix", "refactor" etc seem to implicitly apply to the angular.dart code proper, not the tests. Otherwise, the "test" category would be superfluous --- one could simply use "feat" to indicate the addition of a new test.

@chalin
Copy link
Contributor Author

@chalin chalin commented on c09745b Feb 20, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, I am not saying that I am right in my interpretation, I have just wondered a few times, how to structure my commit title when e.g., work is only done on tests.

@chalin
Copy link
Contributor Author

@chalin chalin commented on c09745b Feb 20, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the noise @mhevery, it seems that I had the privileges to mark #588 and #589 as merged and closed. Done.

Please sign in to comment.