DefaultLauncher should catch exceptions thrown by test engines

[jlink]

Overview

If a test engine throws an exception during discovery (see https://github.com/junit-team/junit5/blob/master/junit-platform-launcher/src/main/java/org/junit/platform/launcher/core/DefaultLauncher.java#L115) other engines won't be queried any more.

I think this is a bug because one test engine can thereby prevent others from working.

Deliverables

• DefaultLauncher should catch and report exceptions from engines and proceed with other engines.
  • This applies to both the discovery phase and execution phase.

Additional Proposals

• DefaultLauncher could even collect discovery results concurrently.

[sbrannen]

Thanks for raising the issue.

I totally agree!

The same applies to the execution phase as well.

[sbrannen]

DefaultLauncher could even collect discovery results concurrently.

It could do that, but I consider that a separate topic. I have therefore updated this issue's description to reflect the deliverables.

Feel free to create a new issue to propose concurrent test discovery across registered engines.

[sbrannen]

Slated for M5 since we are trying to get M4 out the door as soon as possible.

[sormuras]

With #792 merged, an erroneous engine implementation doesn't any longer break the test plan execution.

We should decide if a "simple warning" is enough for the end-user to take note of the failed engine. Perhaps the execution summary needs an update, like:

Test run finished after 3091 ms
[         2 engines found         ]
[         1 engines successful    ]
[         1 engines aborted       ]
[         0 containers skipped    ]
[        70 containers started    ]
[         0 containers aborted    ]
[        70 containers successful ]
[         0 containers failed     ]
[       602 tests found           ]
[         0 tests skipped         ]
[       602 tests started         ]
[         1 tests aborted         ]
[       601 tests successful      ]
[         0 tests failed          ]

Updated issue description accordingly.

[sbrannen]

We should decide if a "simple warning" is enough for the end-user to take note of the failed engine. Perhaps the execution summary needs an update, like:

I think we should go ahead and close this issue and address the topic of exception handling vs. logging in conjunction with #242 (thereby broadening the scope of #242 to cover the Platform as well).

[marcphilipp]

👍

[sbrannen]

I have moved the second deliverable to #242 and am therefore closing this issue.