Closed Bug 1435624 Opened 6 years ago Closed 6 years ago

With runByManifest, Android mochitest-chrome copies all test files to device for each manifest

Categories

(Testing :: Mochitest, enhancement)

Version 3
enhancement
Not set
normal

Tracking

(firefox60 fixed)

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: gbrown, Assigned: gbrown)

References

Details

Attachments

(1 file)

There is a significant opportunity for optimization here. Without the optimization, most android mochitest-chrome tasks timeout with runByManifest.
It is easy to guard against pushing the chrome files twice. The bigger problem is that we currently need to push them for every manifest because cleanup() is called for every manifest, and cleanup() is the only chance for runtestsremote to delete the chrome files once they have been pushed. So I've changed the cleanup protocol a little: after each manifest, call cleanup(final=False), then after everything is done, call cleanup(final=True).

https://treeherder.mozilla.org/#/jobs?repo=try&revision=79ea06387478265c73c7cb2acc7d8c6fd18de485
Attachment #8948868 - Flags: review?(jmaher)
Comment on attachment 8948868 [details] [diff] [review]
copy chrome tests only once

Review of attachment 8948868 [details] [diff] [review]:
-----------------------------------------------------------------

one question

::: testing/mochitest/runtestsremote.py
@@ +57,5 @@
>          # move necko cache to a location that can be cleaned up
>          options.extraPrefs += ["browser.cache.disk.parent_directory=%s" % self.remoteCache]
>  
> +    def cleanup(self, options, final=False):
> +        if final:

we do if final: else:...;

would we not want the else on this statement so in the final case we get the remote log?
Attachment #8948868 - Flags: review?(jmaher) → review-
Comment on attachment 8948868 [details] [diff] [review]
copy chrome tests only once

That's an interesting point.

The remote log file is deleted at the start of each js harness run, so we cannot retrieve a single log reflecting the entire run in a run-by-manifest world. If we retrieve per manifest (cleanup == False), the local mochitest.log reflects the latest manifest run; if we subsequently try to retrieve another log at the final cleanup, there won't be anything there. If we retrieve only when cleanup == True, we should get the same result, except in some cases of KeyboardInterrupt or similar harness exception.

I find this log rather useless actually. In continuous integration, this code pulls the log to the host, but not to the upload directory, so it is basically lost. Running locally, it usually just pollutes my source directory. The content is in raw format -- hard to read -- and is the same as what has already been dumped to stdout in real time. I am tempted to delete the log retrieval code entirely.

But on the whole, I think I like it the way it is: retrieve the log after each manifest is run.
Attachment #8948868 - Flags: review- → review?(jmaher)
Comment on attachment 8948868 [details] [diff] [review]
copy chrome tests only once

Review of attachment 8948868 [details] [diff] [review]:
-----------------------------------------------------------------

got it
Attachment #8948868 - Flags: review?(jmaher) → review+
Pushed by gbrown@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/edd553a6aa7d
In Android mochitest-chrome, never push tests twice; r=jmaher
https://hg.mozilla.org/mozilla-central/rev/edd553a6aa7d
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: