Prefer command line over environment variables for test options (#15559)

At least for some of these options, the command line makes more
sense.  In general we should prefer the command line for all the
normal reasons (e.g. less prone to typos, more local).

For historical reasons (command line options just didn't work
in the past) these evolved as environment variables.

Author: sbc100

tests/common.py

@@ -434,8 +434,8 @@ def setUp(self):
           print('Not clearing existing test directory')
         else:
           print('Clearing existing test directory')
-          # Even when EMTEST_SAVE_DIR we still try to start with an empty directoy as many tests
-          # expect this.  EMTEST_SAVE_DIR=2 can be used to keep the old contents for the new test
+          # Even when --save-dir is used we still try to start with an empty directory as many tests
+          # expect this.  --no-clean can be used to keep the old contents for the new test
           # run. This can be useful when iterating on a given test with extra files you want to keep
           # around in the output directory.
           delete_contents(self.working_dir)
@@ -740,7 +740,7 @@ def assertIdentical(self, values, y, msg=None,
       print("Expected to have '%s' == '%s'" % (limit_size(values[0]), limit_size(y)))
     fail_message = 'Unexpected difference:\n' + limit_size(diff)
     if not EMTEST_VERBOSE:
-      fail_message += '\nFor full output run with EMTEST_VERBOSE=1.'
+      fail_message += '\nFor full output run with --verbose.'
     if msg:
       fail_message += '\n' + msg
     self.fail(fail_message)
@@ -760,9 +760,9 @@ def assertFileContents(self, filename, contents):
 
     if not os.path.exists(filename):
       self.fail('Test expectation file not found: ' + filename + '.\n' +
-                'Run with EMTEST_REBASELINE to generate.')
+                'Run with --rebaseline to generate.')
     expected_content = read_file(filename)
-    message = "Run with EMTEST_REBASELINE=1 to automatically update expectations"
+    message = "Run with --rebaseline to automatically update expectations"
     self.assertTextDataIdentical(expected_content, contents, message,
                                  filename, filename + '.new')
 

tests/runner.py

@@ -334,6 +334,15 @@ def configure():
   assert 'PARALLEL_SUITE_EMCC_CORES' not in os.environ, 'use EMTEST_CORES rather than PARALLEL_SUITE_EMCC_CORES'
   parallel_testsuite.NUM_CORES = os.environ.get('EMTEST_CORES') or os.environ.get('EMCC_CORES')
 
+  # Some options make sense being set in the environment, others not-so-much.
+  # TODO(sbc): eventually just make these command-line only.
+  if os.getenv('EMTEST_SAVE_DIR'):
+    print('Prefer --save-dir over setting $EMTEST_SAVE_DIR')
+  if os.getenv('EMTEST_REBASELINE'):
+    print('Prefer --rebaseline over setting $EMTEST_REBASELINE')
+  if os.getenv('EMTEST_VERBOSE'):
+    print('Prefer --verbose over setting $EMTEST_VERBOSE')
+
 
 def main(args):
   options = parse_args(args)

tests/test_browser.py

@@ -242,7 +242,7 @@ def test_zzz_html_source_map(self):
 If manually bisecting:
   Check that you see src.cpp among the page sources.
   Even better, add a breakpoint, e.g. on the printf, then reload, then step
-  through and see the print (best to run with EMTEST_SAVE_DIR=1 for the reload).
+  through and see the print (best to run with --save-dir for the reload).
 ''')
 
   def test_emscripten_log(self):

tests/test_other.py

@@ -8941,7 +8941,7 @@ def get_file_gzipped_size(f):
     # to rebaseline this test.  However:
     # a) such changes deserve extra scrutiny
     # b) such changes should be few and far between
-    # c) rebaselining is trivial (just run with EMTEST_REBASELINE=1)
+    # c) rebaselining is trivial (just run with --rebaseline)
     # Note that we do not compare the full wasm output since that is
     # even more fragile and can change with LLVM updates.
     if compare_js_output:
@@ -9014,9 +9014,11 @@ def get_file_gzipped_size(f):
       open(results_file, 'w').write(json.dumps(obtained_results, indent=2) + '\n')
     else:
       if total_output_size > total_expected_size:
-        print('Oops, overall generated code size regressed by ' + str(total_output_size - total_expected_size) + ' bytes!')
+        print('Oops, overall generated code size regressed by {total_output_size - total_expected_size} bytes!')
+        print('If this is expected, rerun the test with --rebaseline to update the expected sizes')
       if total_output_size < total_expected_size:
-        print('Hey amazing, overall generated code size was improved by ' + str(total_expected_size - total_output_size) + ' bytes! Rerun test with other.test_minimal_runtime_code_size with EMTEST_REBASELINE=1 to update the expected sizes!')
+        print(f'Hey amazing, overall generated code size was improved by {total_expected_size - total_output_size} bytes!')
+        print('If this is expected, rerun the test with --rebaseline to update the expected sizes')
       self.assertEqual(total_output_size, total_expected_size)
 
   # Tests the library_c_preprocessor.js functionality.
@@ -10569,7 +10571,7 @@ def test_split_main_module(self):
   def test_gen_struct_info(self):
     # This tests is fragile and will need updating any time any of the refereced
     # structs or defines change.   However its easy to rebaseline with
-    # EMTEST_REBASELINE and it prevents regressions or unintended changes
+    # --rebaseline and it prevents regressions or unintended changes
     # to the output json.
     self.run_process([PYTHON, path_from_root('tools/gen_struct_info.py'), '-o', 'out.json'])
     self.assertFileContents(test_file('reference_struct_info.json'), read_file('out.json'))

tests/third_party/bullet/Demos/HelloWorld/frames.html

@@ -4,7 +4,7 @@
 
 <!--
 
-EMTEST_SAVE_DIR=1 ./tests/runner.py asm3.test_the_bullet
+./tests/runner.py asm3.test_the_bullet --save-dir
 
 ./emcc -O3 tests/bullet/Demos/HelloWorld/HelloWorldFrames.cpp tests/bullet/Demos/HelloWorld/BenchmarkDemo.cpp /tmp/emscripten_temp/building/bullet/src/BulletDynamics/libBulletDynamics.a /tmp/emscripten_temp/building/bullet/src/BulletCollision/libBulletCollision.a /tmp/emscripten_temp/building/bullet/src/LinearMath/libLinearMath.a -o tests/bullet/Demos/HelloWorld/frames.js -I./tests/bullet/src -s TOTAL_MEMORY=60000000 -s LINKABLE=1
 

tools/scons/site_scons/site_tools/emscripten/emscripten.py

@@ -20,7 +20,7 @@ def generate(env, emscripten_path=None, **kw):
   # environment variabls from the parent calling process,
   # so manually route all environment variables referenced
   # by Emscripten to the child.
-  for var in ['EM_CACHE', 'EMCC_DEBUG', 'EMTEST_BROWSER',
+  for var in ['EM_CACHE', 'EMCC_DEBUG',
               'EMMAKEN_JUST_CONFIGURE', 'EMCC_CFLAGS', 'EMCC_TEMP_DIR',
               'EMCC_AUTODEBUG', 'EM_COMPILER_WRAPPER',
               'EMMAKEN_COMPILER', 'EMMAKEN_CFLAGS',
@@ -29,8 +29,7 @@ def generate(env, emscripten_path=None, **kw):
               'EMCC_JSOPT_MAX_CHUNK_SIZE', 'EMCC_SAVE_OPT_TEMP', 'EMCC_CORES', 'EMCC_NO_OPT_SORT',
               'EMCC_BUILD_DIR', 'EMCC_DEBUG_SAVE', 'EMCC_SKIP_SANITY_CHECK',
               'EMMAKEN_NO_SDK', 'EM_PKG_CONFIG_PATH', 'EMCC_CLOSURE_ARGS', 'JAVA_HEAP_SIZE',
-              'EMCC_FORCE_STDLIBS', 'EMCC_ONLY_FORCED_STDLIBS', 'EM_PORTS', 'IDL_CHECKS', 'IDL_VERBOSE',
-              'EMTEST_SAVE_DIR']:
+              'EMCC_FORCE_STDLIBS', 'EMCC_ONLY_FORCED_STDLIBS', 'EM_PORTS', 'IDL_CHECKS', 'IDL_VERBOSE']:
     if os.environ.get(var):
       env['ENV'][var] = os.environ.get(var)
   try: