[test] Re-implement the @all_engines decorator based on parameteraization. NFC (#25115)

This means that we can remove the loop over JS_ENGINES from few more
tests.
    
It also splits up these tests in to smaller units which is always good
and means that it much more obvious what is going on when one of them
fails.

Author: sbc100

src/parseTools.mjs

@@ -1108,10 +1108,12 @@ function ENVIRONMENT_IS_WORKER_THREAD() {
 }
 
 function nodeDetectionCode() {
-  if (ENVIRONMENT == 'node') {
+  if (ENVIRONMENT == 'node' && ASSERTIONS == 0) {
     // The only environment where this code is intended to run is Node.js.
     // Return unconditional true so that later Closure optimizer will be able to
     // optimize code size.
+    // Note that when ASSERTIONS are enabled we still want to runtime detect
+    // node so that errors can be reported when run elsewhere.
     return 'true';
   }
   return "typeof process == 'object' && process.versions?.node && process.type != 'renderer'";

test/common.py

@@ -450,19 +450,42 @@ def crossplatform(f):
   return f
 
 
-# without EMTEST_ALL_ENGINES set we only run tests in a single VM by
-# default. in some tests we know that cross-VM differences may happen and
-# so are worth testing, and they should be marked with this decorator
+# Without EMTEST_ALL_ENGINES set we only run tests in a single VM by default.
+# However, for some tests we know that cross-VM differences may happen and
+# so are worth testing, and they should be marked with this decorator which creates
+# N different variants of the test, one for each engine.
 def all_engines(f):
   assert callable(f)
 
+  if len(config.JS_ENGINES) == 1 or EMTEST_ALL_ENGINES:
+    return f
+
   @wraps(f)
-  def decorated(self, *args, **kwargs):
-    self.use_all_engines = True
+  def metafunc(self, engine, *args, **kwargs):
     self.set_setting('ENVIRONMENT', 'web,node,shell')
+    self.js_engines = [engine]
     f(self, *args, **kwargs)
 
-  return decorated
+  engine_mapping = {}
+  for engine in config.JS_ENGINES:
+    if not engine_mapping:
+      # For the first engine we use no suffix at all.
+      # This uffnsures that one of the tests matches the original name, meaning that
+      # you can still run just `test_foo` and always get the first engine without
+      # needing to write, for example, `test_foo_node`.
+      name = ''
+    else:
+      basename = os.path.basename(engine[0])
+      name = basename
+      suffix = 1
+      while name in engine_mapping:
+        name = f'{basename}_{suffix}'
+        suffix += 1
+    engine_mapping[name] = (engine,)
+
+  parameterize(metafunc, engine_mapping)
+
+  return metafunc
 
 
 @contextlib.contextmanager
@@ -1348,7 +1371,6 @@ def setUp(self):
     self.temp_files_before_run = []
     self.required_engine = None
     self.wasm_engines = config.WASM_ENGINES.copy()
-    self.use_all_engines = EMTEST_ALL_ENGINES
     if self.get_current_js_engine() != config.NODE_JS_TEST:
       # If our primary JS engine is something other than node then enable
       # shell support.
@@ -2103,11 +2125,11 @@ def _build_and_run(self, filename, expected_output, args=None,
     self.assertExists(js_file)
 
     engines = self.js_engines.copy()
-    if len(engines) > 1 and not self.use_all_engines:
+    if len(engines) > 1 and not EMTEST_ALL_ENGINES:
       engines = engines[:1]
     # In standalone mode, also add wasm vms as we should be able to run there too.
     if self.get_setting('STANDALONE_WASM'):
-      # TODO once standalone wasm support is more stable, apply use_all_engines
+      # TODO once standalone wasm support is more stable, apply EMTEST_ALL_ENGINES
       # like with js engines, but for now as we bring it up, test in all of them
       if not self.wasm_engines:
         if 'EMTEST_SKIP_WASM_ENGINE' in os.environ:

test/test_core.py

@@ -8783,6 +8783,7 @@ def test_mallinfo(self):
   def test_wrap_malloc(self, args):
     self.do_runf('core/test_wrap_malloc.c', 'OK.', cflags=args)
 
+  @all_engines
   def test_environment(self):
     self.set_setting('ASSERTIONS')
 
@@ -8794,33 +8795,31 @@ def test(assert_returncode=0):
         js = read_file(self.output_name('test_hello_world'))
       assert ('require(' in js) == ('node' in self.get_setting('ENVIRONMENT')), 'we should have require() calls only if node js specified'
 
-    for engine in config.JS_ENGINES:
-      print(f'engine: {engine}')
-      # set us to test in just this engine
-      self.require_engine(engine)
-      # tell the compiler to build with just that engine
-      if engine == config.NODE_JS_TEST:
-        right = 'node'
-        wrong = 'shell'
-      else:
-        right = 'shell'
-        wrong = 'node'
-      # test with the right env
-      self.set_setting('ENVIRONMENT', right)
-      print('ENVIRONMENT =', self.get_setting('ENVIRONMENT'))
-      test()
-      # test with the wrong env
-      self.set_setting('ENVIRONMENT', wrong)
-      print('ENVIRONMENT =', self.get_setting('ENVIRONMENT'))
-      try:
-        test(assert_returncode=NON_ZERO)
-        raise Exception('unexpected success')
-      except Exception as e:
-        self.assertContained('not compiled for this environment', str(e))
-      # test with a combined env
-      self.set_setting('ENVIRONMENT', right + ',' + wrong)
-      print('ENVIRONMENT =', self.get_setting('ENVIRONMENT'))
-      test()
+    engine = self.get_current_js_engine()
+    print(f'engine: {engine}')
+    # tell the compiler to build with just that engine
+    if engine == config.NODE_JS_TEST:
+      right = 'node'
+      wrong = 'shell'
+    else:
+      right = 'shell'
+      wrong = 'node'
+    # test with the right env
+    self.set_setting('ENVIRONMENT', right)
+    print('ENVIRONMENT =', self.get_setting('ENVIRONMENT'))
+    test()
+    # test with the wrong env
+    self.set_setting('ENVIRONMENT', wrong)
+    print('ENVIRONMENT =', self.get_setting('ENVIRONMENT'))
+    try:
+      test(assert_returncode=NON_ZERO)
+      raise Exception('unexpected success')
+    except Exception as e:
+      self.assertContained('not compiled for this environment', str(e))
+    # test with a combined env
+    self.set_setting('ENVIRONMENT', right + ',' + wrong)
+    print('ENVIRONMENT =', self.get_setting('ENVIRONMENT'))
+    test()
 
   @requires_node
   def test_postrun_exception(self):

test/test_other.py

@@ -5953,11 +5953,12 @@ def test_argv0_node(self):
     output = self.do_runf('code.c')
     self.assertContained('I am ' + utils.normalize_path(os.path.realpath(self.get_dir())) + '/code.js', utils.normalize_path(output))
 
+  @all_engines
   @parameterized({
-    'no_exit_runtime': [True],
-    '': [False],
+    'exit_runtime': (['-sEXIT_RUNTIME=1'],),
+    '': ([],),
   })
-  def test_returncode(self, no_exit):
+  def test_returncode(self, args):
     create_file('src.c', r'''
       #include <stdio.h>
       #include <stdlib.h>
@@ -5969,20 +5970,17 @@ def test_returncode(self, no_exit):
       #endif
       }
     ''')
+    msg = 'but keepRuntimeAlive() is set (counter=0) due to an async operation, so halting execution but not exiting the runtime'
     for code in (0, 123):
       for call_exit in (0, 1):
         for async_compile in (0, 1):
-          self.run_process([EMCC, 'src.c', '-sENVIRONMENT=node,shell', '-DCODE=%d' % code, '-sEXIT_RUNTIME=%d' % (1 - no_exit), '-DCALL_EXIT=%d' % call_exit, '-sWASM_ASYNC_COMPILATION=%d' % async_compile])
-          for engine in config.JS_ENGINES:
-            print(code, call_exit, async_compile, engine)
-            proc = self.run_process(engine + ['a.out.js'], stderr=PIPE, check=False)
-            msg = 'but keepRuntimeAlive() is set (counter=0) due to an async operation, so halting execution but not exiting the runtime'
-            if no_exit and call_exit:
-              self.assertContained(msg, proc.stderr)
-            else:
-              self.assertNotContained(msg, proc.stderr)
-            # we always emit the right exit code, whether we exit the runtime or not
-            self.assertEqual(proc.returncode, code)
+          print('code', code, 'call_exit', call_exit, 'async_compile', async_compile)
+          cflags = args + [f'-DCODE={code}', f'-DCALL_EXIT={call_exit}', f'-sWASM_ASYNC_COMPILATION={async_compile}']
+          output = self.do_runf('src.c', cflags=cflags, assert_returncode=code)
+          if '-sEXIT_RUNTIME=1' not in args and call_exit:
+            self.assertContained(msg, output)
+          else:
+            self.assertNotContained(msg, output)
 
   def test_emscripten_force_exit_NO_EXIT_RUNTIME(self):
     create_file('src.c', r'''
@@ -8920,6 +8918,7 @@ def test_run_order(self):
     ]
     self.do_runf('src.c', '\n'.join(expected_order))
 
+  @all_engines
   def test_override_js_execution_environment(self):
     create_file('main.c', r'''
       #include <emscripten.h>
@@ -8934,19 +8933,19 @@ def test_override_js_execution_environment(self):
 ''')
     self.run_process([EMCC, 'main.c', '-sENVIRONMENT=node,shell'])
     src = read_file('a.out.js')
+    engine = self.get_current_js_engine()
     for env in ['web', 'worker', 'node', 'shell']:
-      for engine in config.JS_ENGINES:
-        actual = 'NODE' if engine == config.NODE_JS_TEST else 'SHELL'
-        print(env, actual, engine)
-        module = {'ENVIRONMENT': env}
-        if env != actual:
-          # avoid problems with arguments detection, which may cause very odd failures with the wrong environment code
-          module['arguments'] = []
-        curr = 'var Module = %s;\n' % str(module)
-        print('    ' + curr)
-        create_file('test.js', curr + src)
-        seen = self.run_js('test.js', engine=engine, assert_returncode=NON_ZERO)
-        self.assertContained('Module.ENVIRONMENT has been deprecated. To force the environment, use the ENVIRONMENT compile-time option (for example, -sENVIRONMENT=web or -sENVIRONMENT=node', seen)
+      actual = 'NODE' if engine == config.NODE_JS_TEST else 'SHELL'
+      print('env', env, 'actual', actual, 'engine', engine)
+      module = {'ENVIRONMENT': env}
+      if env != actual:
+        # avoid problems with arguments detection, which may cause very odd failures with the wrong environment code
+        module['arguments'] = []
+      curr = 'var Module = %s;\n' % str(module)
+      print('    ' + curr)
+      create_file('test.js', curr + src)
+      seen = self.run_js('test.js', assert_returncode=NON_ZERO)
+      self.assertContained('Module.ENVIRONMENT has been deprecated. To force the environment, use the ENVIRONMENT compile-time option (for example, -sENVIRONMENT=web or -sENVIRONMENT=node', seen)
 
   def test_override_c_environ(self):
     create_file('pre.js', r'''