Summary: | pytest is not correctly auto-installed | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Pablo Saavedra <psaavedra> | ||||
Component: | Tools / Tests | Assignee: | Pablo Saavedra <psaavedra> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | ap, bburg, cgarcia, commit-queue, ews-watchlist, glenn, jbedard, lforschler, mcatanzaro, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Local Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Linux | ||||||
Attachments: |
|
Description
Pablo Saavedra
2019-02-15 09:19:00 PST
Created attachment 362119 [details]
patch
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? The first one is a dir (_pytest). The line added is a file (pytest.py) 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. My high level question is - why does it work for me? :) 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? $ ls webkitpy/thirdparty/autoinstalled | grep pytest _pytest pytest.py pytest_timeout.py 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: carlosgc@webkit.org <carlosgc@webkit.org@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 The line with pytest.py was removed in r239465. Jonathan, do you remember why? (In reply to Alexey Proskuryakov from comment #9) > The line with pytest.py was removed in r239465. Jonathan, do you remember > why? Good catch. (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. 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. Comment on attachment 362119 [details] patch Clearing flags on attachment: 362119 Committed r241759: <https://trac.webkit.org/changeset/241759> All reviewed patches have been landed. Closing bug. (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. Only GTK port runs WebDriver tests :( (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. (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. (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. (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. |