fix(simulation): any exception ends a test run with error, other runs as on master

main() tells a test run (--test) apart before anything runs. Only a test run
catches every exception and installs a std::terminate handler, both ending the
run with the error code and a test: line; any other run keeps master's
exception handling, so on Linux an uncaught runtime_error aborts at the throw
site again.

A test run doesn't start the Discord RPC thread: it published the test
scenery as the player's presence and held the closing up to 5 s.

run.py drops a case scenery's .sbt cache after a run cut short (wall_timeout,
error, a crash, killed by run.py), which could have been writing it.

The -edit branch of the test command line parser had no effect (a token that
isn't an option is ignored either way) and is gone; its test checks what can
be seen instead.

EU07.cpp

@@ -23,7 +23,10 @@ Stele, firleju, szociu, hunter, ZiomalCl, OLI_EU and others
 #include "utilities/Globals.h"
 #include "utilities/Logs.h"
 #include <cstdlib>
-#include <format>
+#include <exception>
+#include <string_view>
+#include <thread>
+#include <vector>
 #ifdef WITHDUMPGEN
 #ifdef _WIN32
 #include <Windows.h>
@@ -94,10 +97,45 @@ namespace
 void finish_log()
 {
 	StopLogService();
-	if (auto const logservice{Global.threads.find("LogService")}; logservice != Global.threads.end() && logservice->second.joinable())
+	if (auto const logservice{Global.threads.find("LogService")};
+	    logservice != Global.threads.end() && logservice->second.joinable() && logservice->second.get_id() != std::this_thread::get_id())
 		logservice->second.join();
 }
 
+// ends a test run with its verdict on the standard output and in the exit code. a test the simulator didn't end
+// (a failed start, an exception) ends with the error and Reason
+[[noreturn]] void end_test_run(std::string_view const Reason)
+{
+	simulation::test().conclude(simulation::test_result::error, Reason);
+	finish_log();
+	fflush(stdout);
+	fflush(stderr);
+	// skip destructors, there are ordering errors which causes segfaults
+	std::_Exit(simulation::test().exit_code());
+}
+
+// a test run (--test): whatever stops it, it ends with its verdict. any other run never gets here and ends as
+// it always has
+[[noreturn]] void run_test(int argc, char *argv[])
+{
+	// for what doesn't reach the catch below: an exception leaving a noexcept function, or on Linux one leaving
+	// a thread (on Windows the handler is the main thread's only)
+	std::set_terminate([] { end_test_run(simulation::describe_exception(std::current_exception())); });
+	try
+	{
+		if (Application.init(argc, argv) == 0)
+		{
+			Application.run();
+			Application.exit();
+		}
+	}
+	catch (...)
+	{
+		end_test_run(simulation::describe_exception(std::current_exception()));
+	}
+	end_test_run("the simulator stopped before the test ended");
+}
+
 } // namespace
 
 int main(int argc, char *argv[])
@@ -149,6 +187,11 @@ int main(int argc, char *argv[])
 		fflush(stderr);
 		std::_Exit(result);
 	}
+	// unattended test run of a scenery, which says its result in the exit code (eu07/maszyna#541)
+	else if (simulation::parse_test_command_line(std::vector<char const *>(argv, argv + argc)).enabled)
+	{
+		run_test(argc, argv);
+	}
 	else
 	{
 		try
@@ -164,37 +207,18 @@ int main(int argc, char *argv[])
 		{
 			ErrorLog("Critical error, memory allocation failure: " + std::string(Error.what()));
 		}
+#ifdef _WIN32
 		catch (std::runtime_error const &Error)
 		{
-#ifndef _WIN32
-			// outside a test the error ends the program as it always did there, uncaught
-			if (false == Global.test.enabled)
-				throw;
-#endif
-			ErrorLog(std::format("Critical error: {}", Error.what()));
-			// in test mode nobody is there to close the box, the exit code tells of the error
-			simulation::test().conclude(simulation::test_result::error, std::format("critical error: {}", Error.what()));
-#ifdef _WIN32
-			if (false == Global.test.enabled)
-			{
-				std::string msg = "Simulator crash occured :(\n";
-				msg += Error.what();
-				MessageBoxA(nullptr, msg.c_str(), "Simulator crashed :(", MB_ICONERROR);
-			}
-#endif
+			std::string msg = "Simulator crash occured :(\n";
+			msg += Error.what();
+			MessageBoxA(nullptr, msg.c_str(), "Simulator crashed :(", MB_ICONERROR);
 		}
+#endif
 	}
 #ifndef _WIN32
 	fflush(stdout);
 	fflush(stderr);
 #endif
-	if (Global.test.enabled)
-	{
-		// a run that stopped before its test ended (a failed start, an exception) says so like any other end
-		simulation::test().conclude(simulation::test_result::error, "the simulator stopped before the test ended");
-		finish_log();
-	}
-	// skip destructors, there are ordering errors which causes segfaults
-	// a test run (--test) says its result in the exit code
-	std::_Exit(Global.test.enabled ? simulation::test().exit_code() : 0);
+	std::_Exit(0); // skip destructors, there are ordering errors which causes segfaults
 }

application/application.cpp

@@ -433,8 +433,12 @@ int eu07_application::init(int Argc, char *Argv[])
 
 	#if WITH_DISCORD_RPC
 	// Run DiscordRPC service
-	std::thread sDiscordRPC(&eu07_application::DiscordRPCService, this);
-	Global.threads.emplace("DiscordRPC", std::move(sDiscordRPC));
+	// not for a test run: it would show the test scenery as the player's presence, and hold up the closing
+	if (false == Global.test.enabled)
+	{
+		std::thread sDiscordRPC(&eu07_application::DiscordRPCService, this);
+		Global.threads.emplace("DiscordRPC", std::move(sDiscordRPC));
+	}
 	#endif
 
 	if (!init_network())

simulation/simulationtestrules.cpp

@@ -100,15 +100,28 @@ test_command_line parse_test_command_line(std::span<char const *const> const Arg
 		{
 			++i;
 		}
-		else if (option == "-edit" && i + 1 < Arguments.size() && Arguments[i + 1][0] != '-')
-		{
-			// the scenery to edit is optional
-			++i;
-		}
 	}
 	return result;
 }
 
+std::string describe_exception(std::exception_ptr const &Exception)
+{
+	if (nullptr == Exception)
+		return "the simulator was terminated";
+	try
+	{
+		std::rethrow_exception(Exception);
+	}
+	catch (std::exception const &Error)
+	{
+		return std::format("critical error: {}", Error.what());
+	}
+	catch (...)
+	{
+		return "critical error: an exception of unknown type";
+	}
+}
+
 double wall_clock_limit(std::optional<double> const Given, double const Simulationtimeout)
 {
 	if (Given.has_value())

simulation/simulationtestrules.h

@@ -9,6 +9,7 @@ http://mozilla.org/MPL/2.0/.
 
 #pragma once
 
+#include <exception>
 #include <optional>
 #include <span>
 #include <string>
@@ -53,6 +54,10 @@ std::optional<double> parse_seconds(std::string_view Text);
 // options are skipped, so a scenery named like an option isn't taken for one; options it doesn't know are left alone
 test_command_line parse_test_command_line(std::span<char const *const> Arguments);
 
+// what ended a test run that the simulator didn't end itself: the exception caught (or being handled when the
+// program was terminated), or nothing
+std::string describe_exception(std::exception_ptr const &Exception);
+
 // time allowed for loading the scenery, on top of the simulation time, when the real time limit isn't given [s]
 inline constexpr double wall_timeout_loading_allowance{600.0};
 

tests/scenarios/README.md

@@ -31,7 +31,8 @@ With `--test` the simulator:
   motion telemetry is off,
 - gives nothing to the outside either: no keyboard leds, LPT or PoKeys (`feedbackmode`), no
   messages to a multiplayer server (`multiplayer`), no program run at the end (`execonexit`),
-  no console window of its own (`showsystemconsole`), and the game's `imgui.ini` stays as it is,
+  no console window of its own (`showsystemconsole`), no Discord presence, and the game's `imgui.ini`
+  stays as it is,
 - keeps all of the above whatever the `config` block of the scenery says: the settings of the
   test mode are applied again after it,
 - runs the simulation as fast as it can in fixed steps of 0.03 s of simulation time per
@@ -39,8 +40,9 @@ With `--test` the simulator:
 - seeds its random numbers with a fixed seed unless `-seed` or `randomseed` gives one, so two
   runs of a scenery come out the same (the physicslogs of two runs are byte for byte equal),
 - quits when the test ends, and says how it ended in the exit code and in a `test: ...`
-  line on stdout and in the log. A command line it can't take, or a start that fails, ends
-  the same way with the `error` code. That line is all there is on stdout: the log goes to
+  line on stdout and in the log. A command line it can't take, a start that fails, or an
+  exception thrown by the simulator (of any type, on Linux too) ends the same way, with the
+  `error` code. That line is all there is on stdout: the log goes to
   the log file only (`debuglog` doesn't print it on the console in test mode).
 
 On Windows the simulator is a windowed program, which the `cmd` prompt and PowerShell don't wait
@@ -167,7 +169,8 @@ For each case the runner:
 The lead vehicle is named `autotest_<case>`, so every case has its own physicslog and log
 files and cases can run in parallel. The `autotest_*` sceneries (and the simulator's `.sbt`
 geometry cache of them) stay in the game's `scenery/` dir; the cache is dropped when the base
-scenery is newer.
+scenery is newer, and after a run cut short (`wall_timeout`, `error`, a crash, or killed by run.py),
+which could have been writing it.
 
 ## Writing a case
 

tests/scenarios/run.py

@@ -137,6 +137,11 @@ def fill_vars(fragment: str, values: dict, keep=frozenset()) -> str:
     return re.sub(r"\{\{(\w+)\}\}", value, fragment)
 
 
+def scenery_cache(game_dir: Path, case_name: str) -> Path:
+    """The simulator's geometry cache of the case scenery."""
+    return game_dir / "scenery" / f"autotest_{case_name}.sbt"
+
+
 def build_scenery(case: dict, game_dir: Path, case_name: str, vehicle: str) -> str:
     """Writes the case scenery into the game's scenery dir and returns its file name."""
     scenery_dir = game_dir / "scenery"
@@ -174,7 +179,7 @@ def build_scenery(case: dict, game_dir: Path, case_name: str, vehicle: str) -> s
     # the simulator caches the scenery's static geometry in <name>.sbt and reads it instead of
     # the terrain in the .scn (tracks, events and trainsets always come from the .scn), so
     # drop the cache once the base scenery has changed
-    cache = scenery_dir / f"autotest_{case_name}.sbt"
+    cache = scenery_cache(game_dir, case_name)
     if cache.exists() and cache.stat().st_mtime < (scenery_dir / case["base_scenery"]).stat().st_mtime:
         cache.unlink()
     return name
@@ -548,6 +553,11 @@ def run_case(name: str, exe: Path, game_dir: Path, timespeed: float, watch: bool
         watchdog = ", from the real time watchdog" if got == WALL_TIMEOUT else ""
         say(f"  {'PASS' if passed else 'FAIL'}  exit code {describe_exit(result.exit_code)}{watchdog}, expected {expect}")
         ok &= passed
+    # a run cut short (the real time watchdog, an error, a crash, or closed by us) could have been writing
+    # the scenery's geometry cache; the simulator checks only the header of it, so the next runs would read
+    # a truncated one. it is written again on the next run
+    if result.failure or (not watch and got in (None, "error", WALL_TIMEOUT)):
+        scenery_cache(game_dir, name).unlink(missing_ok=True)
     if result.failure:
         say(f"  FAIL  {result.failure}")
         ok = False

tests/unit/test_test_mode.cpp

@@ -4,6 +4,9 @@
 #include "simulation/simulationtestrules.h"
 
 #include <array>
+#include <exception>
+#include <new>
+#include <stdexcept>
 #include <vector>
 
 // the test mode of the simulator (eu07 --test, eu07/maszyna#541): its exit codes, command line and end conditions
@@ -117,10 +120,13 @@ TEST_CASE("test mode: the command line")
 		CHECK_FALSE(commandline.enabled);
 		CHECK(commandline.error.empty());
 	}
-	SUBCASE("the scenery of -edit is optional")
+	SUBCASE("-edit takes no option for its scenery, which is optional")
 	{
+		// a scenery after -edit or none: the option after it is read either way
 		CHECK(parse({"-edit", "--test", "-s", "td.scn"}).enabled);
-		CHECK_FALSE(parse({"-edit", "td.scn"}).enabled);
+		CHECK(parse({"-edit", "td.scn", "--test"}).enabled);
+		// -edit leaves -s alone, which takes --test for its scenery, as the simulator reads it
+		CHECK_FALSE(parse({"-edit", "-s", "--test"}).enabled);
 	}
 }
 
@@ -241,3 +247,22 @@ TEST_CASE("test mode: the end conditions")
 		CHECK(verdict->result == test_result::fail);
 	}
 }
+
+TEST_CASE("test mode: what ended a run the simulator didn't end")
+{
+	SUBCASE("an exception says what it was")
+	{
+		CHECK(simulation::describe_exception(std::make_exception_ptr(std::runtime_error("no scenery"))) == "critical error: no scenery");
+		// e.g. std::stof on a token of a scenery which isn't a number
+		CHECK(simulation::describe_exception(std::make_exception_ptr(std::invalid_argument("stof"))) == "critical error: stof");
+		CHECK(simulation::describe_exception(std::make_exception_ptr(std::bad_alloc())).starts_with("critical error: "));
+	}
+	SUBCASE("something thrown which isn't a std::exception")
+	{
+		CHECK(simulation::describe_exception(std::make_exception_ptr(42)) == "critical error: an exception of unknown type");
+	}
+	SUBCASE("terminated without an exception")
+	{
+		CHECK(simulation::describe_exception(nullptr) == "the simulator was terminated");
+	}
+}