Experimental new test system #1047

dalcde · 2021-01-05T14:03:00Z

This introduces a new way of writing tests via the decorator @pytest.run_code. This runs the entire decorated function via selenium.run, and passes on any error.

Nothing in here is intended to be final. For example, there ought to be a better way to expose run_code to the tests. Comments welcome.

This also produces more legible errors, e.g. if I change the code to simply assert False, I get

__________________________________________________ test_bz2[firefox] __________________________________________________
Message: Error: Traceback (most recent call last):
  File "/lib/python3.8/site-packages/pyodide/_base.py", line 105, in eval_code
    eval(compile(mod, "<exec>", mode="exec"), ns, ns)
  File "<exec>", line 5, in <module>
  File "<exec>", line 3, in test_bz2
AssertionError

------------------------------------------------ Captured stdout setup ------------------------------------------------
Spawning webserver at http://127.0.0.1:49829 (see logs in /tmp/tmp26ej3u19/http-server.log)
---------------------------------------------- Captured stdout teardown -----------------------------------------------
Python exception:
Traceback (most recent call last):
  File "/lib/python3.8/site-packages/pyodide/_base.py", line 105, in eval_code
    eval(compile(mod, "<exec>", mode="exec"), ns, ns)
  File "<exec>", line 5, in <module>
  File "<exec>", line 3, in test_bz2
AssertionError

_hiwire_throw_error@http://127.0.0.1:49829/pyodide.asm.js:8:178853
@http://127.0.0.1:49829/pyodide.asm.wasm:wasm-function[378]:0x11316d
@http://127.0.0.1:49829/pyodide.asm.wasm:wasm-function[376]:0x112e70
pyodide/Module.__pyproxy_apply@http://127.0.0.1:49829/pyodide.asm.js:8:2397731
apply@http://127.0.0.1:49829/pyodide.asm.js:8:182091
languagePluginLoader</Module.runPython@http://127.0.0.1:49829/pyodide.js:333:48
@http://127.0.0.1:49829/test.html:4:34
@http://127.0.0.1:49829/test.html:6:8

dalcde · 2021-01-05T14:04:12Z

mypy is failing due to the horrible hack to expose run_code. An even worse option would be to add it to builtins, but that still doesn't make mypy happy

dalcde · 2021-01-05T14:47:47Z

New error log:

__________________________________________________ test_bz2[firefox] __________________________________________________
Error running function in pydodie

Traceback (most recent call last):
  File "/src/src/tests/test_bz2.py", line 3, in test_bz2
AssertionError
------------------------------------------------ Captured stdout setup ------------------------------------------------
Spawning webserver at http://127.0.0.1:59799 (see logs in /tmp/tmpfjec_mjy/http-server.log)
---------------------------------------------- Captured stdout teardown -----------------------------------------------
Python exception:
Traceback (most recent call last):
  File "/lib/python3.8/site-packages/pyodide/_base.py", line 105, in eval_code
    eval(compile(mod, "<exec>", mode="exec"), ns, ns)
  File "<exec>", line 12, in <module>
  File "<exec>", line 3, in test_bz2
AssertionError

_hiwire_throw_error@http://127.0.0.1:59799/pyodide.asm.js:8:178853
@http://127.0.0.1:59799/pyodide.asm.wasm:wasm-function[378]:0x11316d
@http://127.0.0.1:59799/pyodide.asm.wasm:wasm-function[376]:0x112e70
pyodide/Module.__pyproxy_apply@http://127.0.0.1:59799/pyodide.asm.js:8:2397731
apply@http://127.0.0.1:59799/pyodide.asm.js:8:182091
languagePluginLoader</Module.runPython@http://127.0.0.1:59799/pyodide.js:333:48
@http://127.0.0.1:59799/test.html:4:34
@http://127.0.0.1:59799/test.html:6:8

The line number and file name are now accurate

dalcde · 2021-01-05T15:58:29Z

This partially addresses #1031

hoodmane · 2021-01-06T01:29:47Z

Thanks Dexter! This looks fantastic!
If tests are written like this, it also would fix a couple of other issues: bad editor syntax highlighting for the important part of the code (the Python part), and using lots of short calls to selenium.run and selenium.run_jsmakes it hard to run the test onconsole.html`. This way I should be able to open the browser terminal, type

pyodide.runPython(`
<ctrl+v>
`)

and run the test without having to completely rearrange it.

Maybe we should add a pyodide_py.run_js API for the javascript bits. Or we could use my @jsfunc decorator.

rth

It would certainly be great to be able to do this!

conftest.py

rth · 2021-01-06T23:03:43Z

conftest.py

@@ -455,6 +458,57 @@ def service_actions():
        httpd.serve_forever()


+def run_in_pyodide(*args):
+    # This can be called as @run_in_pyodide or @run_in_pyodide("first_dep",
+    # "second_dep"). In the former case, run_in_pyodide acts as the decorator.


If it can only be called with the second variant and it allow to simplify that's also fine.

conftest.py

dalcde · 2021-01-09T02:02:29Z

I have squashed and force-pushed because I rewrote a huge chunk of it and moved it to a different file, so the diffs won't be very useful either way.

This introduces a new way of writing tests via the decorator @run_in_pyodide. This runs the entire decorated function via selenium.run, and passes on any error.

rth

Thanks mostly looks good. Minor comments below.

Also,

I'm fairly surprised that using a decorator defining an internal function with a pytest fixture works. Could we find some reference on whether it's expected to work, or if this is relying on some internal implementation detail of pytest which can be broken at any time.
just curious would adding @pytest.mark.parametrize, before or after, work (or could be made to work -- though not in in this PR)?

rth · 2021-01-10T11:22:09Z

packages/jedi/test_jedi.py

-        """
-    )
-    assert result == ["load", "loads"]
+from tools.testing import run_in_pyodide


Tools is not really a place for python modules. I would rather we put it in pyodide_build. I mean it's called build, but in practice it's anything that shouldn't be included in WASM VM (build, testing, etc). So can also rename it to be more general.

Among other things mypy and pytest are not currently run on the tools folder.

rth · 2021-01-10T11:23:44Z

tools/testing.py

+    catches the function the decorator is applied to. Otherewise, standalone
+    and packages are the actual arguments to the decorator.
+
+    See docs/testing.md for details on how to use this


Please document parameters in the docstring still and ideally add type annotations.

dalcde · 2021-01-10T11:56:49Z

I would expect fixtures to work by querying the parameter names, e.g. via inspect. This should work with dynamically generated functions.

rth

Thanks!

docs/testing.md

dalcde marked this pull request as draft January 5, 2021 14:11

rth reviewed Jan 6, 2021

View reviewed changes

dalcde force-pushed the pytest branch from a614630 to f22e4d6 Compare January 9, 2021 02:01

New test framework

ed47a2b

This introduces a new way of writing tests via the decorator @run_in_pyodide. This runs the entire decorated function via selenium.run, and passes on any error.

dalcde force-pushed the pytest branch from f22e4d6 to ed47a2b Compare January 9, 2021 02:04

dalcde marked this pull request as ready for review January 10, 2021 00:11

dalcde requested a review from rth January 10, 2021 00:11

rth reviewed Jan 10, 2021

View reviewed changes

dalcde added 3 commits January 10, 2021 20:03

Changes from review

df89b5c

Missed a package

33f85f8

Merge branch 'master' into pytest

f93f333

rth approved these changes Jan 10, 2021

View reviewed changes

docs/testing.md Outdated Show resolved Hide resolved

docs/testing.md Outdated Show resolved Hide resolved

docs/testing.md Outdated Show resolved Hide resolved

Minor documentation fixes

c0bf37b

dalcde mentioned this pull request Jan 11, 2021

ENH: runPython variant with function-like scoping instead of module-like scoping #1106

Closed

rth merged commit 65a9da0 into pyodide:master Jan 11, 2021

hoodmane pushed a commit to hoodmane/pyodide that referenced this pull request Jan 11, 2021

Fix mistake in merging pyodide#1047 and pyodide#1083

4cfc6d8

hoodmane mentioned this pull request Jan 11, 2021

Fix mistake in merging #1047 and #1083 #1121

Merged

rth pushed a commit that referenced this pull request Jan 11, 2021

Fix mistake in merging #1047 and #1083 (#1121)

3635b88

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Experimental new test system #1047

Experimental new test system #1047

dalcde commented Jan 5, 2021

dalcde commented Jan 5, 2021 •

edited

Loading

dalcde commented Jan 5, 2021

dalcde commented Jan 5, 2021

hoodmane commented Jan 6, 2021

rth left a comment

rth Jan 6, 2021

dalcde commented Jan 9, 2021

rth left a comment •

edited

Loading

rth Jan 10, 2021

rth Jan 10, 2021

dalcde commented Jan 10, 2021

rth left a comment

Experimental new test system #1047

Experimental new test system #1047

Conversation

dalcde commented Jan 5, 2021

dalcde commented Jan 5, 2021 • edited Loading

dalcde commented Jan 5, 2021

dalcde commented Jan 5, 2021

hoodmane commented Jan 6, 2021

rth left a comment

Choose a reason for hiding this comment

rth Jan 6, 2021

Choose a reason for hiding this comment

dalcde commented Jan 9, 2021

rth left a comment • edited Loading

Choose a reason for hiding this comment

rth Jan 10, 2021

Choose a reason for hiding this comment

rth Jan 10, 2021

Choose a reason for hiding this comment

dalcde commented Jan 10, 2021

rth left a comment

Choose a reason for hiding this comment

dalcde commented Jan 5, 2021 •

edited

Loading

rth left a comment •

edited

Loading