1604221 - "./mach test" is broken on Windows for Python unit tests

Status: RESOLVED INVALID

(Firefox Build System :: Mach Core, task)

Description

[Ricky Stewart]

./mach python-test works, ./mach test doesn't, failing with an error like this one: https://paste.mozilla.org/WK3LzMb4/raw

Comment 1

[Michael Shal [:mshal]]

I looked into this a bit - the problem is the python value in https://searchfox.org/mozilla-central/rev/2f09184ec781a2667feec87499d4b81b32b6c48e/python/mach_commands.py#246 is used in the virtualenv path, but it has different values depending on whether test or python-test was used. In python-test, python is "2.7", so in the virtualenv path this becomes something like "c:\mozilla-source\mozilla-central\obj-x86_64-pc-mingw32\_virtualenvs\mozilla-central-GFsopzcj-2.7..." (note the "-2.7" appended to the path). In the regular test command, python is None, so it picks up the python_path value of c:...\python.exe. Appending this value to the virtualenv makes the nonsense path of "c:\mozilla-source\mozilla-central\obj-x86_64-pc-mingw32\_virtualenvs\mozilla-central-GFsopzcj-c:\mozilla-source\mozilla-central\obj-x86_64-pc-mingw32\_virtualenvs\init\Scripts\python.exe". (Note the "-c:\mozilla-source..." in the middle of the path).

Windows complains about this because of the drive name in the middle of the path, while Linux is happy to pretend that a path like mozilla-central-blah-/home/user/mozilla-central/objdir/_virtualenvs/init/Scripts/python.exe is a real path.

So that's why it's busted, and presumably we need to use a string like "2.7" in the virtualenv path name rather than the path to python.exe, though I don't know the best way to grab that when python is None. I double-checked m-c versions from a while ago (up to a year ago, I think?) and they all are similarly broken, so this is not a new regression. mars, do you have some time to take a look? With your recent virtualenv work you have more familiarity with this.

Comment 2

[Māris Fogels [:mars] (please needinfo)]

Thanks for the investigation. Getting the change right is going to take a bit of investigation because the python argument can be assigned a version number or a full interpreter path or None, and which datatype it gets assigned depends on which CI test suite called it. I wasn't aware python-test is called by command wrappers, too.

The resolution for this issue should consider that the python argument is there to support pipenv, and ahal is working on a patch to kill pipenv in https://phabricator.services.mozilla.com/D55869 (bug 1601445).

I think landing ahal's pipenv-killing patch is the best approach. It looks like his patch properly handles the different datatypes the python argument can take and landing his patch would fix this bug.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 4

[Alex Hochheiden [:ahochheiden] [he/him]]

./mach test works on Windows as of this writing.