Update WAVE test runner #46286

FritzHeiden · 2024-05-15T14:03:16Z

This PR updates the wave test runner used in WMAS and DPCTF projects by CTA. It mainly includes bug fixes as well as change to support python 3.8

foolip · 2024-05-21T13:41:42Z

@FritzHeiden is there someone familiar with this setup who you'd like review from? If not, then as long as the CI passes and the diff looks reasonable, someone like me could rubberstamp and merge it. Right now the CI isn't passing, have you checked that out?

JohnRiv · 2024-06-12T14:23:20Z

@foolip as the WAVE HATF chair, I'll review this PR for the implementation. Agree we need tests to pass as well for this to be able to be merged.

FritzHeiden · 2024-07-10T11:54:08Z

Unfortunately, I don't know how to resolve these errors:

[Errno 2] No such file or directory: '/home/test/web-platform-tests/tools/wave/../wptrunner/requirements_chrome.txt'

This is not part of the tools/wave directory, so I don't know what this is for and how to resolve it. Can someone please assist me with this?

JohnRiv

@FritzHeiden I left some comments inline. I believe the last few may resolve most/all of the failing tests

JohnRiv · 2024-07-10T18:08:09Z

tools/wave/ecmascript/generate-tests.js

+    }
+
+    const jsRelativePath = path.relative(testsPath, currentPath);
+    // console.log(jsRelativePath.replace('.js', ''))


probably best to remove commented-out debug code to clean up the files

JohnRiv · 2024-07-10T18:08:20Z

tools/wave/ecmascript/generate-tests.js

+    const jsSrc = await readFile(currentPath);
+    const meta = parseFrontmatter(jsSrc);
+    const includes = (meta && meta.includes) || [];
+    //console.log(includes);


remove commented-out debug code

JohnRiv · 2024-07-10T18:10:30Z

tools/wave/ecmascript/test-template.html

+<title>{{ TEST_TITLE }}</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<!-- <script charset="UTF-8" src="/ecmascript/harness/jquery-1.4.2.min.js"></script> -->


removed commented-out code (I'm not sure this path to jQuery is valid if it was uncommented?)

JohnRiv · 2024-07-10T18:10:38Z

tools/wave/ecmascript/test-template.iframe.html

+<script charset="UTF-8" src="/ecmascript/harness/cth.js"></script>
+<script charset="UTF-8" src="/ecmascript/harness/propertyHelper.js"></script>
+<script charset="UTF-8" src="/ecmascript/harness/PromiseHelper.js"></script>
+<!-- <script charset="UTF-8" src="/ecmascript/harness/jquery-1.4.2.min.js"></script> -->


removed commented-out code

JohnRiv · 2024-07-10T18:11:38Z

tools/wave/ecmascript/webplatform-adapter.js

+// /ecmascript/tests/built-ins/RegExp/prototype/Symbol.match/builtin-coerce-global.html "Aw Snap"
+// /ecmascript/tests/built-ins/RegExp/prototype/Symbol.match/coerce-global.html "Aw Snap"
+// /ecmascript/tests/built-ins/RegExp/prototype/Symbol.replace/coerce-global.html "Aw Snap"
+// /ecmascript/tests/language/statements/for-of/iterator-next-reference.html "Website unresponsive"


unclear if we need these comments to stay in the code or not

JohnRiv · 2024-07-10T18:20:43Z

tools/wave/requirements.txt

+ua-parser==0.8.0
+python-dateutil==2.8.1


was this change intentional? the previous versions were set by dependabot, so I assume this will just cause dependabot to issue a follow-up PR

JohnRiv · 2024-07-10T18:26:26Z

tools/wave/tox.ini

 [tox]
-envlist = py38,py39,py310,py311,py312
+envlist = py36,py37,py38,py39


prior commits to this file from the past couple years indicate this envlist change should be reverted

JohnRiv · 2024-07-10T18:29:01Z

tools/wave/tox.ini

  -r{toxinidir}/../wptrunner/requirements.txt
-  -r{toxinidir}/../wptrunner/requirements_chromium.txt
+  -r{toxinidir}/../wptrunner/requirements_chrome.txt


I believe this should be reverted, the filename is requirements_chromium.txt

JohnRiv

There are a few other unit test failures besides no-untyped-def, but this should at least address the majority of them

JohnRiv · 2024-10-29T21:11:36Z

tools/wave/configuration_loader.py

@@ -1,7 +1,8 @@
-# mypy: allow-untyped-defs


@FritzHeiden the majority of the unit test failures are because this line was removed from many of the files. Please restore it to all the files it was removed from.

Thanks @JohnRiv we will update the PR ASAP

JohnRiv

@FritzHeiden if you address the inline comments here, I believe that should take care of all the failures we see in tools/ unittests

JohnRiv · 2024-11-25T17:54:38Z

tools/wave/testing/results_manager.py

@@ -20,12 +19,11 @@
 from ..data.session import COMPLETED

 WAVE_SRC_DIR = "./tools/wave"
-RESULTS_FILE_REGEX = r"^\w\w\d\d\d?\.json$"
+RESULTS_FILE_REGEX = "^\w\w\d\d\d?\.json$"


I noticed there is now this warning in the unit tests regarding this line:

SyntaxWarning: invalid escape sequence '\w'

so the removal of the r should probably be reverted

JohnRiv · 2024-11-25T18:19:15Z

tools/wave/testing/sessions_manager.py

-                raise InvalidDataException(f"Unknown type '{test_type}'")
+                raise InvalidDataException("Unknown type '{}'".format(test_type))
+
+        if expiration_date is not None and type(expiration_date) != int:


this has a lint error in the unit tests:

E721 do not compare types, for exact checks use is / is not, for instance checks use isinstance()

so try this as a suggestion:

Suggested change

if expiration_date is not None and type(expiration_date) != int:

if expiration_date is not None and not isinstance(expiration_date, int):

JohnRiv · 2024-11-25T18:22:45Z

tools/wave/testing/sessions_manager.py

+                raise InvalidDataException("Unknown type '{}'".format(test_type))
+
+        if expiration_date is not None and type(expiration_date) != int:
+            expiration_date = iso_to_millis(expiration_date)


this has a lint error in the unit tests:

F821 undefined name 'iso_to_millis'

So you'll need to import that function from tools/wave/utils/deserializer.py in addition to the deserialize_session function you already import from there

JohnRiv · 2024-11-25T18:24:09Z

tools/wave/testing/sessions_manager.py

+
+        if expiration_date is not None and type(expiration_date) != int:
+            expiration_date = iso_to_millis(expiration_date)
+            if type(expiration_date) != int:


another E721 lint error, so likely need to change this to:

Suggested change

if type(expiration_date) != int:

if not isinstance(expiration_date, int):

JohnRiv · 2024-11-25T18:26:42Z

tools/wave/testing/test_loader.py

+        regex_patterns = []
+
+        if test_list is not None and len(test_list) > 0:
+            is_valid = False


Remove this line, as it does nothing per the lint error

F841 local variable 'is_valid' is assigned to but never used

JohnRiv · 2024-11-25T19:49:01Z

tools/wave/wave-cli.py

+
+
+def download_file(url, file_path):
+    response = urllib2.urlopen(url)


Assuming you make the urllib change on line 5 above, this will have to become

Suggested change

response = urllib2.urlopen(url)

response = urllib.request.urlopen(url)

see urllib.request.urlopen docs

JohnRiv · 2024-11-25T19:55:23Z

tools/wave/wave-cli.py

+        os.remove(os.path.join(
+            result_directory, browser["zip"]))
+
+main()


Error: Call to untyped function "main" in typed context [no-untyped-call]

I'm not sure why that error is happening since you have the # mypy: allow-untyped-defs comment at the top, but this file is relatively small so you can probably add typing to the main function definition above on line 33:

def main() -> None:

and then be sure to type the functions that main calls as well (and so on)

JohnRiv · 2024-11-25T20:01:57Z

tools/wave/network/api/api_handler.py

+except ImportError:
+    from urlparse import parse_qsl


Do you need this? It's throwing 2 errors:

error: Cannot find implementation or library stub for module named "urlparse" [import-not-found]
error: Name "parse_qsl" already defined (possibly by an import) [no-redef]

and other parts of the project use urllib.parse and parse_qsl is in python 3.8, so I assume that's fine?

JohnRiv · 2024-11-25T20:34:28Z

tools/wave/network/api/tests_api_handler.py

+try:
+    from urllib.parse import urlunsplit
+except ImportError:
+    from urlparse import urlunsplit


This is also throwing 2 errors:

error: Cannot find implementation or library stub for module named "urlparse" [import-not-found]
error: Name "urlunsplit" already defined (possibly by an import) [no-redef]

likewise, do you need the except?

JohnRiv · 2024-11-25T20:37:56Z

tools/wave/tests/test_wave.py

+try:
+    from urllib.request import urlopen
+    from urllib.error import URLError
+except ImportError:
+    from urllib2 import urlopen, URLError


one more time, errors related to the try/except on imports:

Cannot find implementation or library stub for module named "urllib2" [import-not-found]
Name "urlopen" already defined (possibly by an import) [no-redef]
Name "URLError" already defined (possibly by an import) [no-redef]

FritzHeiden requested a review from a team as a code owner May 15, 2024 14:03

FritzHeiden marked this pull request as draft May 15, 2024 14:05

FritzHeiden force-pushed the master branch 4 times, most recently from d792e57 to 6b08ce8 Compare May 16, 2024 14:47

FritzHeiden marked this pull request as ready for review May 16, 2024 14:48

wpt-pr-bot added infra wave labels May 16, 2024

wpt-pr-bot assigned DanielRyanSmith May 16, 2024

wpt-pr-bot requested review from DanielRyanSmith, foolip and jgraham May 16, 2024 14:49

FritzHeiden force-pushed the master branch from 6b08ce8 to ef87233 Compare May 27, 2024 11:21

JohnRiv suggested changes Jul 10, 2024

View reviewed changes

JohnRiv suggested changes Oct 30, 2024

View reviewed changes

JohnRiv suggested changes Nov 25, 2024

View reviewed changes

FritzHeiden and others added 6 commits January 8, 2025 19:07

feat: update wave test runner to current version

7acd5c4

style: remove comments

3225407

chore: revert changes in requirements.txt and tox.ini

868d9d7

fix: mypy annotations and future imports

46f2c34

fix: wpt CI pipeline errors

9e61101

fix: flake8 errors

18d6206

FritzHeiden force-pushed the master branch from 3112517 to 18d6206 Compare January 8, 2025 18:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update WAVE test runner #46286

Update WAVE test runner #46286

FritzHeiden commented May 15, 2024

foolip commented May 21, 2024

JohnRiv commented Jun 12, 2024

FritzHeiden commented Jul 10, 2024

JohnRiv left a comment

JohnRiv Jul 10, 2024

JohnRiv Jul 10, 2024

JohnRiv Jul 10, 2024

JohnRiv Jul 10, 2024

JohnRiv Jul 10, 2024

JohnRiv Jul 10, 2024

JohnRiv Jul 10, 2024

JohnRiv Jul 10, 2024

JohnRiv left a comment

JohnRiv Oct 29, 2024

louaybassbouss Oct 30, 2024

JohnRiv left a comment

JohnRiv Nov 25, 2024

JohnRiv Nov 25, 2024

JohnRiv Nov 25, 2024

JohnRiv Nov 25, 2024

JohnRiv Nov 25, 2024

JohnRiv Nov 25, 2024

JohnRiv Nov 25, 2024

JohnRiv Nov 25, 2024

JohnRiv Nov 25, 2024

JohnRiv Nov 25, 2024

	if expiration_date is not None and type(expiration_date) != int:
	if expiration_date is not None and not isinstance(expiration_date, int):

	if type(expiration_date) != int:
	if not isinstance(expiration_date, int):



		def download_file(url, file_path):
		response = urllib2.urlopen(url)

	response = urllib2.urlopen(url)
	response = urllib.request.urlopen(url)

		ua-parser==0.8.0
		python-dateutil==2.8.1

Update WAVE test runner #46286

Are you sure you want to change the base?

Update WAVE test runner #46286

Conversation

FritzHeiden commented May 15, 2024

foolip commented May 21, 2024

JohnRiv commented Jun 12, 2024

FritzHeiden commented Jul 10, 2024

JohnRiv 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

JohnRiv left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JohnRiv 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment