Allow empty_run! and reporter to display summary for empty runs

[zzak]

If we make it possible to print both messages, which fixes the rails test suite that are failing even after 5.22.2.

Running 0 tests in a single process (parallelization threshold is 50)
Run options: --seed 15821

# Running:



Finished in 0.000369s, 0.0000 runs/s, 0.0000 assertions/s.
0 runs, 0 assertions, 0 failures, 0 errors, 0 skips

For example when there are no tests run, we still don't get the default Rails reporter with number of runs, asserts, benchmark, etc.

# Running:

Running 0 tests in a single process (parallelization threshold is 50)
Run options: --seed 20717

This results in failing test cases, like:

Failure:
ApplicationTests::TestRunnerTest#test_system_tests_are_not_run_with_the_default_test_command [test/application/test_runner_test.rb:1193]:
Expected /0\ runs,\ 0\ assertions,\ 0\ failures,\ 0\ errors,\ 0\ skips/ to match "Running 0 tests in a single process (parallelization threshold is 50)\nRun options: --seed 20717\n\n# Running:\n\n".

See rails/rails#50978

[zenspider]

I'm starting to seriously regret this addition. Damn you, @tenderlove!

This PR does a few things differently:

1. Reformats for reasons I don't understand. All you wanted was to remove the return, no?
2. Feels like if it isn't going to be a shortcut, then maybe the ! should go?
3. Removes the non-zero exit code in cases where tests were found but nothing was ran. (which, again, I'm starting to regret... this might be a good thing to revert)

I would think that moving down as-is would be what you really want, no?

But also... Why change this for a rails test of questionable merit? Is it valuable that the test exist (or exist as-is)? Is it valuable that the summary line get printed if nothing is run? The empty_run! call makes it perfectly clear, no?

If somehow it IS a valuable test, this seems a sufficient change to me:

@@ -167,10 +167,10 @@
 
     # might have been removed/replaced during init_plugins:
     summary = reporter.reporters.grep(SummaryReporter).first
-    return empty_run! options if summary && summary.count == 0
 
     reporter.report
 
+    return empty_run! options if summary && summary.count == 0
     reporter.passed?
   end

[tenderlove]

I'm confused why we don't change the upstream Rails test? The test name is test_system_tests_are_not_run_with_the_default_test_command, surely there's a different way we can accomplish checking that. Why does 0\ runs,\ 0\ assertions,\ 0\ failures,\ 0\ errors,\ 0\ skips mean that we didn't use the default test command? I'm very confused

[zzak]

Tbh, I'm also confused so I opened this PR because it seemed the right way to ask what's going on.

I'm happy to follow your suggestions, but I wanted to investigate if there was a path to avoid breaking Rails tests, while also keeping the behavior you sought for.

Does the original change breaks the contract with reporter.report?
I'm wondering for things like JUnit that expects to dump some xml for each test run, but I have no concrete idea.

This PR also prompted me to try and add a similar thing to Rails, but this might not be the right approach, appreciate any feedback: rails/rails#51005

FWIW, here are the related test failures, I haven't looked that closely at them yet but I'd say a fair bit of them are like the assertion Aaron mentioned.

Test runner tests

.........F

Failure:
ApplicationTests::TestRunnerTest#test_more_than_one_line_filter_test_method_syntax [test/application/test_runner_test.rb:505]:
Expected /0\ runs,\ 0\ assertions/ to match "Running 3 tests in a single process (parallelization threshold is 50)\nRun options: --seed 42476\n\n# Running:\n\n".


bin/rails test test/application/test_runner_test.rb:469

..F

Failure:
ApplicationTests::TestRunnerTest#test_system_tests_are_not_run_with_the_default_test_command [test/application/test_runner_test.rb:1191]:
Expected /0\ runs,\ 0\ assertions,\ 0\ failures,\ 0\ errors,\ 0\ skips/ to match "Running 0 tests in a single process (parallelization threshold is 50)\nRun options: --seed 42930\n\n#
Running:\n\n".


bin/rails test test/application/test_runner_test.rb:1179

........................F

Failure:
ApplicationTests::TestRunnerTest#test_can_exclude_files_from_being_tested_via_rake_task_by_setting_DEFAULT_TEST_EXCLUDE_env_var [test/application/test_runner_test.rb:1238]:
Expected /0\ runs,\ 0\ assertions,\ 0\ failures,\ 0\ errors,\ 0\ skips/ to match "Running 0 tests in a single process (parallelization threshold is 50)\nRun options: --seed 35899\n\n#
Running:\n\n".


bin/rails test test/application/test_runner_test.rb:1234

F

Failure:
ApplicationTests::TestRunnerTest#test_can_exclude_files_from_being_tested_via_default_rails_command_by_setting_DEFAULT_TEST_EXCLUDE_env_var [test/application/test_runner_test.rb:1230]:
Expected /0\ runs,\ 0\ assertions,\ 0\ failures,\ 0\ errors,\ 0\ skips/ to match "Running 0 tests in a single process (parallelization threshold is 50)\nRun options: --seed 30780\n\n#
Running:\n\n".


bin/rails test test/application/test_runner_test.rb:1226


...................................F

Failure:
ApplicationTests::TestRunnerTest#test_more_than_one_line_filter_macro_syntax [test/application/test_runner_test.rb:415]:
Expected /0\ runs,\ 0\ assertions/ to match "Running 3 tests in a single process (parallelization threshold is 50)\nRun options: --seed 49588\n\n# Running:\n\n".


bin/rails test test/application/test_runner_test.rb:379

..F

Failure:
ApplicationTests::TestRunnerTest#test_run_app_without_rails_loaded [test/application/test_runner_test.rb:749]:
Expected /0\ runs,\ 0\ assertions/ to match "Running 0 tests in a single process (parallelization threshold is 50)\nRun options: --seed 26814\n\n# Running:\n\n".


bin/rails test test/application/test_runner_test.rb:741

.F

Failure:
ApplicationTests::TestRunnerTest#test_system_tests_are_not_run_through_rake_test [test/application/test_runner_test.rb:1207]:
Expected /0\ runs,\ 0\ assertions,\ 0\ failures,\ 0\ errors,\ 0\ skips/ to match "Running 0 tests in a single process (parallelization threshold is 50)\nRun options: --seed 35926\n\n#
Running:\n\n".


bin/rails test test/application/test_runner_test.rb:1195

Test command tests


E

Error:
Rails::Command::TestTest#test_test:*_with_name_option_skips_test:prepare_task:
RuntimeError: rails command failed (1): bin/rails test:models -n test_some_code 2>&1
Nothing ran for filter: test_some_code
Run options: -n test_some_code --seed 5763

# Running:


    test/commands/test_test.rb:95:in `run_test_command'
    test/commands/test_test.rb:89:in `block (2 levels) in <class:TestTest>'
    test/commands/test_test.rb:121:in `assert_skips_prepare_task'
    test/commands/test_test.rb:88:in `block in <class:TestTest>'


bin/rails test test/commands/test_test.rb:87

F

Failure:
Rails::Command::TestTest#test_test_command_with_path_arg_skips_test:prepare_task [test/commands/test_test.rb:19]:
Expected /0\ failures,\ 0\ errors/ to match "Run options: --seed 14030\n\n# Running:\n\n".

bin/rails test test/commands/test_test.rb:16

F

Failure:
Rails::Command::TestTest#test_test_command_with_no_args_runs_test:prepare_task [test/commands/test_test.rb:11]:
Expected /0\ failures,\ 0\ errors/ to match "Prepare yourself!\nRunning 0 tests in a single process (parallelization threshold is 50)\nRun options: --seed 53368\n\n# Running:\n\n".


bin/rails test test/commands/test_test.rb:10

E

Error:
Rails::Command::TestTest#test_test:all_with_name_option_skips_test:prepare_task:
RuntimeError: rails command failed (1): bin/rails test:all -n test_some_code 2>&1
Nothing ran for filter: test_some_code
Running 0 tests in a single process (parallelization threshold is 50)
Run options: -n test_some_code --seed 38475

# Running:


    test/commands/test_test.rb:95:in `run_test_command'
    test/commands/test_test.rb:77:in `block (2 levels) in <class:TestTest>'
    test/commands/test_test.rb:121:in `assert_skips_prepare_task'
    test/commands/test_test.rb:76:in `block in <class:TestTest>'


bin/rails test test/commands/test_test.rb:75

F

Failure:
Rails::Command::TestTest#test_test:*_runs_test:prepare_task [test/commands/test_test.rb:82]:
Expected /0\ failures,\ 0\ errors/ to match "Prepare yourself!\nRun options: --seed 65165\n\n# Running:\n\n".


bin/rails test test/commands/test_test.rb:81

F

Failure:
Rails::Command::TestTest#test_test_command_with_options_runs_test:prepare_task [test/commands/test_test.rb:37]:
Expected /0\ failures,\ 0\ errors/ to match "Prepare yourself!\nRunning 0 tests in a single process (parallelization threshold is 50)\nRun options: --seed 1234 -e development\n\n# Runn
ing:\n\n".


bin/rails test test/commands/test_test.rb:36

F

Failure:
Rails::Command::TestTest#test_test:all_runs_test:prepare_task [test/commands/test_test.rb:70]:
Expected /0\ failures,\ 0\ errors/ to match "Prepare yourself!\nRunning 0 tests in a single process (parallelization threshold is 50)\nRun options: --seed 29424\n\n# Running:\n\n".


bin/rails test test/commands/test_test.rb:69

F

Failure:
Rails::Command::TestTest#test_test_command_runs_successfully_when_no_tasks_defined [test/commands/test_test.rb:66]:
Expected /0\ failures,\ 0\ errors/ to match "Running 0 tests in a single process (parallelization threshold is 50)\nRun options: --seed 36784\n\n# Running:\n\n".


bin/rails test test/commands/test_test.rb:64

E

Error:
Rails::Command::TestTest#test_test_command_with_name_option_skips_test:prepare_task:
RuntimeError: rails command failed (1): bin/rails test -n test_some_code 2>&1
Nothing ran for filter: test_some_code
Running 0 tests in a single process (parallelization threshold is 50)
Run options: -n test_some_code --seed 25836

# Running:


    test/commands/test_test.rb:95:in `run_test_command'
    test/commands/test_test.rb:44:in `block (2 levels) in <class:TestTest>'
    test/commands/test_test.rb:121:in `assert_skips_prepare_task'
    test/commands/test_test.rb:43:in `block in <class:TestTest>'


bin/rails test test/commands/test_test.rb:42

[skipkayhil]

@@ -167,10 +167,10 @@
 
     # might have been removed/replaced during init_plugins:
     summary = reporter.reporters.grep(SummaryReporter).first
-    return empty_run! options if summary && summary.count == 0
 
     reporter.report
 
+    return empty_run! options if summary && summary.count == 0
     reporter.passed?
   end

I also think this diff is the thing we want. I would definitely find it surprising that a custom reporter no longer gets to run because the number of tests run happened to be 0.

---

Re: the tests in Rails, all of the failing tests in test_runner_test.rb are for the additional ways that Rails allows the test command to filter tests. So the assertions matching on the reporter output makes sense to me as a way to assert that tests were/weren't filtered successfully. As an alternative, matching on the lack of a report line feels much more brittle as many different things could break that cause the report to not print.

[rafaelfranca]

I tried to change the tests upstream but for some tests there is no other way to assert what we want to assert.

[tenderlove]

I also think this diff is the thing we want. I would definitely find it surprising that a custom reporter no longer gets to run because the number of tests run happened to be 0.

This makes sense. It's what I was missing. Custom reporters should get the message.

[zenspider]

Done! Sorry for all the confusion. I blame @tenderlove 😛

I'll get this out in a bit... still poking at #995

[zenspider]

done and released