Benchmark output is now hidden

[kripken]

I think this is due to the recent multithreaded test harness improvements? It seems to suppress stdout, which is where we print the sizes and times, so the benchmarking data is never printed anywhere. For example:

$ EMTEST_BENCHMARKERS=v8 ./test/runner benchmark.test_primes benchmark.test_corrections --save-dir --no-clean
Running test_benchmark: (2 tests)
[1/2] test_corrections (test_benchmark.benchmark.test_corrections) ... 

It stays like that while running the first benchmark. When it gets to start the second:

Running test_benchmark: (2 tests)
[2/2] test_primes (test_benchmark.benchmark.test_primes) ... 

Note that nothing was shown for the first benchmark's output, we just update from 1/2 to 2/2 in place, and update the benchmark name. And even after finishing, nothing is printed:

$ EMTEST_BENCHMARKERS=v8 ./test/runner benchmark.test_primes benchmark.test_corrections --save-dir --no-clean
Running test_benchmark: (2 tests)
[2/2] test_primes (test_benchmark.benchmark.test_primes) ... ok

----------------------------------------------------------------------
Ran 2 tests in 80.258s

OK

[sbc100]

You can use -v/--verbose to show the output from the test runs.

We should probably detect the benchmark suite and default to level 1 verbosity at least.

[kripken]

Makes sense. Do we have a mechanism to do that? This works for example but I'm not sure I like it...

diff --git a/test/runner.py b/test/runner.py
index 383f12694..9a69497e6 100755
--- a/test/runner.py
+++ b/test/runner.py
@@ -136,6 +136,13 @@ def get_and_import_modules():
   return modules
 
 
+def update_options_for_modules(modules, options):
+  # If a module requires verbose logging, enable that.
+  for module in modules:
+    if getattr(module, 'verbose_logging', 0):
+      options.verbose = max(options.verbose, 1)
+
+
 def get_all_tests(modules):
   # Create a list of all known tests so that we can choose from them based on a wildcard search
   all_tests = []
@@ -714,6 +723,7 @@ def main():
   tests = [prepend_default(t) for t in options.tests]
 
   modules = get_and_import_modules()
+  update_options_for_modules(modules, options)
   all_tests = get_all_tests(modules)
   if options.crossplatform_only:
     tests = get_crossplatform_tests(modules)
diff --git a/test/test_benchmark.py b/test/test_benchmark.py
index 6e641b350..50f47d7c6 100644
--- a/test/test_benchmark.py
+++ b/test/test_benchmark.py
@@ -56,6 +56,8 @@ LLVM_FEATURE_FLAGS = ['-mnontrapping-fptoint']
 # `named_benchmarkers` for what is available.
 EMTEST_BENCHMARKERS = os.getenv('EMTEST_BENCHMARKERS', 'clang,v8,v8-lto,v8-ctors')
 
+verbose_logging = True
+
 
 class Benchmarker:
   # Whether to record statistics. Set by SizeBenchmarker.
`

[sbc100]

I'm not sure its worth. As long as we all know that -v is needed to see the test stdout it seems easy enough to add.

[kripken]

I worry about other random people that try it, though - it looks broken at first try, with no obvious hint as to what could fix it...

[sbc100]

But random people won't know to expect any output will they?

Honestly, I think only the more advanced constributors run these tests. I for one only run them about once or twice a year. Most contributors are likely not concerned with test_benchmark.py.

In fact, every time I've ever run anything from there I've had to hand edit to the code to do an A/B comparison two different configurations since the set of hardcoded configurations is almost always not what one wants (at least in my experience).

I guess I don't minds adding something like your patch above (but we don't even need to be that clever about it we can just do if suite == 'test_benchmark' or something dumb like that. See how we already have an explicit list of test suites that don't support running in parallel?

[kripken]

In fact, every time I've ever run anything from there I've had to hand edit to the code to do an A/B comparison

It is a lot more useful now, after @brendandahl 's changes that let you pick the benchmark runners with an env var. A single line, no python editing, gives me useful data when I need it.

I guess I don't minds adding something like your patch above (but we don't even need to be that clever about it we can just do if suite == 'test_benchmark' or something dumb like that.

sg, I'll make a simpler patch and open a PR

[sbc100]

It is a lot more useful now, after [@brendandahl](https://github.com/brendandahl) 's changes that let you pick the benchmark runners with an env var. A single line, no python editing, gives me useful data when I need it.

Oh.. I've not seen that. Will check it out when I next run them,