1138759 - talos broken when running locally on windows

Status: RESOLVED FIXED

(Testing :: Talos, defect)

Description

[Avi Halachmi (:avih)]

It used to work well in the past.

Today I updated talos after some weeks, and it breaks before the actual test starts (IIRC after the initial window opens and closes).

On: windows 8.1 64, Firefox nightly 32:

$ talos -e /d/run/firefox-nightly/core~nightly-32/firefox.exe -a tsvgx --develop
mozversion INFO | application_buildid: 20150302030204
mozversion INFO | application_changeset: eea6188b9b05
mozversion INFO | application_display_name: Nightly
mozversion INFO | application_name: Firefox
mozversion INFO | application_repository: https://hg.mozilla.org/mozilla-central
mozversion INFO | application_version: 39.0a1
mozversion INFO | platform_buildid: 20150302030204
mozversion INFO | platform_changeset: eea6188b9b05
mozversion INFO | platform_repository: https://hg.mozilla.org/mozilla-central
qm-pxp01:
                Started Mon, 02 Mar 2015 19:14:29
Running test tsvgx:
                Started Mon, 02 Mar 2015 19:14:29
Shumway is registered
__metrics       Screen width/height:1920/1080
        colorDepth:24
        Browser inner width/height: 1010/674
__metrics
JavaScript error: resource:///modules/WebappManager.jsm, line 48: NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIObserverService.removeObserver]
JavaScript error: resource:///modules/CustomizableUI.jsm, line 1571: TypeError: aWindowPalette is undefined
INFO : Shumway is registered
__metrics       Screen width/height:1920/1080
        colorDepth:24
        Browser inner width/height: 1010/674
__metrics
JavaScript error: resource:///modules/WebappManager.jsm, line 48: NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIObserverService.removeObserver]
JavaScript error: resource:///modules/CustomizableUI.jsm, line 1571: TypeError: aWindowPalette is undefined

Traceback (most recent call last):
  File "e:\dev\moz\src\talos\Scripts\talos-script.py", line 8, in <module>
    load_entry_point('talos==0.0', 'console_scripts', 'talos')()
  File "e:\dev\moz\src\talos\talos\run_tests.py", line 310, in main
    sys.exit(run_tests(parser))
  File "e:\dev\moz\src\talos\talos\run_tests.py", line 248, in run_tests
    talos_results.add(mytest.runTest(browser_config, test))
  File "e:\dev\moz\src\talos\talos\ttest.py", line 319, in runTest
    self.initializeProfile(profile_dir, browser_config)
  File "e:\dev\moz\src\talos\talos\ttest.py", line 117, in initializeProfile
    processes = self._ffprocess.checkProcesses(self._pids)
  File "e:\dev\moz\src\talos\talos\ffprocess.py", line 43, in checkProcesses
    pids = [pid for pid in pids if utils.is_running(pid)]
  File "e:\dev\moz\src\talos\talos\utils.py", line 102, in is_running
    return bool([i for i in mozpid.ps() if pid == int(i['PID'])])
ValueError: invalid literal for int() with base 10: 'I'

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: solve non int pids (1.0)

we have a pid = 'I':
    [{'TTY': '10828', 'PGID': '1', 'UID': 'con', 'WINPID': '10828', 'PID': 'I', 'COMMAND': '09:47:59 /usr/bin/bash', 'STIME'
    : '500', 'PPID': '10828'}, {'TTY': '?', 'PGID': '10312', 'UID': '500', 'WINPID': '10312', 'PID': '10312', 'COMMAND': '/u
    sr/bin/ssh-agent', 'STIME': '09:47:59', 'PPID': '1'}, {'TTY': 'con', 'PGID': '24756', 'UID': '500', 'WINPID': '24756', '
    PID': '24756', 'COMMAND': '/usr/bin/bash', 'STIME': '14:02:23', 'PPID': '1'}, {'TTY': 'con', 'PGID': '27232', 'UID': '50
    0', 'WINPID': '27232', 'PID': '27232', 'COMMAND': '/c/talos/Scripts/talos', 'STIME': '14:23:35', 'PPID': '24756'}, {'TTY
    ': 'con', 'PGID': '27260', 'UID': '500', 'WINPID': '27260', 'PID': '27260', 'COMMAND': '/usr/bin/ps', 'STIME': '14:23:45
    ', 'PPID': '1'}] 

this solves that and lets talos work locally on windows.

Comment 2

[Julien Pagès (:parkouss)]

As I said to jmaher on irc, it would be better to write "except ValueError" instead of just "except". This way there is no possibility of catching something we do not want, like a KeyboardInterrupt.

Appart from that, this looks good to me. :)

Comment 3

[Avi Halachmi (:avih)]

FWIW, _if_ the value comes from parsing the output of 'ps', then this is how the output looks on my system when I run it from a bash shell (the same shell I use to run talos - the mozilla-build for windows shell) notice the "I" at PID 2140:

> $ ps
>       PID    PPID    PGID     WINPID  TTY  UID    STIME COMMAND
>      2908       1    2908       2908    ?  500   Mar  2 /bin/ssh-agent
> I    2140       1    2140       2140  con  500   Mar  2 /bin/bash
>      6904       1    6904       6904  con  500   Mar  2 /bin/bash
>      4892    6904    4892        128  con  500 17:25:45 /bin/ps

At which case, pid == "I" might not not necessarily mean "firefox is not running anymore", but that's how the patch interprets it.

But overall, I think this is a bug at the stuff which parses the output of 'ps'

Comment 4

[William Lachance (:wlach)]

Comment on attachment 8572615 [details] [diff] [review]
solve non int pids (1.0)

This looks like an upstream bug in mozprocess, specifically it looks like this function isn't processing the output of 'ps' correctly:

http://hg.mozilla.org/mozilla-central/file/a27dd304348d/testing/mozbase/mozprocess/mozprocess/pid.py#l19

I can reproduce the wrong behaviour with this test program

input = """PID    PPID    PGID     WINPID  TTY  UID    STIME COMMAND
     2908       1    2908       2908    ?  500   Mar  2 /bin/ssh-agent
I    2140       1    2140       2140  con  500   Mar  2 /bin/bash
     6904       1    6904       6904  con  500   Mar  2 /bin/bash
     4892    6904    4892        128  con  500 17:25:45 /bin/ps"""
ret = []
header = None
for line in input.splitlines():
    line = line.strip()
    if header is None:
        # first line is the header
        header = line.split()
        continue
    split = line.split(None, len(header)-1)
    print split
    process_dict = dict(zip(header, split))
    ret.append(process_dict)
print ret

The problem is that we're not expecting the "I" when splitting the output. Honestly we should just be using `psutil` (https://code.google.com/p/psutil/) for this stuff and just kill mozprocess.pid. Short term, let's write a bit of win32 code to handle the windows case and use mozprocess.pid otherwise.

Comment 5

[William Lachance (:wlach)]

Attachment: Patch to do what I suggest

I think this should work for more cases where "ps" is giving busted output on windows.

Comment 6

[William Lachance (:wlach)]

I think we should just kill mozprocess.pid. Filed bug 1139487 for that.

Comment 7

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8572695 [details] [diff] [review]
Patch to do what I suggest

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

thanks Will!  Can you land this on the talos repo?

Comment 8

[William Lachance (:wlach)]

Pushed: https://hg.mozilla.org/build/talos/rev/96095438bf72

Comment 9

[Avi Halachmi (:avih)]

Confirmed fixed. Thanks.