Bug 194707 – pytest is not correctly auto-installed

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Pablo Saavedra]

```
python ./Tools/Scripts/run-webdriver-tests --json-output=webdriver_tests.json --release --gtk
 in dir /home/buildbot/webkitgtk/gtk-linux-stable4-64/build (timeout 1200 secs)
Using port gtk
Test configuration: <, x86, release>
Using display server xvfb
Using driver at /home/buildbot/webkitgtk/gtk-linux-stable4-64/build/WebKitBuild/Release/bin/WebKitWebDriver
Browser: MiniBrowser
Parsing expectations
Traceback (most recent call last):
  File "./Tools/Scripts/run-webdriver-tests", line 86, in <module>
    runner.run(args)
  File "/home/buildbot/webkitgtk/gtk-linux-stable4-64/build/Tools/Scripts/webkitpy/webdriver_tests/webdriver_test_runner.py", line 70, in run
    runner_tests = [runner.collect_tests(tests) for runner in self._runners]
  File "/home/buildbot/webkitgtk/gtk-linux-stable4-64/build/Tools/Scripts/webkitpy/webdriver_tests/webdriver_test_runner_selenium.py", line 51, in collect_tests
    executor = WebDriverSeleniumExecutor(self._driver, self._env)
  File "/home/buildbot/webkitgtk/gtk-linux-stable4-64/build/Tools/Scripts/webkitpy/webdriver_tests/webdriver_selenium_executor.py", line 57, in __init__
    do_delayed_imports()
  File "/home/buildbot/webkitgtk/gtk-linux-stable4-64/build/Tools/Scripts/webkitpy/webdriver_tests/webdriver_selenium_executor.py", line 35, in do_delayed_imports
    import webkitpy.webdriver_tests.pytest_runner as pytest_runner
  File "/home/buildbot/webkitgtk/gtk-linux-stable4-64/build/Tools/Scripts/webkitpy/webdriver_tests/pytest_runner.py", line 32, in <module>
    import webkitpy.thirdparty.autoinstalled.pytest
ImportError: No module named pytest
```

[Pablo Saavedra]

Created attachment 362119 [details]
patch

[Michael Catanzaro]

Comment on attachment 362119 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=362119&action=review

> Tools/Scripts/webkitpy/thirdparty/__init__.py:162
>          self._install("https://files.pythonhosted.org/packages/a2/ec/415d0cccc1ed41cd7fdf69ad989da16a8d13057996371004cab4bafc48f3/pytest-3.6.2.tar.gz",
>                                "pytest-3.6.2/src/_pytest")
> +        self._install("https://files.pythonhosted.org/packages/a2/ec/415d0cccc1ed41cd7fdf69ad989da16a8d13057996371004cab4bafc48f3/pytest-3.6.2.tar.gz",
> +                              "pytest-3.6.2/src/pytest.py")

I don't understand. Now it's listed twice, and the only difference is the missing underscore. Is this really the right way to do it?

[Pablo Saavedra]

The first one is a dir (_pytest). The line added is a file (pytest.py)

[Pablo Saavedra]

Let me work on it, I guess I could overhead the install() function to allow the definition of more than one path, dir or files.

[Alexey Proskuryakov]

My high level question is - why does it work for me? :)

[Pablo Saavedra]

s/overhead/overload/ :-).


Do you have the pytest.py module in the webkitpy/thirdparty/autoinstalled dir?


If not, do you have the pytest library installed in your system?

[Alexey Proskuryakov]

$ ls webkitpy/thirdparty/autoinstalled | grep pytest
_pytest
pytest.py
pytest_timeout.py

[Pablo Saavedra]

I suppose that the auto-install was able to download this file  at some point 
 but not now. Once you have the pytest.py file in the autoinstalled directory you don't need to download it anymore and this error doesn't happens anymore so I guess you got this py module time ago.

I'd suggest to check the change date of the pytest.py to get more information about your particular case:

  stat Tools/Scripts/webkitpy/thirdparty/autoinstalled/pytest.py 


In any case, this is a weird situation because I didn't find any commit which justify this supposition. The only that I found is this one which is where pytest is added to the auto-install but it doesn't includes the pytest.py neither:

commit b6a225b5f87136ac5dec1380344c5cd082a92473
Author: [e-mail] <[e-mail]@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Sat Dec 2 14:56:26 2017 +0000

    WebDriver: auto-install pytest instead of importing it from wpt tools directory
    https://bugs.webkit.org/show_bug.cgi?id=180243




The Tools/Scripts/webkitpy/thirdparty/autoinstalled/ directory is auto-generated by running the `Tools/Scripts/run-webdriver-tests` command and is safe to delete as it says in the README file included in the this directory.

Therefore, You can reproduce the error following this instructions:


    rm -rf Tools/Scripts/webkitpy/thirdparty/autoinstalled/*
    python Tools/Scripts/run-webdriver-tests --json-output=webdriver_tests.json --release --gtk

***
*** Warning: jhbuild environment not present and not running in flatpak.
*** Run update-webkitgtk-libs or update-webkitgtk-flatpak before build-webkit to ensure proper testing..
***
Using port gtk
Using port gtk
Test configuration: <, x86, release>
Test configuration: <, x86, release>
Using display server xvfb
Using display server xvfb
Using driver at /home/psaavedra/local/workshop/debian-armhf/WebKit/WebKitBuild/Release/bin/WebKitWebDriver
Using driver at /home/psaavedra/local/workshop/debian-armhf/WebKit/WebKitBuild/Release/bin/WebKitWebDriver
Browser: MiniBrowser
Browser: MiniBrowser
Parsing expectations
Parsing expectations
Auto-installing package: py
Auto-installing package: py
Auto-installing package: pluggy
Auto-installing package: pluggy
Auto-installing package: more_itertools
Auto-installing package: more_itertools
Auto-installing package: six.py
Auto-installing package: six.py
Auto-installing package: atomicwrites
Auto-installing package: atomicwrites
Auto-installing package: funcsigs
Auto-installing package: funcsigs
Auto-installing package: attr
Auto-installing package: attr
Auto-installing package: _pytest
Auto-installing package: _pytest
Traceback (most recent call last):
  File "./Tools/Scripts/run-webdriver-tests", line 86, in <module>
    runner.run(args)
  File "/home/psaavedra/local/workshop/debian-armhf/WebKit/Tools/Scripts/webkitpy/webdriver_tests/webdriver_test_runner.py", line 70, in run
    runner_tests = [runner.collect_tests(tests) for runner in self._runners]
  File "/home/psaavedra/local/workshop/debian-armhf/WebKit/Tools/Scripts/webkitpy/webdriver_tests/webdriver_test_runner_selenium.py", line 51, in collect_tests
    executor = WebDriverSeleniumExecutor(self._driver, self._env)
  File "/home/psaavedra/local/workshop/debian-armhf/WebKit/Tools/Scripts/webkitpy/webdriver_tests/webdriver_selenium_executor.py", line 57, in __init__
    do_delayed_imports()
  File "/home/psaavedra/local/workshop/debian-armhf/WebKit/Tools/Scripts/webkitpy/webdriver_tests/webdriver_selenium_executor.py", line 35, in do_delayed_imports
    import webkitpy.webdriver_tests.pytest_runner as pytest_runner
  File "/home/psaavedra/local/workshop/debian-armhf/WebKit/Tools/Scripts/webkitpy/webdriver_tests/pytest_runner.py", line 32, in <module>
    import webkitpy.thirdparty.autoinstalled.pytest
ImportError: No module named pytest



Once you apply my patch:

./Tools/Scripts/run-webdriver-tests --json-output=webdriver_tests.json --release --gtk
***
*** Warning: jhbuild environment not present and not running in flatpak.
*** Run update-webkitgtk-libs or update-webkitgtk-flatpak before build-webkit to ensure proper testing..
***
Using port gtk
Using port gtk
Test configuration: <, x86, release>
Test configuration: <, x86, release>
Using display server xvfb
Using display server xvfb
Using driver at /home/psaavedra/local/workshop/debian-armhf/WebKit/WebKitBuild/Release/bin/WebKitWebDriver
Using driver at /home/psaavedra/local/workshop/debian-armhf/WebKit/WebKitBuild/Release/bin/WebKitWebDriver
Browser: MiniBrowser
Browser: MiniBrowser
Parsing expectations
Parsing expectations
Auto-installing package: pytest.py
Auto-installing package: pytest.py
Auto-installing package: pytest_timeout.py
Auto-installing package: pytest_timeout.py

[Alexey Proskuryakov]

The line with pytest.py was removed in r239465. Jonathan, do you remember why?

[Pablo Saavedra]

(In reply to Alexey Proskuryakov from comment #9)
> The line with pytest.py was removed in r239465. Jonathan, do you remember
> why?

Good catch.

[Carlos Garcia Campos]

(In reply to Alexey Proskuryakov from comment #9)
> The line with pytest.py was removed in r239465. Jonathan, do you remember
> why?

He probably thought it was duplicated by mistake, we need to bring it back.

[Carlos Garcia Campos]

Comment on attachment 362119 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=362119&action=review

cq- for now just in case there was a good reason to remove that. Please, set cq+ if it was removed by mistake.

>> Tools/Scripts/webkitpy/thirdparty/__init__.py:162
>> +                              "pytest-3.6.2/src/pytest.py")
> 
> I don't understand. Now it's listed twice, and the only difference is the missing underscore. Is this really the right way to do it?

This is simple enough, and we are not really downloading twice.

[WebKit Commit Bot]

Comment on attachment 362119 [details]
patch

Clearing flags on attachment: 362119

Committed r241759: <https://trac.webkit.org/changeset/241759>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/48194758>

[Jonathan Bedard]

(In reply to Carlos Garcia Campos from comment #11)
> (In reply to Alexey Proskuryakov from comment #9)
> > The line with pytest.py was removed in r239465. Jonathan, do you remember
> > why?
> 
> He probably thought it was duplicated by mistake, we need to bring it back.

I can confirm that I thought it was duplicated by mistake.

I had to reconstruct all these URLs because the old ones were invalid, it works for existing machines because if the library is already there, none of this install code gets triggered. Interesting, however, that we require this module for web driver tests. Do we run those anywhere in buildbot? We must not, I would have expected infrastructure to catch this weeks ago.

[Michael Catanzaro]

Only GTK port runs WebDriver tests :(

[Jonathan Bedard]

(In reply to Michael Catanzaro from comment #17)
> Only GTK port runs WebDriver tests :(

Ah, that explains it. And we probably haven't deployed new GTK bots recently, hence Pablo being the first one to stumble upon this.

[Pablo Saavedra]

(In reply to Jonathan Bedard from comment #18)
> (In reply to Michael Catanzaro from comment #17)
> > Only GTK port runs WebDriver tests :(
> 
> Ah, that explains it. And we probably haven't deployed new GTK bots
> recently, hence Pablo being the first one to stumble upon this.

Just I idea, not a strong opinion, but maybe could be better clean up the autoinstall dir as part of the steps of the bot to avoid this kind of situations. 

However, it has counterparts in the case of temporary "remote resource unavailable" situations so not sure if it worst.

[Jonathan Bedard]

(In reply to Pablo Saavedra from comment #19)
> (In reply to Jonathan Bedard from comment #18)
> > (In reply to Michael Catanzaro from comment #17)
> > > Only GTK port runs WebDriver tests :(
> > 
> > Ah, that explains it. And we probably haven't deployed new GTK bots
> > recently, hence Pablo being the first one to stumble upon this.
> 
> Just I idea, not a strong opinion, but maybe could be better clean up the
> autoinstall dir as part of the steps of the bot to avoid this kind of
> situations. 
> 
> However, it has counterparts in the case of temporary "remote resource
> unavailable" situations so not sure if it worst.

I've discussed this code with a number of apple folks, really, we should be using virtualenv here. If we were going to make any large changes, it should be in that direction.

[Blaze Burg]

(In reply to Jonathan Bedard from comment #20)
> (In reply to Pablo Saavedra from comment #19)
> > (In reply to Jonathan Bedard from comment #18)
> > > (In reply to Michael Catanzaro from comment #17)
> > > > Only GTK port runs WebDriver tests :(
> > > 
> > > Ah, that explains it. And we probably haven't deployed new GTK bots
> > > recently, hence Pablo being the first one to stumble upon this.
> > 
> > Just I idea, not a strong opinion, but maybe could be better clean up the
> > autoinstall dir as part of the steps of the bot to avoid this kind of
> > situations. 
> > 
> > However, it has counterparts in the case of temporary "remote resource
> > unavailable" situations so not sure if it worst.
> 
> I've discussed this code with a number of apple folks, really, we should be
> using virtualenv here. If we were going to make any large changes, it should
> be in that direction.

+1. This is especially important for WebDriver and other tests where WebKit's version of WPT tests, or the driver functionality itself, can be significantly ahead or behind the client library that's available via PyPI. Virtualenv makes it pretty easy to manage situations like this. It's used at Apple for our internal safaridriver tests.