Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Results of running the regression test suite on nodejs #41

Closed
emmatyping opened this issue Jan 10, 2022 · 7 comments
Closed

Results of running the regression test suite on nodejs #41

emmatyping opened this issue Jan 10, 2022 · 7 comments

Comments

@emmatyping
Copy link
Owner

emmatyping commented Jan 10, 2022

After spending a lot of time going through all the tests, here is a summary of my findings towards running the regression test suite on Emscripten/nodejs:

The command line I used and exclude list I used are here: https://gist.github.com/ethanhs/656637de570cc2c239c8728c4aa1e731

The list of tests excluded also includes a brief description of each test's failure(s). A quick summary:

  • almost half of the test either directly import subprocess (51 tests) or indirectly does so via e.g. unitest.mock (97 tests)
  • despite pthread being compiled in, I seem unable to create threads under nodejs (6 tests), I am getting EAGAIN...
  • there are some locale issues which I presume are Emscripten bugs
  • it also requires a couple of small changes to CPython, to patch around an Emscripten bug and also to disable the faulthandler thread because it won't start and causes the test runner to crash after a successful run.
diff --git a/Lib/test/libregrtest/main.py b/Lib/test/libregrtest/main.py
index fc3c2b9692..0879bb6baa 100644
--- a/Lib/test/libregrtest/main.py
+++ b/Lib/test/libregrtest/main.py
@@ -659,7 +659,9 @@ def main(self, tests=None, **kwargs):
        except SystemExit as exc:
            # bpo-38203: Python can hang at exit in Py_Finalize(), especially
            # on threading._shutdown() call: put a timeout
-            faulthandler.dump_traceback_later(EXIT_TIMEOUT, exit=True)
+            # Emscripten has issues starting threads
+            if sys.platform != 'emscripten':
+                faulthandler.dump_traceback_later(EXIT_TIMEOUT, exit=True)

            sys.exit(exc.code)

diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c
index 89e93c5818..dbd7d4dc63 100644
--- a/Modules/socketmodule.c
+++ b/Modules/socketmodule.c
@@ -5114,6 +5114,7 @@ sock_initobj(PyObject *self, PyObject *args, PyObject *kwds)
                                     &family, &type, &proto, &fdobj))
        return -1;

+
#ifdef MS_WINDOWS
    /* In this case, we don't use the family, type and proto args */
    if (fdobj == NULL || fdobj == Py_None)
@@ -7926,7 +7927,7 @@ PyInit__socket(void)
#ifdef  IPPROTO_VRRP
    PyModule_AddIntMacro(m, IPPROTO_VRRP);
#endif
-#ifdef  IPPROTO_SCTP
+#if defined(IPPROTO_SCTP) && !defined(__EMSCRIPTEN__)
    PyModule_AddIntMacro(m, IPPROTO_SCTP);
#endif
#ifdef  IPPROTO_BIP

Huge thanks to @tiran for helping figure out the IPPROTO_SCTP issue which was causing test_socket to crash the test runner!

With this I think the steps towards adding a build bot for Emscripten/nodejs:

Want to run things yourself? Here are the steps:

  1. Do a normal checkout of this repo, pull my above mentioned PR
  2. Apply the above patches
  3. Grab the tests.txt file in the above gist and put it in the top level of the checkout
  4. Run the command in the gist to run the test suite!
@emmatyping
Copy link
Owner Author

emmatyping commented Jan 10, 2022

I've fixed the threading issue, it seems we need to pass -pthread in a few more places and -s USE_PTHREADS at link time.

We also need to decide whether we should use -s PROXY_TO_PTHREAD, which runs the main thread in a webworker/thread or use -s PTHREAD_POOL_SIZE="_emscripten_num_logical_cores()" -s DEFAULT_LIBRARY_FUNCS_TO_INCLUDE="emscripten_num_logical_cores" which makes a threadpool for Emscripten to use with the number of cores on the system (this seems better)

@tiran
Copy link
Collaborator

tiran commented Jan 12, 2022

_testcapi used in lots of tests. We might be able to turn it into a built-in module.

--- a/Modules/_testcapimodule.c
+++ b/Modules/_testcapimodule.c
@@ -12,6 +12,14 @@
    macro defined, but only the public C API must be tested here. */
 
 #undef Py_BUILD_CORE_MODULE
+
+/* On WASM shared extensions modules do not work. Allow building with
+   statically */
+
+#ifdef __wasm__
+#  undef Py_BUILD_CORE_BUILTIN
+#endif
+
 /* Always enable assertions */
 #undef NDEBUG
 

@tiran
Copy link
Collaborator

tiran commented Jan 12, 2022

With python/cpython#30552 and --with-emscripten-target=browser the faulthandler workaround is no longer necessary. test_hashlib is passing, too.

# node --experimental-wasm-threads --experimental-wasm-bulk-memory ./python.js -m test test
0:00:00 load avg: 0.10 Run tests sequentially
0:00:00 load avg: 0.10 [1/1] test_hashlib
warning: unsupported syscall: __syscall_wait4

== Tests result: SUCCESS ==

1 test OK.

Total duration: 2.7 sec
Tests result: SUCCESS

@emmatyping
Copy link
Owner Author

Yeah the faulthandler work around was only needed because threads were broken. I believe a couple of other tests like test_enum should now work to!

@tiran
Copy link
Collaborator

tiran commented Jan 12, 2022

clock_nanosleep is broken, sleeps forever. I'm adding a workaround to my PR.

@brettcannon
Copy link
Collaborator

Now that we have nightly CI working, is this issue worth keeping open, or are the more targeted issues we should open about what to try and tackle?

@emmatyping
Copy link
Owner Author

I think this is pretty much superseded by #43 and we can open issues with anything else that isn't working (that needs fixing on the CPython end)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants