Closed Bug 1290765 Opened 4 years ago Closed 3 years ago

Mochitest on Windows takes ages to start on Windows

Categories

(Testing :: Mochitest, defect, P1)

Version 3
x86_64
All
defect

Tracking

(firefox52 fixed)

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: cpearce, Assigned: ahal)

References

Details

Attachments

(4 files)

If I run `./mach mochitest dom/media/test_eme*` on Windows it takes ages for mochitest to start running.

I get the following output:

$ ./mach mochitest dom/media/test/test_eme*

######
### Now running mochitest-plain with subsuite media.
######

Checking for orphan ssltunnel processes...
Checking for orphan xpcshell processes...
SUITE-START | Running 13 tests
dir: dom/media/test
mozprofile.addons WARNING | Could not install c:\Users\cpearce\src\mozilla\inbound\objdir\_tests\testing\mochitest\extensions\mozscreenshots: [Errno 2] No such file or directory: '
c:\\Users\\cpearce\\src\\mozilla\\inbound\\objdir\\_tests\\testing\\mochitest\\extensions\\mozscreenshots\\install.rdf'
c:\Users\cpearce\src\mozilla\inbound\objdir\dist\bin\pk12util.exe: PKCS12 IMPORT SUCCESSFUL
MochitestServer : launching [u'c:\\Users\\cpearce\\src\\mozilla\\inbound\\objdir\\dist\\bin\\xpcshell.exe', '-g', u'c:\\Users\\cpearce\\src\\mozilla\\inbound\\objdir\\dist\\bin', '
-v', '170', '-f', u'c:\\Users\\cpearce\\src\\mozilla\\inbound\\objdir\\dist\\bin\\components\\httpd.js', '-e', "const _PROFILE_PATH = 'c:\\\\users\\\\cpearce\\\\appdata\\\\local\\\
\temp\\\\tmppeuvxn.mozrunner'; const _SERVER_PORT = '8888'; const _SERVER_ADDR = '127.0.0.1'; const _TEST_PREFIX = undefined; const _DISPLAY_RESULTS = false;", '-f', 'c:\\Users\\cp
earce\\src\\mozilla\\inbound\\objdir\\_tests\\testing\\mochitest\\server.js']
runtests.py | Server pid: 18112
runtests.py | Websocket server pid: 18568
runtests.py | websocket/process bridge pid: 18144
Traceback (most recent call last):
  File "websocketprocessbridge\websocketprocessbridge.py", line 6, in <module>
    from twisted.internet import protocol, reactor
ImportError: No module named twisted.internet



Then I get a pause for about 3 minutes, and then finally the tests start.

Mochitest on Linux runs fast (as expected).

Can we get this fixed ASAP please? It makes working on Windows painful.
Jonathan - are you the person I should be asking for help from here?
Flags: needinfo?(jgriffin)
Ahal, Gps, any idea what's going on here?
Flags: needinfo?(jgriffin)
Flags: needinfo?(gps)
Flags: needinfo?(ahalberstadt)
I didn't even realize mochitests used Twisted. Although it looks like that may be limited to the websocket server?

My guess is the bug is the mochitest command isn't installing the Python dependencies required by the websocketprocessbridge package.

As a workaround, try running:

  <objdir>/_virtualenv/Scripts/pip.exe install -r testing/tools/websocketprocessbridg/websocketprocessbridge_requirements.txt
Flags: needinfo?(gps)
Attached file log.txt
The websocketprocessbridge_requirements.txt deps didn't fix it, I had to install "Microsoft Visual C++ Compiler for Python 2.7" and then even then it failed and I ended up having to pip install twisted, txws, six, and psutil. It worked as before once I'd done that however.

I've attached the log of what I did.
Yeah, I think twisted is only used by the dom/media tests, gps is probably right. I can reproduce this on linux as well, so it doesn't seem to be a windows problem. It only happens for the 'media' subsuite. I'll leave the needinfo until I have time to dig into it some more.
OS: Windows 10 → All
Chris would you mind verifying this patch gets mochitest working again on Windows?
Flags: needinfo?(ahalberstadt) → needinfo?(cpearce)
Sorry for the delay getting to this. My machine at the office is fixed, so I had to test this on my Windows machine at home.

Here are the results of running on that machine.

The patch installed some of the dependencies, but failed because the "Microsoft Visual C++ Compiler for Python 2.7" wasn't included.
Flags: needinfo?(cpearce)
Attachment #8780241 - Flags: feedback?(ahalberstadt)
Comment on attachment 8780241 [details]
Results of attachment  8777356 [details]

Gps probably knows what that's about better than me. But it's likely a separate issue from this, possibly something needs to get added to |mach bootstrap| if it isn't already.

The current patch at least fixes the issue for me on Linux, so is at least a step in the right direction.
Flags: needinfo?(gps)
Attachment #8780241 - Flags: feedback?(ahalberstadt)
(In reply to Chris Pearce (:cpearce) from comment #10)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=77c061845ec4

Sorry, I did a try push, and this bug's patch was the top-most commit in my active head. Please ignore this push.
Comment on attachment 8777356 [details]
Bug 1290765 - Add ability to install requirements.txt files in mozbuild/virtualenv.py,

https://reviewboard.mozilla.org/r/68926/#review69276

This is more or less how I'd do it.

The error about MSVC is because Python 2.7 on Windows was built with Visual Studio 2008 and Python packaging insists that Python C extensions are also built with VS2008. We have a hack in our virtualenv.py that attempts to disable some of these checks. Search for `VSNNCOMNTOOLS`. That hack may need extracted to its own function and the `pip` invocations may need to be wrapped around it to unblock compilation.

That being said, this issue should only be present on Windows. If we're downloading 3rd party packages with C extensions, I would expect binary wheels to be available. If that's not true, it is either a packaging failure from the package provider. Or we need to upload the wheel to our PyPI mirror used by automation.

::: python/mozbuild/mozbuild/virtualenv.py:498
(Diff revision 1)
> +        if not os.path.isabs(path):
> +            path = os.path.join(self.topsrcdir, path)
> +
> +        args = [
> +            'install',
> +            '--requirement',

The version of pip we have vendored supports hash verification. I'd **really** like the default pip invocation to have `--require-hashes` because it will protect us against MiTM attacks and unwanted upstream package changes.

This does mean that requirements files must ping all package versions and their dependencies and have hashes listed. In some cases, we'll need to list multiple hashes since e.g. Windows and Linux could download different packages.
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
Flags: needinfo?(gps)
I added --require-hashes, but it looks like automation is still using pip 1.5 doesn't like the --hash syntax in the requirements.txt very much:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=83291f2ef9ea4bc8542a4e95e0ca7408c0840532
Depends on: 1297515
Are you ok landing this without --require-hashes for the time being until bug 1297515 gets fixed? This can be a big pain for developers and I don't think that other bug will be resolved any time soon. I would file a follow-up to add the hashes back in when CI can support it.
Flags: needinfo?(gps)
Blocks: 1315080
I'm fine forgoing --require-hashes at this time - there are plenty of parts of automation doing the wrong thing already. That's not a good excuse. But if this is annoying developers, that concern is greater.

FWIW, when you have source checkouts, mozharness will use pip from the checkout automatically. We /could/ have these tests create a source checkout. But until the EBS perf issues are fixed, we're holding off on source checkouts on testers for perf reasons.
Flags: needinfo?(gps)
Comment on attachment 8777356 [details]
Bug 1290765 - Add ability to install requirements.txt files in mozbuild/virtualenv.py,

https://reviewboard.mozilla.org/r/68926/#review69276

> The version of pip we have vendored supports hash verification. I'd **really** like the default pip invocation to have `--require-hashes` because it will protect us against MiTM attacks and unwanted upstream package changes.
> 
> This does mean that requirements files must ping all package versions and their dependencies and have hashes listed. In some cases, we'll need to list multiple hashes since e.g. Windows and Linux could download different packages.

As per discussion, not making this a blocker. Filed bug 1315693 as a follow-up.
Joel, I'm needinfo'ing you because you filed bug 1315080.

Do these patches get that test working for you? Following your STR, the ImportError is fixed but then the harness still seems to hang. However you said tests were successfully running after the ImportError was fixed, so I suspect there is something else wrong with my environment. Would be good to verify this patch gets things working for someone though.
Flags: needinfo?(jmaher)
I did uninstall twisted |pip uninstall twisted|, then verified I got the same error about the missing package twisted, applied the changes, and it automatically installed twisted and ran the tests as expected.
Flags: needinfo?(jmaher)
Thanks, I realized my mochitest runs were timing out because there was something busted with my build. It's unrelated to this patch or issue.
Comment on attachment 8777356 [details]
Bug 1290765 - Add ability to install requirements.txt files in mozbuild/virtualenv.py,

https://reviewboard.mozilla.org/r/68926/#review92174
Attachment #8777356 - Flags: review?(gps) → review+
Comment on attachment 8808205 [details]
Bug 1290765 - Install websocketprocessbridge requirements when running media mochitests with mach,

https://reviewboard.mozilla.org/r/91064/#review92176

::: testing/mochitest/mach_commands.py:401
(Diff revision 1)
> +            # sys.executable is used to start the websocketprocessbridge, though for some
> +            # reason it doesn't get set when calling `activate_this.py` in the virtualenv.
> +            sys.executable = self.virtualenv_manager.python_path

It feels like sys.executable should be set **after** doing virtualenv install.

Also, I too wonder why we don't set `sys.executable`. Probably because we'd be lying since the real executable was not in the virtualenv! But, lying is arguably the correct thing to do, as we activate the virtualenv from within the process as opposed to from outside. So this seems like a fine hack. My only question is whether we should set it in `activate()`. But that's for another bug.
Attachment #8808205 - Flags: review?(gps) → review+
Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6a47316764f0
Add ability to install requirements.txt files in mozbuild/virtualenv.py, r=gps
https://hg.mozilla.org/integration/autoland/rev/626134b9080e
Install websocketprocessbridge requirements when running media mochitests with mach, r=gps
https://hg.mozilla.org/mozilla-central/rev/6a47316764f0
https://hg.mozilla.org/mozilla-central/rev/626134b9080e
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.