fix flakiness in testRepeatedHandlers

testRepeatedHandlers is occasionally flaky. I'm unable to reproduce the
flakiness, but I think it's a timing issue.

the test has a mouse event handler that uses setTimeout to call a
check function, but the test driver can click and trigger the
event handler multiple times before the setTimeout fires.

I found the existing logic pretty confusing, and I didn't see any
sensible way of fixing it as-is, so I rewrote the logic to
be more linear and avoid setTimeout.

[kpreid2, 2013-06-26 16:55:33]

https://codereview.appspot.com/10572043/diff/1/tests/com/google/caja/plugin/e...
File tests/com/google/caja/plugin/es53-test-domado-events-guest.html (right):

https://codereview.appspot.com/10572043/diff/1/tests/com/google/caja/plugin/e...
tests/com/google/caja/plugin/es53-test-domado-events-guest.html:371: var clicks
= 0;
I don't like the name 'clicks' because it suggests this is a counter rather than
state-machine (more regular than irregular behavior). How about 'state', 'step',
'phase', or 'progress'?

https://codereview.appspot.com/10572043/diff/1/tests/com/google/caja/plugin/e...
tests/com/google/caja/plugin/es53-test-domado-events-guest.html:388:
container.removeEventListener('mouseup', check, false);
'check' here is bound to the named function which is !== to the jsunitCallback
wrapper which was added, so this removeEventListener does nothing.

Also, if it throws for some reason, the test will "pass then fail", so I'd
rather see removeEventListener(…) before pass(…) just for cleaner output. No
harm either way.

[felix8a, 2013-07-01 20:39:41]

testRepeatedHandlers is occasionally flaky. I'm unable to reproduce the
flakiness, but I think it's a timing issue.

the test has a mouse event handler that uses setTimeout to call a
check function, but the test driver can click and trigger the
event handler multiple times before the setTimeout fires.

I found the existing logic pretty confusing, and I didn't see any
sensible way of fixing it as-is, so I rewrote the logic to
be more linear and avoid setTimeout.

[felix8a, 2013-07-01 20:39:49]

https://codereview.appspot.com/10572043/diff/1/tests/com/google/caja/plugin/e...
File tests/com/google/caja/plugin/es53-test-domado-events-guest.html (right):

https://codereview.appspot.com/10572043/diff/1/tests/com/google/caja/plugin/e...
tests/com/google/caja/plugin/es53-test-domado-events-guest.html:371: var clicks
= 0;
On 2013/06/26 16:55:33, kpreid2 wrote:
> I don't like the name 'clicks' because it suggests this is a counter rather
than
> state-machine (more regular than irregular behavior). How about 'state',
'step',
> 'phase', or 'progress'?

Done.

https://codereview.appspot.com/10572043/diff/1/tests/com/google/caja/plugin/e...
tests/com/google/caja/plugin/es53-test-domado-events-guest.html:388:
container.removeEventListener('mouseup', check, false);
On 2013/06/26 16:55:33, kpreid2 wrote:
> 'check' here is bound to the named function which is !== to the jsunitCallback
> wrapper which was added, so this removeEventListener does nothing.

ah, yeah, good catch. fixed.

[kpreid2, 2013-07-02 18:00:28]

LGTM

[felix8a, 2013-07-02 18:39:35]

@r5466