tools/test.py is not yet Python 3 compatible #29246

cclauss · 2019-08-21T13:41:42Z

The other four Travis CI test runs now complete successfully on Python 3.6 but the Test C++ Suites run fails. HELP NEEDED: This blocks our progress on Python 3 compatibility and the root cause is hidden behind JavaScript code that I do not understand.

See #29451 to understand how the C++ tests fail on Travis CI. Any progress or hints would be much appreciated.

Note that https://github.com/nodejs/node/blob/master/tools/test.py#L32 does import imp which raises a deprecation warning and sometime soon we need to modify the related code to use import importlib (and possibly also import importlib.util) https://docs.python.org/3/library/importlib.html#module-importlib.util instead but I have tried a few times without success. I do not believe that this is the source of our inability to pass the Test C++ Suites.

As mentioned at:

Version:
Platform:
Subsystem:

cclauss · 2019-08-27T09:32:12Z

@nodejs/python

Trott · 2019-08-27T21:42:10Z

@nodejs/testing

sam-github · 2019-09-04T20:35:22Z

@cclauss I'd like to help, but I don't know what C++ tests this refers to, or how to reproduce it.

make PYTHON=python3 test (which includes cctest) and the more explicit python3 tools/test.py -J --mode=release both pass on Linux, with python 3.7.3

Could you provide an example make or other CLI command that fails, so I can try running it?

Or, if its Travis specific, could you provide a link to a failing travis build, preferably with a PR branch that causes the failure (assuming some kind of Travis config change is required to cause the failure)?

This helps contributors understand how they can test Python 3.6 or 3.7 on Travis CI. Helps with issues like #29246 (comment)

cclauss · 2019-09-05T14:24:14Z

The other four Travis CI test runs now complete successfully on Python 3.6

This means that this issue is about Travis CI tests on Python 3.6.
It is also trying to point out there are five tests runs that are made on Travis CI.

but the Test C++ Suites run fails.

This means that the other four tests can safely be ignored for now because they already pass.
Test C++ Suites is in bold because it is the one test run that is failing and needs attention.

#29360 made these test failures evident but unfortunately it had to be reverted. Now #29451 clarifies how to test Python 3 on Travis CI.

Results from one week ago: https://travis-ci.com/nodejs/node/builds/125061793

richardlau · 2019-09-05T14:53:00Z

The Test C++ Suites run node-gyp to build the addons tests. Is node-gyp expected to be Python 3.6 compatible?

This helps contributors understand how they can test Python 3.6 or 3.7 on Travis CI. Helps with issues like: - #29246 (comment)

sam-github · 2019-09-05T17:40:27Z

Thanks for #29451, I attempted to use it to reproduce this, but since compilation is blocked by #29340, I'm not able to, yet.

Are there other fixes I can cherry-pick into #29451 to get a compile, so that I can get to the "Test C++ Suites" stage?

cclauss · 2019-09-06T04:40:43Z

#29346

This helps contributors understand how they can test Python 3.6 or 3.7 on Travis CI. Helps with issues like: - nodejs#29246 (comment)

sam-github · 2019-09-06T19:43:02Z

The root cause of this is that the node-gyp.js in the npm that is included in node's deps/ ONLY works with python 2.7, at least by default. Poking around in deps/npm/node_modules/node-gyp/lib/find-python.js it appears node-gyp allows python3, if EXPERIMENTAL_NODE_GYP_PYTHON3=yes.

Retrying that, we get back to failures in python instead of javascript. Yay?

% make -j1 V=1 test/addons/.buildstamp EXPERIMENTAL_NODE_GYP_PYTHON3=yes
Building addon in /home/sam/w/core/lts/test/addons/01_function_arguments
Building addon in /home/sam/w/core/lts/test/addons/02_callbacks
Building addon in /home/sam/w/core/lts/test/addons/03_object_factory
Building addon in /home/sam/w/core/lts/test/addons/04_function_factory
/home/sam/w/core/lts/tools/build-addons.js:58
main(process.argv[3]).catch((err) => setImmediate(() => { throw err; }));
                                                          ^

Error: Command failed: /home/sam/w/core/lts/out/Release/node /home/sam/w/core/lts/deps/npm/node_modules/node-gyp/bin/node-gyp.js rebuild --directory=/home/sam/w/core/lts/test/addons/01_function_arguments
Traceback (most recent call last):
  File "/home/sam/w/core/lts/deps/npm/node_modules/node-gyp/gyp/gyp_main.py", line 13, in <module>
    import gyp
  File "/home/sam/w/core/lts/deps/npm/node_modules/node-gyp/gyp/pylib/gyp/__init__.py", line 10, in <module>
    import gyp.input
  File "/home/sam/w/core/lts/deps/npm/node_modules/node-gyp/gyp/pylib/gyp/input.py", line 7, in <module>
    from compiler.ast import Const
ModuleNotFoundError: No module named 'compiler'

    at ChildProcess.exithandler (child_process.js:295:12)
    at ChildProcess.emit (events.js:209:13)
    at maybeClose (internal/child_process.js:1028:16)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:283:5) {
  killed: false,
  code: 1,
  signal: null,
  cmd: '/home/sam/w/core/lts/out/Release/node /home/sam/w/core/lts/deps/npm/node_modules/node-gyp/bin/node-gyp.js rebuild --directory=/home/sam/w/core/lts/test/addons/01_function_arguments',
  stdout: '',
  stderr: 'Traceback (most recent call last):\n' +
    '  File "/home/sam/w/core/lts/deps/npm/node_modules/node-gyp/gyp/gyp_main.py", line 13, in <module>\n' +
    '    import gyp\n' +
    '  File "/home/sam/w/core/lts/deps/npm/node_modules/node-gyp/gyp/pylib/gyp/__init__.py", line 10, in <module>\n' +
    '    import gyp.input\n' +
    '  File "/home/sam/w/core/lts/deps/npm/node_modules/node-gyp/gyp/pylib/gyp/input.py", line 7, in <module>\n' +
    '    from compiler.ast import Const\n' +
    "ModuleNotFoundError: No module named 'compiler'\n"
}
make: *** [Makefile:412: test/addons/.buildstamp] Error 1

sam-github · 2019-09-06T19:57:26Z

@nodejs/npm @nodejs/gyp --- I suspect the above is known.

Since node tests whether addons can be built with the node-gyp included with npm, that's going to block node's tests passing until its fixed in npm.

Of course, it was always going to be a blocker for the ecosystem.

py3 support does exist, nodejs/node-gyp#1844, perhaps this conversation should move over there.

bnoordhuis · 2019-09-06T21:06:16Z

We could vendor node-gyp in deps/ as a temporary workaround until npm picks up a python3-capable node-gyp (and we pick up that version of npm), it only needs trivial changes to the Makefile:

diff --git a/Makefile b/Makefile
index 33d43798f5..d38b11e6cb 100644
--- a/Makefile
+++ b/Makefile
@@ -351,7 +351,7 @@ benchmark/napi/function_call/build/$(BUILDTYPE)/binding.node: \
                benchmark/napi/function_call/napi_binding.c \
                benchmark/napi/function_call/binding.cc \
                benchmark/napi/function_call/binding.gyp | all
-       $(NODE) deps/npm/node_modules/node-gyp/bin/node-gyp rebuild \
+       $(NODE) deps/node-gyp/bin/node-gyp rebuild \
                --python="$(PYTHON)" \
                --directory="$(shell pwd)/benchmark/napi/function_call" \
                --nodedir="$(shell pwd)"
@@ -360,7 +360,7 @@ benchmark/napi/function_args/build/$(BUILDTYPE)/binding.node: \
                benchmark/napi/function_args/napi_binding.c \
                benchmark/napi/function_args/binding.cc \
                benchmark/napi/function_args/binding.gyp | all
-       $(NODE) deps/npm/node_modules/node-gyp/bin/node-gyp rebuild \
+       $(NODE) deps/node-gyp/bin/node-gyp rebuild \
                --python="$(PYTHON)" \
                --directory="$(shell pwd)/benchmark/napi/function_args" \
                --nodedir="$(shell pwd)"
@@ -391,14 +391,14 @@ ADDONS_BINDING_SOURCES := \
        $(filter-out test/addons/??_*/*.h, $(wildcard test/addons/*/*.h))
 
 ADDONS_PREREQS := config.gypi \
-       deps/npm/node_modules/node-gyp/package.json tools/build-addons.js \
+       deps/node-gyp/package.json tools/build-addons.js \
        deps/uv/include/*.h deps/v8/include/*.h \
        src/node.h src/node_buffer.h src/node_object_wrap.h src/node_version.h
 
 define run_build_addons
 env npm_config_loglevel=$(LOGLEVEL) npm_config_nodedir="$$PWD" \
        npm_config_python="$(PYTHON)" $(NODE) "$$PWD/tools/build-addons" \
-       "$$PWD/deps/npm/node_modules/node-gyp/bin/node-gyp.js" \
+       "$$PWD/deps/node-gyp/bin/node-gyp.js" \
        $1
 touch $2
 endef

sam-github · 2019-09-06T21:36:10Z

@bnoordhuis we could, but should we? If the test is trying to show addons can be built by npm (as people actually do, for the most part), then showing that upstream node-gyp works OK doesn't help so much. That said, I might give it a shot to see if updating node-gyp allows the build to continue and uncover a later problem.

This helps contributors understand how they can test Python 3.6 or 3.7 on Travis CI. Helps with issues like: - #29246 (comment)

cclauss · 2019-09-07T06:55:52Z

Perhaps we can modify just two deps/npm files for now... #29451 (comment)

rvagg · 2019-09-09T02:32:13Z

do we need to get a release out? npm is pretty quick in picking up our releases these days, so if we have something in the queue that we want to get into the funnel then we should just push it. We have an awkward breaking-change list now so we might need to start maintaining two release lines for a little while, 5 & 6.

This helps contributors understand how they can test Python 3.6 or 3.7 on Travis CI. Helps with issues like: - #29246 (comment)

This helps contributors understand how they can test Python 3.6 or 3.7 on Travis CI. Helps with issues like: - nodejs#29246 (comment)

cclauss · 2019-09-10T22:23:07Z

Are there any objections to @rvagg's plan above and make a release and then encourage npm to adopt it so that we can cherrypick it back in?!?

This helps contributors understand how they can test Python 3.6 or 3.7 on Travis CI. Helps with issues like: - nodejs#29246 (comment)

cclauss · 2019-09-11T21:47:24Z

Closing this issue because the file tools/test.py was not incompatible with Python 3. #29451 has proven that changes made to several other files can enable tools/test.py to pass on Python 3.

cclauss added the python PRs and issues that require attention from people who are familiar with Python. label Aug 21, 2019

cclauss mentioned this issue Aug 21, 2019

build: ongoing list of actions for Python 3 compatibility #25789

Closed

63 tasks

cclauss added the help wanted Issues that need assistance from volunteers or PRs that need help to proceed. label Aug 27, 2019

This was referenced Aug 28, 2019

build: add full Python 3 tests to Travis CI #29360

Closed

deprecated python3 module "imp" used in tools/test.py #29418

Closed

cclauss added a commit that referenced this issue Sep 5, 2019

build: explain how to test Python 3.6 or 3.7

3043157

This helps contributors understand how they can test Python 3.6 or 3.7 on Travis CI. Helps with issues like #29246 (comment)

cclauss mentioned this issue Sep 5, 2019

build: test Python 3.7 on Travis CI #29451

Closed

3 tasks

sam-github pushed a commit that referenced this issue Sep 5, 2019

build: explain how to test Python 3.6 or 3.7

89129b1

This helps contributors understand how they can test Python 3.6 or 3.7 on Travis CI. Helps with issues like: - #29246 (comment)

cclauss added a commit to cclauss/node that referenced this issue Sep 6, 2019

build: explain how to test Python 3.6 or 3.7

b5fd477

This helps contributors understand how they can test Python 3.6 or 3.7 on Travis CI. Helps with issues like: - nodejs#29246 (comment)

sam-github mentioned this issue Sep 6, 2019

build: add comment to .travis.yml on how to test Py3 #29473

Closed

4 tasks

This was referenced Sep 6, 2019

Accept Python 3 by default nodejs/node-gyp#1844

Closed

doc: add minutes for meeting 28 Aug 2019 nodejs/TSC#751

Merged

cclauss added a commit that referenced this issue Sep 7, 2019

build: explain how to test Python 3.6 or 3.7

fd29bde

This helps contributors understand how they can test Python 3.6 or 3.7 on Travis CI. Helps with issues like: - #29246 (comment)

cclauss added a commit that referenced this issue Sep 9, 2019

build: explain how to test Python 3.6 or 3.7

3d1abce

This helps contributors understand how they can test Python 3.6 or 3.7 on Travis CI. Helps with issues like: - #29246 (comment)

cclauss added a commit to cclauss/node that referenced this issue Sep 10, 2019

build: explain how to test Python 3.6 or 3.7

a08e709

This helps contributors understand how they can test Python 3.6 or 3.7 on Travis CI. Helps with issues like: - nodejs#29246 (comment)

cclauss added a commit to cclauss/node that referenced this issue Sep 11, 2019

build: explain how to test Python 3.6 or 3.7

8b4d70e

This helps contributors understand how they can test Python 3.6 or 3.7 on Travis CI. Helps with issues like: - nodejs#29246 (comment)

cclauss closed this as completed Sep 11, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tools/test.py is not yet Python 3 compatible #29246

tools/test.py is not yet Python 3 compatible #29246

cclauss commented Aug 21, 2019 •

edited

cclauss commented Aug 27, 2019

Trott commented Aug 27, 2019

sam-github commented Sep 4, 2019

cclauss commented Sep 5, 2019 •

edited

richardlau commented Sep 5, 2019

sam-github commented Sep 5, 2019

cclauss commented Sep 6, 2019

sam-github commented Sep 6, 2019

sam-github commented Sep 6, 2019

bnoordhuis commented Sep 6, 2019

sam-github commented Sep 6, 2019

cclauss commented Sep 7, 2019 •

edited

rvagg commented Sep 9, 2019

cclauss commented Sep 10, 2019

cclauss commented Sep 11, 2019

tools/test.py is not yet Python 3 compatible #29246

tools/test.py is not yet Python 3 compatible #29246

Comments

cclauss commented Aug 21, 2019 • edited

cclauss commented Aug 27, 2019

Trott commented Aug 27, 2019

sam-github commented Sep 4, 2019

cclauss commented Sep 5, 2019 • edited

richardlau commented Sep 5, 2019

sam-github commented Sep 5, 2019

cclauss commented Sep 6, 2019

sam-github commented Sep 6, 2019

sam-github commented Sep 6, 2019

bnoordhuis commented Sep 6, 2019

sam-github commented Sep 6, 2019

cclauss commented Sep 7, 2019 • edited

rvagg commented Sep 9, 2019

cclauss commented Sep 10, 2019

cclauss commented Sep 11, 2019

cclauss commented Aug 21, 2019 •

edited

cclauss commented Sep 5, 2019 •

edited

cclauss commented Sep 7, 2019 •

edited