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)
Tracking
(firefox60 fixed)
RESOLVED
FIXED
mozilla60
Tracking | Status | |
---|---|---|
firefox60 | --- | fixed |
People
(Reporter: gbrown, Assigned: gbrown)
References
Details
Attachments
(1 file)
4.38 KB,
patch
|
jmaher
:
review+
|
Details | Diff | Splinter Review |
There is a significant opportunity for optimization here. Without the optimization, most android mochitest-chrome tasks timeout with runByManifest.
Assignee | ||
Comment 1•6 years ago
|
||
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 2•6 years ago
|
||
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-
Assignee | ||
Comment 3•6 years ago
|
||
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 4•6 years ago
|
||
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
Comment 6•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/edd553a6aa7d
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox60:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in
before you can comment on or make changes to this bug.
Description
•