Closed Bug 779250 Opened 12 years ago Closed 12 years ago

Make Robocop tests (and maybe some others) run faster


(Testing :: General, defect)

Not set


(Not tracked)



(Reporter: gbrown, Assigned: gbrown)



(1 file, 1 obsolete file)

I have noticed a couple of places where our test framework just waits around for no apparent reason while running robocop tests. I think we can save 48 seconds per test, or about 16 minutes per tbpl robocop run. We might save additional time on mochitests and other tests too.

1. See devicemanagerSUT.fireProc: The intent of the loop is to wait 30 (actually 33) seconds for the process to start. But actually, in the normal case for Robocop, runCmds(exec...) has already waited for the process to start and end before this loop is executed. When the loop executes, the ps process list does not contain a fennec entry, but the loop still waits for 33 seconds (I'm not sure of why exactly).
2. See remoteautomation.RProcess.init: which sleeps for 15 seconds ... why?
Try run looks good:

Note that all 3 rc runs are about 41 minutes, compared to about 60 minutes for most of our current runs.
Attached patch avoid unnecessary waits (obsolete) — Splinter Review
Attachment #649281 - Flags: review?(jmaher)
Comment on attachment 649281 [details] [diff] [review]
avoid unnecessary waits

Review of attachment 649281 [details] [diff] [review]:

The Try server run looks great.  Can we test on XUL also?  I don't want to check this in and break other stuff.

::: build/mobile/
@@ +507,5 @@
>      except AgentError:
>        return None
> +    # The 'exec' command usually waits for the process to start and end, so checking
> +    # for the process here often results in process = None.

this is actually the case for robocop, but not for mochitest/reftest.  Talos is a different ball of wax.  We used to not wait in sutagent for the process to start, and this is why this code is in here.

::: build/mobile/
@@ +153,5 @@
>              # Setting timeout at 1 hour since on a remote device this takes much longer
>              self.timeout = 3600
> +            # The need/benefit of the following sleep is unclear; it was formerly 15 seconds
> +            time.sleep(1)

we added the time.sleep(15) in here to allow for fennec to properly terminate.  Maybe robocop and fennec are both better now.
Attachment #649281 - Flags: review?(jmaher) → review+
Updated comment (only) to acknowledge that the processExist call may or may not succeed (it usually does for mochitest/reftest; it usually doesn't for robocop).
Attachment #649281 - Attachment is obsolete: true
Attachment #650115 - Flags: review+
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in before you can comment on or make changes to this bug.