From 4356e7c3fd8beb83c9c22e0ae55b0ac34df7705e Mon Sep 17 00:00:00 2001 From: jerrrrycho Date: Sun, 8 Nov 2026 09:36:28 +0100 Subject: [PATCH] fix(simulation): a test run that can't start ends with the error code and says why - `--test` is looked for before the rest of the command line is read. An unknown option before it (e.g. `-s x.scn --test-timout 90 --test`) used to end the run with exit code 0, which a caller reads as a pass. - `--test-timeout` without a value, or with one that isn't a number of seconds, fails the run instead of leaving the default or, through atof, turning the timeout off. - These, a missing -s, a failed start and an exception caught in main() end the test through test_manager::conclude(), which writes the usual `test: error (exit code 5) ... : ` line to stdout and the log. Before, `eu07 --test` alone exited with 5 and nothing anywhere: the error went to the log queue and the process ended before the log thread wrote it. In test mode main() now stops the log thread and waits for it before _Exit(). Outside the test mode nothing changes: conclude() does nothing there. --- EU07.cpp | 21 +++++++++++++++++++++ application/application.cpp | 34 ++++++++++++++++++++++++++++++---- simulation/simulationtest.cpp | 11 ++++++++--- simulation/simulationtest.h | 5 ++++- 4 files changed, 63 insertions(+), 8 deletions(-) diff --git a/EU07.cpp b/EU07.cpp index ec3e972e..790f6ffb 100644 --- a/EU07.cpp +++ b/EU07.cpp @@ -84,6 +84,20 @@ LONG WINAPI CrashHandler(EXCEPTION_POINTERS *ExceptionInfo) #endif +namespace +{ + +// the log is written by a thread of its own, which the closing of the application stops and waits for. on the ways +// out that skip the closing (a failed start, an exception) what was logged last would be lost with the process +void finish_log() +{ + StopLogService(); + if (auto const logservice{Global.threads.find("LogService")}; logservice != Global.threads.end() && logservice->second.joinable()) + logservice->second.join(); +} + +} // namespace + int main(int argc, char *argv[]) { #ifdef WITHDUMPGEN @@ -153,6 +167,7 @@ int main(int argc, char *argv[]) { 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())); if (false == Global.test.enabled) { std::string msg = "Simulator crash occured :(\n"; @@ -166,6 +181,12 @@ int main(int argc, char *argv[]) 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); diff --git a/application/application.cpp b/application/application.cpp index eaf60c7c..f8e20bed 100644 --- a/application/application.cpp +++ b/application/application.cpp @@ -30,8 +30,15 @@ http://mozilla.org/MPL/2.0/. #include "utilities/Timer.h" #include "utilities/dictionary.h" #include "version_info.h" +#include "simulation/simulationtest.h" +#include +#include #include +#include #include +#include +#include +#include #include "utilities/translation.h" #if WITH_DISCORD_RPC @@ -1174,6 +1181,16 @@ void apply_test_mode_settings() Global.random_seed = seed_of("test"); } +// seconds given on the command line: a number, not negative. returns nothing for anything else +std::optional parse_seconds(std::string_view const Text) +{ + auto const *const last{Text.data() + Text.size()}; + double value{0.0}; + if (auto const [parsed, status]{std::from_chars(Text.data(), last, value)}; status != std::errc{} || parsed != last || false == std::isfinite(value) || value < 0.0) + return std::nullopt; + return value; +} + } // namespace int eu07_application::init_settings(int Argc, char *Argv[]) @@ -1191,6 +1208,10 @@ int eu07_application::init_settings(int Argc, char *Argv[]) Global.LoadIniFile("eu07.ini"); } + // a test run is known before the rest of the command line is read, so a command line it can't take + // ends with the error code of the test, not with 0, which would pass it + Global.test.enabled = std::ranges::any_of(std::span(Argv, static_cast(Argc)), [](char const *Argument) { return std::string_view(Argument) == "--test"; }); + // process command line arguments bool sceneryfromcommandline{false}; bool editwithoutscenery{false}; @@ -1244,11 +1265,15 @@ int eu07_application::init_settings(int Argc, char *Argv[]) } else if (token == "--test-timeout") { - if (i + 1 < Argc) + auto const timeout{i + 1 < Argc ? parse_seconds(Argv[++i]) : std::nullopt}; + if (false == timeout.has_value()) { - Global.test.timeout = std::max(0.0, std::atof(Argv[++i])); - Global.test.timeout_from_command_line = true; + // a typo would otherwise turn the timeout off + simulation::test().conclude(simulation::test_result::error, "--test-timeout needs a number of seconds"); + return -1; } + Global.test.timeout = *timeout; + Global.test.timeout_from_command_line = true; } else if (token == "-ini") { @@ -1267,6 +1292,7 @@ int eu07_application::init_settings(int Argc, char *Argv[]) << " [-seed number|word]" << " [-ini settingsfile]" << " [--test [--test-timeout seconds]]" << std::endl; + simulation::test().conclude(simulation::test_result::error, std::format("unknown command line argument {}", token)); return -1; } } @@ -1277,7 +1303,7 @@ int eu07_application::init_settings(int Argc, char *Argv[]) { if (false == sceneryfromcommandline || Global.SceneryFile.empty()) { - ErrorLog("test: --test needs a scenery given with -s"); + simulation::test().conclude(simulation::test_result::error, "--test needs a scenery given with -s"); return -1; } apply_test_mode_settings(); diff --git a/simulation/simulationtest.cpp b/simulation/simulationtest.cpp index 45aa513f..214d7e46 100644 --- a/simulation/simulationtest.cpp +++ b/simulation/simulationtest.cpp @@ -156,9 +156,15 @@ void test_manager::signal_passed_at_danger(std::string_view const Vehicle, std:: } void test_manager::finish(test_result const Result, std::string_view const Reason) +{ + if (conclude(Result, Reason)) + Application.queue_quit(true); +} + +bool test_manager::conclude(test_result const Result, std::string_view const Reason) { if (m_finished || false == Global.test.enabled) - return; + return false; m_finished = true; m_result = Result; @@ -170,8 +176,7 @@ void test_manager::finish(test_result const Result, std::string_view const Reaso // the process has no console of its own; whoever started it with a redirected output gets the verdict there too std::fprintf(stdout, "%s\n", line.c_str()); std::fflush(stdout); - - Application.queue_quit(true); + return true; } } // namespace simulation diff --git a/simulation/simulationtest.h b/simulation/simulationtest.h index a58e9ae2..2b5012e9 100644 --- a/simulation/simulationtest.h +++ b/simulation/simulationtest.h @@ -46,8 +46,11 @@ public: void update(double Deltatime); // reported by a driver whose train went past a signal showing stop void signal_passed_at_danger(std::string_view Vehicle, std::string_view Signal); - // ends the test with given result; the first call wins + // ends the test with given result and closes the application; the first call wins void finish(test_result Result, std::string_view Reason); + // ends the test with given result, without closing the application: for the ways out that close it anyway + // (a bad command line, a failed start, an exception). returns false if the test had already ended + bool conclude(test_result Result, std::string_view Reason); bool finished() const { return m_finished; } // exit code of the process: the result, or error while the test hasn't finished