Skip to content

Remove node canary test decorators. NFC #24478

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -524,15 +524,15 @@ jobs:
test-core0:
executor: focal
environment:
EMTEST_SKIP_NODE_CANARY: "1"
EMTEST_SKIP_NODE24: "1"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps 24 => latest or such, so we can update it without changing all these places?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that we don't want tests marked as require_node_version(24) to be disabled by EMTEST_SKIP_NODE_LATEST once latest becomes 25 or 26.

Unless you think we should also replace require_node_version(24) with just require_node_latest?

The approach here seems to me like the least complicated approach for now. We can revisit later.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough, this does seem simple for now.

steps:
- run-tests-linux:
test_targets: "core0"
test-core2:
executor: focal
environment:
EMTEST_BROWSER: "node"
EMTEST_SKIP_NODE_CANARY: "1"
EMTEST_SKIP_NODE24: "1"
steps:
- run-tests-linux:
# also run a few asan tests. Run these with frozen_cache disabled
Expand Down Expand Up @@ -577,7 +577,7 @@ jobs:
test-core3:
executor: focal
environment:
EMTEST_SKIP_NODE_CANARY: "1"
EMTEST_SKIP_NODE24: "1"
steps:
- run-tests-linux:
frozen_cache: false
Expand Down Expand Up @@ -656,7 +656,7 @@ jobs:
test-modularize-instance:
executor: focal
environment:
EMTEST_SKIP_NODE_CANARY: "1"
EMTEST_SKIP_NODE24: "1"
steps:
- run-tests-linux:
test_targets: "instance"
Expand All @@ -677,7 +677,7 @@ jobs:
- upload-test-results
test-wasm2js1:
environment:
EMTEST_SKIP_NODE_CANARY: "1"
EMTEST_SKIP_NODE24: "1"
executor: focal
steps:
- run-tests-linux:
Expand Down Expand Up @@ -920,7 +920,7 @@ jobs:
test-other:
executor: focal
environment:
EMTEST_SKIP_NODE_CANARY: "1"
EMTEST_SKIP_NODE24: "1"
EMTEST_SKIP_RUST: "1"
EMTEST_SKIP_WASM64: "1"
EMTEST_SKIP_NEW_CMAKE: "1"
Expand Down Expand Up @@ -997,7 +997,7 @@ jobs:
executor: focal
environment:
EMTEST_LACKS_WEBGPU: "1"
EMTEST_SKIP_NODE_CANARY: "1"
EMTEST_SKIP_NODE24: "1"
steps:
- run-tests-chrome:
title: "browser64_4gb"
Expand Down Expand Up @@ -1069,7 +1069,7 @@ jobs:
EMTEST_SKIP_WASM64: "1"
EMTEST_SKIP_SCONS: "1"
EMTEST_SKIP_RUST: "1"
EMTEST_SKIP_NODE_CANARY: "1"
EMTEST_SKIP_NODE24: "1"
EMTEST_BROWSER: "0"
steps:
- checkout
Expand Down
40 changes: 19 additions & 21 deletions test/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -263,15 +263,19 @@ def decorated(self, *args, **kwargs):
return decorated


def requires_node_canary(func):
assert callable(func)
def requires_node_version(major):
assert not callable(major)

@wraps(func)
def decorated(self, *args, **kwargs):
self.require_node_canary()
return func(self, *args, **kwargs)
def decorator(f):
assert callable(f)

return decorated
@wraps(f)
def decorated(self, *args, **kwargs):
self.require_node_version(major)
return f(self, *args, **kwargs)
return decorated

return decorator


# Used to mark dependencies in various tests to npm developer dependency
Expand Down Expand Up @@ -1012,20 +1016,6 @@ def require_node(self):
self.require_engine(nodejs)
return nodejs

def node_is_canary(self, nodejs):
return nodejs and nodejs[0] and 'canary' in nodejs[0]

def require_node_canary(self):
nodejs = self.get_nodejs()
if self.node_is_canary(nodejs):
self.require_engine(nodejs)
return

if 'EMTEST_SKIP_NODE_CANARY' in os.environ:
self.skipTest('test requires node canary and EMTEST_SKIP_NODE_CANARY is set')
else:
self.fail('node canary required to run this test. Use EMTEST_SKIP_NODE_CANARY to skip')

def require_engine(self, engine):
logger.debug(f'require_engine: {engine}')
if self.required_engine and self.required_engine != engine:
Expand Down Expand Up @@ -1062,6 +1052,14 @@ def try_require_node_version(self, major, minor = 0, revision = 0):
self.js_engines = [nodejs]
return True

def require_node_version(self, major):
if not self.try_require_node_version(major):
env = f'EMTEST_SKIP_NODE{major}'
if env in os.environ:
self.skipTest(f'test requires node >= {major} (and {env} is set)')
else:
self.fail(f'node >= {major} required to run wasm64 tests. Use {env} to skip')

def require_simd(self):
if self.is_browser_test():
return
Expand Down
4 changes: 2 additions & 2 deletions test/test_browser.py
Original file line number Diff line number Diff line change
Expand Up @@ -1643,9 +1643,9 @@ def test_hello_world_worker(self, file_data):
self.run_browser('main.html', '/report_result?hello from worker, and :' + ('data for w' if file_data else '') + ':')

# code should run standalone too
# To great memories >4gb we need the canary version of node
# To create memories >4gb we need a recent version of node
if self.is_4gb():
self.require_node_canary()
self.require_node_version(24)
self.assertContained('you should not see this text when in a worker!', self.run_js('worker.js'))

@no_wasmfs('https://github.com/emscripten-core/emscripten/issues/19608')
Expand Down
7 changes: 3 additions & 4 deletions test/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
from common import RunnerCore, path_from_root, requires_native_clang, test_file, create_file
from common import skip_if, no_windows, no_mac, is_slow_test, parameterized, parameterize
from common import env_modify, with_env_modify, disabled, flaky, node_pthreads, also_with_wasm_bigint
from common import read_file, read_binary, requires_v8, requires_node, requires_dev_dependency, requires_wasm2js, requires_node_canary
from common import read_file, read_binary, requires_v8, requires_node, requires_dev_dependency, requires_wasm2js
from common import compiler_for, crossplatform, no_4gb, no_2gb, also_with_minimal_runtime, also_with_modularize
from common import with_all_fs, also_with_nodefs, also_with_nodefs_both, also_with_noderawfs, also_with_wasmfs
from common import with_all_eh_sjlj, with_all_sjlj, also_with_standalone_wasm, can_do_standalone, no_wasm64, requires_wasm_eh, requires_jspi
Expand Down Expand Up @@ -409,7 +409,7 @@ def should_use_closure(self):
return all(f not in self.emcc_args for f in prohibited) and any(f in self.emcc_args for f in required)

def setup_esm_integration(self):
self.require_node_canary()
self.require_node_version(24)
self.node_args += ['--experimental-wasm-modules', '--no-warnings']
self.set_setting('WASM_ESM_INTEGRATION')
self.emcc_args += ['-Wno-experimental']
Expand Down Expand Up @@ -8408,11 +8408,10 @@ def test_asyncify_main_module(self):
self.do_core_test('test_hello_world.c')

# Test that pthread_join works correctly with asyncify.
@requires_node_canary
@node_pthreads
@requires_jspi
def test_pthread_join_and_asyncify(self):
# TODO Test with ASYNCIFY=1 https://github.com/emscripten-core/emscripten/issues/17552
self.require_jspi()
self.do_runf('core/test_pthread_join_and_asyncify.c', 'joining thread!\njoined thread!',
emcc_args=['-sJSPI',
'-sEXIT_RUNTIME=1',
Expand Down
10 changes: 4 additions & 6 deletions test/test_other.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
from common import RunnerCore, path_from_root, is_slow_test, ensure_dir, disabled, make_executable
from common import env_modify, no_mac, no_windows, only_windows, requires_native_clang, with_env_modify
from common import create_file, parameterized, NON_ZERO, node_pthreads, TEST_ROOT, test_file
from common import compiler_for, EMBUILDER, requires_v8, requires_node, requires_wasm64, requires_node_canary, requires_dev_dependency
from common import compiler_for, EMBUILDER, requires_v8, requires_node, requires_wasm64, requires_node_version, requires_dev_dependency
from common import requires_wasm_eh, crossplatform, with_all_eh_sjlj, with_all_sjlj, requires_jspi
from common import also_with_standalone_wasm, also_with_wasm2js, also_with_noderawfs
from common import also_with_modularize, also_with_wasmfs, with_all_fs
Expand Down Expand Up @@ -357,7 +357,7 @@ def test_esm(self, args):
self.assertContained('export default Module;', read_file('hello_world.mjs'))
self.assertContained('hello, world!', self.run_js('hello_world.mjs'))

@requires_node_canary
@requires_node_version(24)
def test_esm_source_phase_imports(self):
self.node_args += ['--experimental-wasm-modules', '--no-warnings']
self.run_process([EMCC, '-o', 'hello_world.mjs', '-sSOURCE_PHASE_IMPORTS',
Expand Down Expand Up @@ -3414,7 +3414,7 @@ def test_embind_return_value_policy(self):

self.do_runf('embind/test_return_value_policy.cpp')

@requires_node_canary
@requires_node_version(24)
def test_embind_resource_management(self):
self.node_args.append('--js-explicit-resource-management')

Expand Down Expand Up @@ -7471,7 +7471,6 @@ def test_failing_growth_2gb(self):
self.assertContained('done', self.run_js('a.out.js'))

@requires_wasm64
@requires_node_canary
def test_failing_growth_wasm64(self):
self.require_wasm64()
create_file('test.c', r'''
Expand Down Expand Up @@ -15501,7 +15500,6 @@ def test_quick_exit(self):
self.do_other_test('test_quick_exit.c')

@requires_wasm64
@requires_node_canary
def test_memory64_proxies(self):
self.run_process([EMCC, test_file('hello_world.c'),
'-sMEMORY64=1',
Expand Down Expand Up @@ -16110,7 +16108,7 @@ def test_locate_file_abspath_esm(self, args):
emcc_args=['--pre-js', 'pre.js',
'--extern-post-js', test_file('modularize_post_js.js')] + args)

@requires_node_canary
@requires_node_version(24)
def test_js_base64_api(self):
self.node_args += ['--js_base_64']
self.do_runf('hello_world.c', 'hello, world!', emcc_args=['-sSINGLE_FILE'], output_basename='baseline')
Expand Down