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

Reverts #25115

Author: sbc100

src/parseTools.mjs

@@ -1108,12 +1108,10 @@ function ENVIRONMENT_IS_WORKER_THREAD() {
 }
 
 function nodeDetectionCode() {
-  if (ENVIRONMENT == 'node' && ASSERTIONS == 0) {
+  if (ENVIRONMENT == 'node') {
     // 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

@@ -458,42 +458,19 @@ def crossplatform(f):
   return f
 
 
-# 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.
+# 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
 def all_engines(f):
   assert callable(f)
 
-  if len(config.JS_ENGINES) == 1 or EMTEST_ALL_ENGINES:
-    return f
-
   @wraps(f)
-  def metafunc(self, engine, *args, **kwargs):
+  def decorated(self, *args, **kwargs):
+    self.use_all_engines = True
     self.set_setting('ENVIRONMENT', 'web,node,shell')
-    self.js_engines = [engine]
     f(self, *args, **kwargs)
 
-  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
+  return decorated
 
 
 @contextlib.contextmanager
@@ -1379,6 +1356,7 @@ 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.
@@ -2133,11 +2111,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 EMTEST_ALL_ENGINES:
+    if len(engines) > 1 and not self.use_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 EMTEST_ALL_ENGINES
+      # TODO once standalone wasm support is more stable, apply use_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

@@ -8782,7 +8782,6 @@ 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,31 +8793,33 @@ 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'
 
-    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()
+    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()
 
   @requires_node
   def test_postrun_exception(self):

test/test_other.py

@@ -5953,12 +5953,11 @@ 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({
-    'exit_runtime': (['-sEXIT_RUNTIME=1'],),
-    '': ([],),
+    'no_exit_runtime': [True],
+    '': [False],
   })
-  def test_returncode(self, args):
+  def test_returncode(self, no_exit):
     create_file('src.c', r'''
       #include <stdio.h>
       #include <stdlib.h>
@@ -5970,17 +5969,20 @@ def test_returncode(self, args):
       #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):
-          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)
+          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)
 
   def test_emscripten_force_exit_NO_EXIT_RUNTIME(self):
     create_file('src.c', r'''
@@ -8917,7 +8919,6 @@ 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>
@@ -8932,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']:
-      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)
+      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)
 
   def test_override_c_environ(self):
     create_file('pre.js', r'''