921676 - mochitest screenshots fallout from bug 746243

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[Jeff Hammel]

Sept 26:
16:57 < philor> did someone touch the creation/logging of screenshots
      on timeouts recently?
16:59 < philor> scroll up from
https://tbpl.mozilla.org/php/getParsedLog.php?id=28435155&full=1&branch=mozilla-inbound#error0
- the screenshot before the timeout is odd, but whatever; the other one above that just ain't right

The code from bug 746243 should be audited to see what has changed
regarding screenshots and the issue fixed.

Comment 1

[Jeff Hammel]

Attachment: referenced TBPL log

Comment 2

[Jeff Hammel]

Attachment: section of interest

Comment 3

[Jeff Hammel]

This appears due to the lack of piping in the mozprocess call:

http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/runtests.py#681

we actually do pipe, but the outputhandler writes to stdout, so we should block that from happening.  So here we should pass a handler to accumulate the output to supress the default of printing.

Comment 4

[Jeff Hammel]

So I'm thinking of

Comment 5

[Jeff Hammel]

(In reply to Jeff Hammel [:jhammel] from comment #4)
> So I'm thinking of

(beh, form submit too soon) ...of changing this bug to move the dumpScreen function to automationutils.py mostly because its easier to test, but also because I need it for bug 915865 .  In this case, I would 

- get rid of the `self` references, which are only used for ensuring dumping is done but once
  - ... and keep this functionality in `runtest.py`

- use subprocess.Popen instead of mozprocess.ProcessHandler.  why? because i'm not 100% sure that mozprocess is available everywhere that automationutils is imported nor does it seem critical for this particular problem.

- add a CLI front-end to test this functionality.  I'd be willing to include this for checkin or not

What do you think, Ted?

Comment 6

[Jeff Hammel]

Attachment: bug-921676

the full on -> automationutils.py change; from comment 5

Comment 7

[Jeff Hammel]

(In reply to Jeff Hammel [:jhammel] from comment #6)
> Created attachment 813345 [details] [diff] [review]
> bug-921676
> 
> the full on -> automationutils.py change; from comment 5

green try: https://tbpl.mozilla.org/?tree=Try&rev=4e1e9f794505

Comment 8

[Jeff Hammel]

Attachment: scr.uri

data uri using mozprocess; does not work

Comment 9

[Jeff Hammel]

Attachment: scr2.uri

data uri using subprocess; works

Comment 10

[Jeff Hammel]

I've been working on the alternate "simple" solution (that is, just putting the pipe in place).  However, there is a problem with using mozprocess to save the screenshots aside from that.  Could be my mistake, but, could be a mozprocess issue. TBD.

Comment 11

[Jeff Hammel]

Attachment: bug-921676-alt

code to get results from comment 8 and comment 9

Comment 12

[Phil Ringnalda (:philor)]

FWIW, every single OS X screenshot from a 330 seconds without output timeout that I've seen today has had no browser window, no sign we're still running, like it has been taken far too late, after we've already killed the browser.

Comment 13

[(not currently active) Ted Mielczarek]

jhammel: Your plan sounds pretty plausible.

philor: that sucks, maybe MozRunner is killing the process before we get around to it?

Comment 14

[Jeff Hammel]

pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=923906412841

Looks green; will land on inbound

Comment 15

[Jeff Hammel]

pushed to inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/ef2d30255b7a

Comment 16

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/ef2d30255b7a