tools/test.py is not yet Python 3 compatible

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:

@nodejs/python

@nodejs/testing

@cclauss

@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)?

build: explain how to test Python 3.6 or 3.7

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

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

build: explain how to test Python 3.6 or 3.7

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?

#29346

build: explain how to test Python 3.6 or 3.7

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

@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.

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:

Details

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

@bnoordhuis

@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.

build: explain how to test Python 3.6 or 3.7

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

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.

@rvagg

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?!?

build: explain how to test Python 3.6 or 3.7

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.

Navigation Menu

Repository navigation

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

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

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

Description

Activity

cclauss commented on Aug 27, 2019

Trott commented on Aug 27, 2019

sam-github commented on Sep 4, 2019

cclauss commented on Sep 5, 2019

richardlau commented on Sep 5, 2019

sam-github commented on Sep 5, 2019

cclauss commented on Sep 6, 2019

sam-github commented on Sep 6, 2019

sam-github commented on Sep 6, 2019

bnoordhuis commented on Sep 6, 2019

sam-github commented on Sep 6, 2019

cclauss commented on Sep 7, 2019

rvagg commented on Sep 9, 2019

cclauss commented on Sep 10, 2019

cclauss commented on Sep 11, 2019

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions