Opened 7 years ago

Closed 5 years ago

Last modified 5 years ago

#15590 closed defect (fixed)

fix robot timing inconsistencies

Reported by: bill Owned by: bill
Priority: undecided Milestone: 1.7.6
Component: TestFramework Version: 1.7.3
Keywords: Cc:
Blocked By: Blocking:

Description

This ticket covers fixes to doh.robot to make the timing behavior more consistent, to avoid spurious failures due to unexpected delays of robot events.

Attachments (2)

dohEvents2.patch​ (23.7 KB) - added by bill 7 years ago.
Patch to maintain listeners on <body> rather then creating/destroying them on every robot call. Like dohEvents1.patch (which is probably better than this one), shares the problem that a stopPropagation() call on IE will mask us from seeing the event. For mouse events, really need to connect to the node that the mouse is over, using document.elementFromPoint(). For keyboard it might be good enough to connect to <body>.
dohEvents1.patch​ (9.0 KB) - added by bill 7 years ago.
Patch to setup new event handler for each robot call. Needs to be enhanced to ignore mousemove notifications if they don't match the requested coordinates (see dohEvents2.patch)

Download all attachments as: .zip

Change History (16)

comment:1 Changed 7 years ago by bill

Milestone: tbd → 1.8
Owner: set to bill
Status: new → assigned

comment:2 Changed 7 years ago by bill

In [29098]:

Since setTimeout(f, n) can take longer than n milliseconds, it's better to schedule a sequence of actions by waiting until the first setTimeout() finishes before calling the second setTimeout(). Refactoring robot code to do this, and hopefully avoid some intermittent timing failures.

As a side benefit, enhances sequence() so that the callback function can return a Promise, and execution won't continue until the Promise resolves.

Also rearranged typeKeys() so that all the timing delays happen in Javascript, rather than doing Thread.sleep() calls from java. (Thread.sleep() can have the same problem as setTimeout() of "oversleeping".) Like above, it doesn't set the timer for a next keystroke until the previous keystroke has finished.

Finally, added code to make sure that when a test fixture fails (due to an assertion failing, timeout, etc.), the remaining sequence() registered actions for that fixture are cleared, so that they don't interfere with the next test fixture.

This change may increase test runtime since many of the tests currently set unnecessarily large delays and durations to workaround robot delays. (For that reason I had to increase the timeouts for a few test fixtures.)

Refs #15590 !strict.

comment:3 Changed 7 years ago by bill

In [29099]:

just adding some guard code to avoid an exception when robot doesn't initialize correctly, or user hits refresh button during initialization, refs #15590 !strict.

comment:4 Changed 7 years ago by bill

In [29101]:

Leverage dojo/Deferred to chain actions registered via sequence(), refs #15590 !strict.

comment:5 Changed 7 years ago by bill

In [29202]:

Do mousemove timing in javascript, rather than in java. This is getting intermittent failures on Tree_dnd.html but I think I can resolve them after the next checkin, where I monitor events on the document to tell when java robot actually executes each command. Refs #15590 !strict.

comment:6 Changed 7 years ago by bill

In [29227]:

Fix bug where _positionMouseOnLine() was accessing the version of lastMousePos from when the test fixture started (i.e. using the position of the mouse when the test fixture started) rather than the lastMousePos from when the mouseMove() or mouseMoveAt() command started executing.

Refs #15590 !strict

comment:7 Changed 7 years ago by bill

In [29228]:

small simplification, refs #15590 !strict

comment:8 Changed 7 years ago by bill

In [29258]:

Further refinements to moveTo() method:

  1. don't call java robot to move the mouse to a position that it's already at
  2. fix easeInOutQuad() method to do rounding on all return values
  3. refactor, simplifying how mousemove events are queued


Refs #15590 !strict.

Changed 7 years ago by bill

Attachment: dohEvents2.patch​ added

Patch to maintain listeners on <body> rather then creating/destroying them on every robot call. Like dohEvents1.patch (which is probably better than this one), shares the problem that a stopPropagation() call on IE will mask us from seeing the event. For mouse events, really need to connect to the node that the mouse is over, using document.elementFromPoint(). For keyboard it might be good enough to connect to <body>.

Changed 7 years ago by bill

Attachment: dohEvents1.patch​ added

Patch to setup new event handler for each robot call. Needs to be enhanced to ignore mousemove notifications if they don't match the requested coordinates (see dohEvents2.patch)

comment:9 Changed 7 years ago by bill

Milestone: 1.8 → 2.0

Deferring further work until 2.0. The remaining task, as prototyped in the patches above, is to monitor mouse and keyboard events in the tested document, to know when robot has executed the command that you asked it to execute.

However, it's unclear if that code will still be needed if/when we switch from java robot to WebDriver (see ​http://bugs.dojotoolkit.org/attachment/ticket/12740/12740_util.patch). Presumably with WebDriver you send it a command over XHR, and the XHR doesn't return a value until the command has completed. If that's true, there's no reason to monitor events.

comment:10 Changed 5 years ago by bill

Milestone: 2.0 → 1.8
Resolution: → fixed
Status: assigned → closed

Actually, let's just mark this fixed. The future is webdriver, so I don't think we'll have any more timing fixes for doh/robot.

Mark is backporting a bunch of DOH changes to 1.7 at which point this milestone should be changed to 1.7.6.

comment:11 Changed 5 years ago by Bill Keese <bill@…>

In 59ba49664c1a6b0feb13623be32e10b9679d5cbb/util:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled?

comment:12 Changed 5 years ago by Bill Keese <bill@…>

In a09078ac37bf6a91a885089aa9712939fa39b4b9/util:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled?

comment:13 Changed 5 years ago by Bill Keese <bill@…>

In c1b47070cd9055bdc4797e4bb6aa39bd472dbb9a/dijit:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled?

comment:14 Changed 5 years ago by bill

Milestone: 1.8 → 1.7.6
Note: See TracTickets for help on using tickets.