Skip to content

Commit

Permalink
Update test to work properly on IE
Browse files Browse the repository at this point in the history
  • Loading branch information
@apipkin
apipkin committed Mar 6, 2013
1 parent c160f44 commit c0af401
Showing 1 changed file with 11 additions and 11 deletions.
22 changes: 11 additions & 11 deletions src/event-valuechange/tests/unit/assets/event-valuechange-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,21 +178,21 @@ suite.add(new Y.Test.Case({
},

'valuechange should not report stale changes that occurred while a node was not focused - with focus() and blur()': function () {
var fired = false;
this.wait(function () {
var fired = false;

this.textInput.simulate('mousedown');
this.textInput.set('value', 'foo');
this.textInput.simulate('mousedown');
this.textInput.set('value', 'foo');

this.textInput.on('valuechange', function (e) {
fired = true;
});
this.textInput.on('valuechange', function (e) {
fired = true;
});

this.textInput.blur();
this.textInput.set('value', 'bar');
this.textInput.focus();
this.textInput.simulate('mousedown');
this.textInput.blur();
this.textInput.set('value', 'bar');
this.textInput.focus();
this.textInput.simulate('mousedown');

this.wait(function () {
Assert.isFalse(fired);
}, 200);
},
Expand Down

3 comments on commit c0af401

@apipkin
Copy link
Contributor Author

@apipkin apipkin commented on c0af401 Mar 6, 2013

Choose a reason for hiding this comment

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

This commit was just moving the this.wait(function () { to the first line of the test function and indenting the contents of the test. I'm not sure why the diff is making it look more than that.

@ericf
Copy link
Member

@ericf ericf commented on c0af401 Mar 6, 2013

Choose a reason for hiding this comment

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

The diff is showing white spaces changes as well. If you append ?w=1 to the URL it will remove them: c0af401?w=1

@sdesai
Copy link
Contributor

@sdesai sdesai commented on c0af401 Mar 6, 2013

Choose a reason for hiding this comment

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

I don't think this is the right root fix. As mentioned in my findings, it indicates something about a prior test (or the setup) is not fully complete and/or reversed by the time it gets to this test (that is, it's not starting with a clean state). That's why adding the wait() as the first thing in the test 'fixes' it. With the wait you're basically saying run this test 200ms later than you would normally have run it. If you remove all the other tests in the file, aside from this one, it'll pass, in it's original form, which again, indicates one of the prior tests (which I isolated to 2 specific ones in the spreadsheet) is interfering with the above test.

My gut feel is that the polling from one of the prior tests is not completely done, and overlaps with this test, so finding a way to conclusively stop polling during teardown (if that's the root issue) may be a more robust root fix, but that's as far as I looked into it. For example, when I added a blur() to the resume/asserts of the prior 2 tests, it also fixed the issue.

Please sign in to comment.