Add a wptrun frontend to wptrunner. #5893

jgraham · 2017-05-11T16:02:27Z

This provides a convenient way for users to run tests in a browser
without having to grasp the many command line switches of wptrunner,
or install dependencies by hand. The syntax is basically

wptrun browser [tests]

e.g.

wptrun firefox dom/historical.html

Currently this is implemented for Firefox and Chrome, and will not
download the actual browser binary for you. For Firefox downloading
certutil is also a problem because there aren't convenient binary
releases. However we can still download the appropriate webdriver
implementations, firefox prefs, create a virtualenv, install required
wptrunner dependencies for the browser and generally make the process
of running tests substantially easier.

The keen-eyed with notice that this stole a lot of code from the
stability checker. The intent in the long term is cearly to re-merge
these codepaths so that we have a single implementation here.

This change is

w3c-bots · 2017-05-11T16:10:25Z

View the complete job log.

Firefox (nightly)

Testing web-platform-tests at revision aa06e88
Using browser at version BuildID 20170512100218; SourceStamp 8a7d0b15595f9916123848ca906f29c62d4914c9
Starting 10 test iterations
No tests run.

w3c-bots · 2017-05-11T16:10:26Z

View the complete job log.

Chrome (unstable)

Testing web-platform-tests at revision aa06e88
Using browser at version 60.0.3095.5 dev
Starting 10 test iterations
No tests run.

jugglinmike

I'm happy to see this developing; it's going to help a lot of people. I've tested it locally, and it seems to work as intended. Could we document its presence in the README.md file and including installation instructions?

jugglinmike · 2017-05-11T18:22:04Z

tools/browserutils/browser.py

+        }.get(uname[0])
+
+        if platform is None:
+            raise ValueError("Unable to construct a valid Firefox package name for current platform")


Typo: "Firefox package" -> "GeckoDriver package"

jugglinmike · 2017-05-11T18:39:15Z

wptrun.py

@@ -0,0 +1,260 @@
+import argparse


Think this file can go in tools/? The top-level is already plenty cluttered, but I'd still like to avoid adding more when possible.

jugglinmike · 2017-05-11T18:56:35Z

wptrun.py

+        return True
+    while True:
+        resp = raw_input("Download and install %s [Y/n]? " % component).strip().lower()
+        if not resp or resp == "y":


I'd rather interpret "empty response" as invalid, not "yes".

jugglinmike · 2017-05-11T18:59:33Z

wptrun.py

+            hosts_path = "/etc/hosts"
+        else:
+            hosts_path = "C:\Windows\System32\drivers\etc\hosts"
+        with open(hosts_path) as f:


In case you missed it, the lint script is complaining about this line:

wptrun.py:79: File opened without providing an explicit mode (note: binary files must be read with 'b' in the mode flags) (OPEN-NO-MODE)

jugglinmike · 2017-05-11T19:13:28Z

tools/browserutils/browser.py

+        return "%s%s" % (platform, bits)
+
+    def install(self):
+        return None


I know we don't invoke this method today, but it seems like for the time being, we'd want to inherit the base class's implementation.

jugglinmike · 2017-05-11T19:14:16Z

tools/browserutils/browser.py

+        url = "http://chromedriver.storage.googleapis.com/%s/chromedriver_%s.zip" % (latest,
+                                                                                     self.platform_string())
+        unzip(get(url).raw, dest)
+        path = os.path.join(dest, 'chromedriver')


Typo: single quotes

jugglinmike · 2017-05-11T19:14:27Z

tools/browserutils/virtualenv.py

+        return path
+
+    def activate(self):
+        path = os.path.join(self.bin_path, 'activate_this.py')


Typo: single quotes

gsnedders · 2017-05-11T19:39:27Z

.gitignore

@@ -14,3 +14,4 @@ webdriver/.idea
 .vscode/
 .DS_Store
 *.rej
+_venv


Missing new line at the EOF

jgraham · 2017-05-12T10:55:09Z

Review status: 0 of 10 files reviewed at latest revision, 8 unresolved discussions, some commit checks failed.

wptrun.py, line 95 at r1 (raw file):

Previously, jugglinmike wrote…

I'd rather interpret "empty response" as invalid, not "yes".

I quite strongly disagree. There's a long history of tools using empty response to mean "do whatever the sensible default is" e.g. apt.

tools/browserutils/browser.py, line 227 at r1 (raw file):

Previously, jugglinmike wrote…

I know we don't invoke this method today, but it seems like for the time being, we'd want to inherit the base class's implementation.

That doesn't work because the base class implementation is marked as abstract.

Comments from Reviewable

bobholt · 2017-05-12T11:29:44Z

Reviewed 5 of 11 files at r1, 6 of 6 files at r2.
Review status: all files reviewed at latest revision, 8 unresolved discussions, some commit checks failed.

tools/wptrun.py, line 95 at r1 (raw file):

prompt_install
I see the appeal of forcing the user to make a choice, and if this were a new UI we creating, I'd want that to be the case. But since this is a command-line prompt, and it ends with [Y/n]?, in apt style, I'm fine with it as-is.

Comments from Reviewable

bobholt · 2017-05-12T11:31:54Z

Review status: all files reviewed at latest revision, 8 unresolved discussions, some commit checks failed.

Comments from Reviewable

This provides a convenient way for users to run tests in a browser without having to grasp the many command line switches of wptrunner, or install dependencies by hand. The syntax is basically wptrun browser [tests] e.g. wptrun firefox dom/historical.html Currently this is implemented for Firefox and Chrome, and will not download the actual browser binary for you. For Firefox downloading certutil is also a problem because there aren't convenient binary releases. However we can still download the appropriate webdriver implementations, firefox prefs, create a virtualenv, install required wptrunner dependencies for the browser and generally make the process of running tests substantially easier. The keen-eyed with notice that this stole a lot of code from the stability checker. The intent in the long term is cearly to re-merge these codepaths so that we have a single implementation here.

bobholt · 2017-05-12T17:02:48Z

Reviewed 1 of 6 files at r2, 3 of 3 files at r3.
Review status: all files reviewed at latest revision, 10 unresolved discussions, all commit checks successful.

README.md, line 77 at r3 (raw file):

Tests can be run automatically in a browser using the `wptrun` script
in the root of the checkout. This requires the hosts file and OpenSSL

This is no longer in the root of the checkout, but in the tools directory, correct?

README.md, line 93 at r3 (raw file):

with `wptrun --help` and `wptrun --wptrunner-help`.

Not all dependnecies can be automatically installed; in paticular the

TYPOs: "dependencies", "particular"

Comments from Reviewable

emanlove · 2017-05-21T12:18:07Z

tools/browserutils/browser.py

+import re
+import stat
+from abc import ABCMeta, abstractmethod
+from configparser import RawConfigParser


configparser is 3.x while ConfigParser is 2.x noting that the web-platform-tests documentation states that "test environment requires Python 2.7+".

emanlove · 2017-05-21T13:03:32Z

tools/browserutils/virtualenv.py

+    def create(self):
+        if os.path.exists(self.path):
+            shutil.rmtree(self.path)
+        call(self.virtualenv, "--no-download", self.path)


The --no-download option was removed in pip version 7.0.0.

This isn't called from anywhere. The DBUS_SESSION_BUS_ADDRESS bits were added in #5893 but was unused even then. Since then it's been copied around a lot.

…rowser.py, a=testonly Automatic update from web-platform-testsRemove dead code prepare_environment in browser.py (#9653) This isn't called from anywhere. The DBUS_SESSION_BUS_ADDRESS bits were added in web-platform-tests/wpt#5893 but was unused even then. Since then it's been copied around a lot. wpt-commits: c58e51bef6e114539375fa89e094b20c2056ebe9 wpt-pr: 9653 wpt-commits: c58e51bef6e114539375fa89e094b20c2056ebe9 wpt-pr: 9653

…rowser.py, a=testonly Automatic update from web-platform-testsRemove dead code prepare_environment in browser.py (#9653) This isn't called from anywhere. The DBUS_SESSION_BUS_ADDRESS bits were added in web-platform-tests/wpt#5893 but was unused even then. Since then it's been copied around a lot. wpt-commits: c58e51bef6e114539375fa89e094b20c2056ebe9 wpt-pr: 9653 wpt-commits: c58e51bef6e114539375fa89e094b20c2056ebe9 wpt-pr: 9653 UltraBlame original commit: 15aacce348c225e44a048048176449e24868b09e

jgraham requested review from bobholt, gsnedders and jugglinmike May 11, 2017 16:02

wpt-pr-bot added infra wptrun labels May 11, 2017

jgraham force-pushed the wptrun branch from 27f5141 to 1ddd05c Compare May 11, 2017 17:04

jugglinmike reviewed May 11, 2017

View reviewed changes

gsnedders reviewed May 11, 2017

View reviewed changes

.gitignore

@@ -14,3 +14,4 @@ webdriver/.idea

.vscode/

.DS_Store

*.rej

_venv

Copy link

Member

gsnedders May 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing new line at the EOF

jgraham force-pushed the wptrun branch from 1ddd05c to 899d6c3 Compare May 12, 2017 10:55

jgraham force-pushed the wptrun branch from 899d6c3 to 5c80d16 Compare May 12, 2017 15:41

jgraham force-pushed the wptrun branch from 5c80d16 to 4e76745 Compare May 12, 2017 15:51

fixup! Typos

5d63149

jgraham merged commit 32aa301 into master May 12, 2017

emanlove reviewed May 21, 2017

View reviewed changes

plehegar deleted the wptrun branch September 25, 2017 20:43

foolip added a commit that referenced this pull request Feb 23, 2018

Remove dead code prepare_environment in browser.py

e4b3335

This isn't called from anywhere. The DBUS_SESSION_BUS_ADDRESS bits were added in #5893 but was unused even then. Since then it's been copied around a lot.

foolip mentioned this pull request Feb 23, 2018

Remove dead code prepare_environment in browser.py #9653

Merged

foolip added a commit that referenced this pull request Feb 25, 2018

Remove dead code prepare_environment in browser.py

8e296d0

This isn't called from anywhere. The DBUS_SESSION_BUS_ADDRESS bits were added in #5893 but was unused even then. Since then it's been copied around a lot.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a wptrun frontend to wptrunner. #5893

Add a wptrun frontend to wptrunner. #5893

jgraham commented May 11, 2017 •

edited

w3c-bots commented May 11, 2017 •

edited

w3c-bots commented May 11, 2017 •

edited

jugglinmike left a comment

jugglinmike May 11, 2017

jugglinmike May 11, 2017

jugglinmike May 11, 2017

jugglinmike May 11, 2017

jugglinmike May 11, 2017

jugglinmike May 11, 2017

jugglinmike May 11, 2017

gsnedders May 11, 2017

jgraham commented May 12, 2017

bobholt commented May 12, 2017

bobholt commented May 12, 2017

bobholt commented May 12, 2017

emanlove May 21, 2017

emanlove May 21, 2017

Add a wptrun frontend to wptrunner. #5893

Add a wptrun frontend to wptrunner. #5893

Conversation

jgraham commented May 11, 2017 • edited

w3c-bots commented May 11, 2017 • edited

Firefox (nightly)

w3c-bots commented May 11, 2017 • edited

Chrome (unstable)

jugglinmike left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jgraham commented May 12, 2017

bobholt commented May 12, 2017

bobholt commented May 12, 2017

bobholt commented May 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jgraham commented May 11, 2017 •

edited

w3c-bots commented May 11, 2017 •

edited

w3c-bots commented May 11, 2017 •

edited