From 1fea365cf1dde8754ce7cee1e253d70a4a189ab3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Tue, 11 Aug 2026 21:29:55 -0300 Subject: [PATCH] Fix presenter timezone tests on hosts with stale /tmp fixtures The presenter time/locale tests injected synthetic zone1970.tab data by writing a fixed shared path (/tmp/opencattus-test-zone1970.tab) and pointing production code at it through the OPENCATTUS_ZONE1970_TAB env var. On self-hosted runners littered with root-owned files from previous sudo podman jobs, the fixture's unchecked std::ofstream write silently failed and the tests read whatever stale content was left behind, failing three test cases with menus that did not match the scripted selections (run 31017554351). Follow the Connection::ScopedTestInterfaces pattern from 34faf4d: - Add Timezone::ScopedTestZone1970Tab (BUILD_TESTING only) so tests inject zone1970.tab lines in-process; an empty override still exercises the timedatectl fallback through the runner singleton. - Drop the OPENCATTUS_ZONE1970_TAB env var and temp-file fixture; production timezone discovery now always reads /usr/share/zoneinfo/zone1970.tab with no test backdoor. - initializePresenterTestEnvironment returns the RAII guard and all call sites bind it, so no filesystem or env state leaks across tests. Validated: almalinux:10 container preflight (1213/1213 assertions); bare RHEL 10.2 host with a stale root-owned immutable fixture file planted (274/274 test cases, 1235/1235 assertions; pre-fix code reproduces the exact three CI failures under the same conditions); AlmaLinux 10.1 + Confluent libvirt e2e lab (installer success, timezone applied, sinfo/NFS/MPI smoke green). --- include/opencattus/services/timezone.h | 33 +++++ src/services/timezone.cpp | 22 ++-- test/presenter_tui.cpp | 161 ++++++++++++++----------- 3 files changed, 134 insertions(+), 82 deletions(-) diff --git a/include/opencattus/services/timezone.h b/include/opencattus/services/timezone.h index bb11e80c..21b4e63c 100644 --- a/include/opencattus/services/timezone.h +++ b/include/opencattus/services/timezone.h @@ -8,7 +8,9 @@ #include #include +#include #include +#include #include /** @@ -70,6 +72,37 @@ class Timezone { */ std::multimap fetchAvailableTimezones(); +#ifdef BUILD_TESTING + // Test-only seam: when set, fetchAvailableTimezones() parses the override + // lines as the zone1970.tab contents instead of reading the host's + // /usr/share/zoneinfo/zone1970.tab. An empty override still exercises the + // timedatectl fallback through the runner singleton. Bare CI hosts carry + // real tzdata state that would otherwise leak into tests expecting + // synthetic timezones. Use Timezone::ScopedTestZone1970Tab for RAII + // teardown rather than poking this directly. + static std::optional> s_testZone1970Override; + + class ScopedTestZone1970Tab { + public: + explicit ScopedTestZone1970Tab(std::vector lines) + : m_previous(std::move(s_testZone1970Override)) + { + s_testZone1970Override = std::move(lines); + } + ScopedTestZone1970Tab(const ScopedTestZone1970Tab&) = delete; + ScopedTestZone1970Tab& operator=(const ScopedTestZone1970Tab&) = delete; + ScopedTestZone1970Tab(ScopedTestZone1970Tab&&) = delete; + ScopedTestZone1970Tab& operator=(ScopedTestZone1970Tab&&) = delete; + ~ScopedTestZone1970Tab() + { + s_testZone1970Override = std::move(m_previous); + } + + private: + std::optional> m_previous; + }; +#endif + void setTimezoneArea(std::string_view); std::string_view getTimezoneArea() const; diff --git a/src/services/timezone.cpp b/src/services/timezone.cpp index 9ad62fdf..6fe4fc1a 100644 --- a/src/services/timezone.cpp +++ b/src/services/timezone.cpp @@ -3,7 +3,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -#include #include #include #include @@ -20,7 +19,6 @@ using namespace opencattus; namespace { -constexpr std::string_view zone1970TabEnv = "OPENCATTUS_ZONE1970_TAB"; constexpr std::string_view zone1970TabPath = "/usr/share/zoneinfo/zone1970.tab"; constexpr std::string_view timedatectlTimezoneCommand = "timedatectl list-timezones --no-pager"; @@ -68,19 +66,15 @@ void insertTimezone( timezones.insert({ tz.substr(0, slash), tz.substr(slash + 1) }); } -std::filesystem::path systemZone1970TabPath() +std::vector readZone1970Tab() { - if (const auto* overridePath = std::getenv(zone1970TabEnv.data()); - overridePath != nullptr && std::string_view(overridePath).size() > 0) { - return overridePath; +#ifdef BUILD_TESTING + if (Timezone::s_testZone1970Override.has_value()) { + return parseZone1970Tab(*Timezone::s_testZone1970Override); } +#endif - return zone1970TabPath; -} - -std::vector readZone1970Tab() -{ - const auto path = systemZone1970TabPath(); + const std::filesystem::path path { zone1970TabPath }; std::ifstream file(path); if (!file.is_open()) { LOG_DEBUG( @@ -103,6 +97,10 @@ std::vector readZone1970Tab() } } +#ifdef BUILD_TESTING +std::optional> Timezone::s_testZone1970Override; +#endif + Timezone::Timezone() : m_availableTimezones { fetchAvailableTimezones() } { diff --git a/test/presenter_tui.cpp b/test/presenter_tui.cpp index 435334d0..60828af8 100644 --- a/test/presenter_tui.cpp +++ b/test/presenter_tui.cpp @@ -6,7 +6,6 @@ #include #include -#include #include #include #include @@ -38,6 +37,7 @@ #include #include #include +#include #include namespace { @@ -505,34 +505,25 @@ auto hasUsableInfinibandInterface() -> bool return !usableInfinibandInterfaces().empty(); } -constexpr std::string_view zone1970TabCommand - = R"(bash -c "test -r /usr/share/zoneinfo/zone1970.tab && cat /usr/share/zoneinfo/zone1970.tab || true")"; -constexpr std::string_view zone1970TabEnv = "OPENCATTUS_ZONE1970_TAB"; +// Synthetic zone1970.tab lines are injected in-process through +// Timezone::ScopedTestZone1970Tab instead of a shared temp file so tests +// never observe the host's tzdata (or another run's stale fixture). The +// Outputs entry under this key carries the injected lines. +constexpr std::string_view zone1970TabKey = "zone1970.tab"; constexpr std::string_view localeMetadataCommand = "locale -av"; constexpr std::string_view advancedLocaleChoice = "Advanced / legacy locales"; -void configureZone1970Fixture(const ScriptedRunner::Outputs& outputs) -{ - const auto path = std::filesystem::temp_directory_path() - / "opencattus-test-zone1970.tab"; - - std::ofstream file(path, std::ios::trunc); - const auto output = outputs.find(std::string(zone1970TabCommand)); - if (output != outputs.end()) { - for (const auto& line : output->second) { - file << line << '\n'; - } - } - file.close(); - - setenv(zone1970TabEnv.data(), path.c_str(), 1); -} - -void initializePresenterTestEnvironment( +[[nodiscard]] Timezone::ScopedTestZone1970Tab +initializePresenterTestEnvironment( ScriptedRunner::Outputs outputs = ScriptedRunner::Outputs { }, bool dryRun = false) { - configureZone1970Fixture(outputs); + std::vector zone1970Lines; + if (const auto entry = outputs.find(std::string(zone1970TabKey)); + entry != outputs.end()) { + zone1970Lines = std::move(entry->second); + outputs.erase(entry); + } opencattus::Singleton::init( std::make_unique(Options { @@ -542,12 +533,14 @@ void initializePresenterTestEnvironment( std::unique_ptr runner = std::make_unique(std::move(outputs)); opencattus::Singleton::init(std::move(runner)); + + return Timezone::ScopedTestZone1970Tab(std::move(zone1970Lines)); } auto defaultRunnerOutputs() -> ScriptedRunner::Outputs { return { - { std::string(zone1970TabCommand), + { std::string(zone1970TabKey), { "# country\tcoordinates\tTZ\tcomments", "BR\t-2332-04637\tAmerica/Sao_Paulo\tBrazil southeast", @@ -697,7 +690,8 @@ TEST_SUITE("opencattus::presenter::tui") { TEST_CASE("mail questionnaire stores postfix SASL settings on the model") { - initializePresenterTestEnvironment(defaultRunnerOutputs()); + const auto zone1970Fixture + = initializePresenterTestEnvironment(defaultRunnerOutputs()); auto model = std::make_unique(); seedClusterMetadata(*model); @@ -739,7 +733,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE("mail questionnaire keeps TLS certificate overrides optional") { - initializePresenterTestEnvironment(defaultRunnerOutputs()); + const auto zone1970Fixture + = initializePresenterTestEnvironment(defaultRunnerOutputs()); auto model = std::make_unique(); seedClusterMetadata(*model); @@ -766,8 +761,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE( "time questionnaire fails cleanly when no timezones are available") { - initializePresenterTestEnvironment({ - { std::string(zone1970TabCommand), { } }, + const auto zone1970Fixture = initializePresenterTestEnvironment({ + { std::string(zone1970TabKey), { } }, { "timedatectl list-timezones --no-pager", { } }, { "locale -a", { "en_US.utf8" } }, }); @@ -784,8 +779,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE( "locale questionnaire fails cleanly when no locales are available") { - initializePresenterTestEnvironment({ - { std::string(zone1970TabCommand), + const auto zone1970Fixture = initializePresenterTestEnvironment({ + { std::string(zone1970TabKey), { "BR\t-2332-04637\tAmerica/Sao_Paulo\tBrazil southeast", "FR\t+4852+00220\tEurope/Paris" } }, { "timedatectl list-timezones --no-pager", @@ -804,8 +799,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE("time questionnaire prefers canonical zone1970 timezones") { - initializePresenterTestEnvironment({ - { std::string(zone1970TabCommand), + const auto zone1970Fixture = initializePresenterTestEnvironment({ + { std::string(zone1970TabKey), { "# country\tcoordinates\tTZ\tcomments", "BR\t-2332-04637\tAmerica/Sao_Paulo\tBrazil southeast", @@ -837,8 +832,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE("time questionnaire falls back to timedatectl timezones") { - initializePresenterTestEnvironment({ - { std::string(zone1970TabCommand), { } }, + const auto zone1970Fixture = initializePresenterTestEnvironment({ + { std::string(zone1970TabKey), { } }, { "timedatectl list-timezones --no-pager", { "UTC", "America/Sao_Paulo" } }, { "locale -a", { "en_US.utf8" } }, @@ -860,8 +855,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE( "time questionnaire drills into nested timezone paths alphabetically") { - initializePresenterTestEnvironment({ - { std::string(zone1970TabCommand), + const auto zone1970Fixture = initializePresenterTestEnvironment({ + { std::string(zone1970TabKey), { "AR\t-3124-06411\tAmerica/Argentina/Cordoba", "BR\t-1259-03831\tAmerica/Bahia", @@ -896,8 +891,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE("locale questionnaire groups UTF-8 locales by language") { - initializePresenterTestEnvironment({ - { std::string(zone1970TabCommand), + const auto zone1970Fixture = initializePresenterTestEnvironment({ + { std::string(zone1970TabKey), { "BR\t-2332-04637\tAmerica/Sao_Paulo\tBrazil southeast" } }, { "timedatectl list-timezones --no-pager", { "America/Sao_Paulo" } }, @@ -940,8 +935,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE("locale questionnaire asks region when a language has choices") { - initializePresenterTestEnvironment({ - { std::string(zone1970TabCommand), + const auto zone1970Fixture = initializePresenterTestEnvironment({ + { std::string(zone1970TabKey), { "BR\t-2332-04637\tAmerica/Sao_Paulo\tBrazil southeast" } }, { "timedatectl list-timezones --no-pager", { "America/Sao_Paulo" } }, @@ -981,8 +976,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE("locale questionnaire keeps legacy locales behind advanced") { - initializePresenterTestEnvironment({ - { std::string(zone1970TabCommand), + const auto zone1970Fixture = initializePresenterTestEnvironment({ + { std::string(zone1970TabKey), { "BR\t-2332-04637\tAmerica/Sao_Paulo\tBrazil southeast" } }, { "timedatectl list-timezones --no-pager", { "America/Sao_Paulo" } }, @@ -1016,8 +1011,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE("locale questionnaire uses language metadata for unknown codes") { - initializePresenterTestEnvironment({ - { std::string(zone1970TabCommand), + const auto zone1970Fixture = initializePresenterTestEnvironment({ + { std::string(zone1970TabKey), { "BR\t-2332-04637\tAmerica/Sao_Paulo\tBrazil southeast" } }, { "timedatectl list-timezones --no-pager", { "America/Sao_Paulo" } }, @@ -1049,7 +1044,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE("dry-run ISO download skips the progress UI and keeps the " "planned image path") { - initializePresenterTestEnvironment(defaultRunnerOutputs(), true); + const auto zone1970Fixture + = initializePresenterTestEnvironment(defaultRunnerOutputs(), true); auto model = std::make_unique(); model->getHeadnode().setOS( @@ -1080,7 +1076,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE("ISO download choice is scheduled until preflight confirms") { - initializePresenterTestEnvironment(defaultRunnerOutputs()); + const auto zone1970Fixture + = initializePresenterTestEnvironment(defaultRunnerOutputs()); auto model = std::make_unique(); auto state = std::make_shared(); @@ -1104,7 +1101,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE("RHEL download choice retries instead of aborting the TUI") { - initializePresenterTestEnvironment(defaultRunnerOutputs(), true); + const auto zone1970Fixture + = initializePresenterTestEnvironment(defaultRunnerOutputs(), true); auto model = std::make_unique(); auto state = std::make_shared(); @@ -1132,7 +1130,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE("existing ISO path retries when the path is not a readable " "directory") { - initializePresenterTestEnvironment(defaultRunnerOutputs(), true); + const auto zone1970Fixture + = initializePresenterTestEnvironment(defaultRunnerOutputs(), true); const auto invalidPath = tempPath("opencattus-tui-iso-file", "txt"); std::ofstream(invalidPath).close(); @@ -1168,7 +1167,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE( "existing ISO path explains unmatched distro and retries directory") { - initializePresenterTestEnvironment(defaultRunnerOutputs(), true); + const auto zone1970Fixture + = initializePresenterTestEnvironment(defaultRunnerOutputs(), true); const auto emptyDir = createEmptyIsoDirectory("opencattus-tui-empty-iso-retry"); @@ -1208,7 +1208,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE("existing ISO path can switch to downloading after no match") { - initializePresenterTestEnvironment(defaultRunnerOutputs(), true); + const auto zone1970Fixture + = initializePresenterTestEnvironment(defaultRunnerOutputs(), true); const auto emptyDir = createEmptyIsoDirectory("opencattus-tui-empty-iso-download"); @@ -1238,7 +1239,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE("PBS queue questionnaire dumps PBS settings without SLURM " "placeholders") { - initializePresenterTestEnvironment(defaultRunnerOutputs(), true); + const auto zone1970Fixture + = initializePresenterTestEnvironment(defaultRunnerOutputs(), true); const auto outputPath = tempPath("opencattus-tui-pbs-answerfile", "ini"); @@ -1288,7 +1290,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE("compute questionnaire presenters populate the cluster model") { - initializePresenterTestEnvironment(defaultRunnerOutputs()); + const auto zone1970Fixture + = initializePresenterTestEnvironment(defaultRunnerOutputs()); auto model = std::make_unique(); model->getHeadnode().setOS( @@ -1400,7 +1403,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE( "repository questionnaire stores selected repositories on the model") { - initializePresenterTestEnvironment(defaultRunnerOutputs()); + const auto zone1970Fixture + = initializePresenterTestEnvironment(defaultRunnerOutputs()); auto model = std::make_unique(); const auto os @@ -1427,7 +1431,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE("repository questionnaire expands BeeGFS monitoring dependencies") { - initializePresenterTestEnvironment(defaultRunnerOutputs()); + const auto zone1970Fixture + = initializePresenterTestEnvironment(defaultRunnerOutputs()); auto model = std::make_unique(); const auto os @@ -1449,7 +1454,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE("repository questionnaire stop aborts the questionnaire") { - initializePresenterTestEnvironment(defaultRunnerOutputs()); + const auto zone1970Fixture + = initializePresenterTestEnvironment(defaultRunnerOutputs()); auto model = std::make_unique(); const auto os @@ -1469,7 +1475,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE("network questionnaires hide already consumed interfaces while " "keeping service and management sharing available") { - initializePresenterTestEnvironment(defaultRunnerOutputs()); + const auto zone1970Fixture + = initializePresenterTestEnvironment(defaultRunnerOutputs()); const auto interfaces = usableHostInterfaces(); if (interfaces.size() < 2) { @@ -1519,7 +1526,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE("service network questionnaire accepts an empty gateway") { - initializePresenterTestEnvironment(defaultRunnerOutputs()); + const auto zone1970Fixture + = initializePresenterTestEnvironment(defaultRunnerOutputs()); const auto interfaces = usableHostInterfaces(); if (interfaces.size() < 2) { @@ -1568,7 +1576,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE("network questionnaire rejects a gateway outside the selected " "subnet") { - initializePresenterTestEnvironment(defaultRunnerOutputs()); + const auto zone1970Fixture + = initializePresenterTestEnvironment(defaultRunnerOutputs()); const auto interfaces = usableHostInterfaces(); if (interfaces.size() < 2) { @@ -1620,7 +1629,8 @@ TEST_SUITE("opencattus::presenter::tui") "service network sharing management interface must use a separate " "subnet") { - initializePresenterTestEnvironment(defaultRunnerOutputs()); + const auto zone1970Fixture + = initializePresenterTestEnvironment(defaultRunnerOutputs()); const auto interfaces = usableHostInterfaces(); if (interfaces.empty()) { @@ -1676,7 +1686,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE("management and service network questionnaires reuse known " "defaults") { - initializePresenterTestEnvironment(defaultRunnerOutputs()); + const auto zone1970Fixture + = initializePresenterTestEnvironment(defaultRunnerOutputs()); const auto interfaces = usableHostInterfaces(); if (interfaces.size() < 2) { @@ -1718,7 +1729,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE("infiniband questionnaire persists an explicit OFED version") { - initializePresenterTestEnvironment(defaultRunnerOutputs()); + const auto zone1970Fixture + = initializePresenterTestEnvironment(defaultRunnerOutputs()); const auto interfaces = usableInfinibandInterfaces(); if (interfaces.empty()) { @@ -1751,7 +1763,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE("compute node entry retries instead of throwing on invalid node " "definitions") { - initializePresenterTestEnvironment(defaultRunnerOutputs()); + const auto zone1970Fixture + = initializePresenterTestEnvironment(defaultRunnerOutputs()); auto model = std::make_unique(); addHeadnodeNetwork(*model, Network::Profile::Management, "eno2", @@ -1782,7 +1795,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE("compute node questionnaire leaves BMC pattern blank without a " "service network") { - initializePresenterTestEnvironment(defaultRunnerOutputs()); + const auto zone1970Fixture + = initializePresenterTestEnvironment(defaultRunnerOutputs()); auto model = std::make_unique(); addHeadnodeNetwork(*model, Network::Profile::Management, "eno2", @@ -1814,7 +1828,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE("compute node questionnaire accepts nodes without BMC addresses") { - initializePresenterTestEnvironment(defaultRunnerOutputs()); + const auto zone1970Fixture + = initializePresenterTestEnvironment(defaultRunnerOutputs()); auto model = std::make_unique(); addHeadnodeNetwork(*model, Network::Profile::Management, "eno2", @@ -1842,7 +1857,8 @@ TEST_SUITE("opencattus::presenter::tui") "compute node questionnaire rejects BMC addresses matching compute " "node addresses") { - initializePresenterTestEnvironment(defaultRunnerOutputs()); + const auto zone1970Fixture + = initializePresenterTestEnvironment(defaultRunnerOutputs()); auto model = std::make_unique(); addHeadnodeNetwork(*model, Network::Profile::Management, "eno2", @@ -1885,7 +1901,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE("compute node questionnaire suggests node and BMC IP patterns") { - initializePresenterTestEnvironment(defaultRunnerOutputs()); + const auto zone1970Fixture + = initializePresenterTestEnvironment(defaultRunnerOutputs()); auto model = std::make_unique(); addHeadnodeNetwork(*model, Network::Profile::Management, "eno2", @@ -1938,7 +1955,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE("presenter install can drive the questionnaire end to end") { - initializePresenterTestEnvironment(defaultRunnerOutputs()); + const auto zone1970Fixture + = initializePresenterTestEnvironment(defaultRunnerOutputs()); const auto interfaces = usableHostInterfaces(); if (interfaces.size() < 2) { @@ -2180,7 +2198,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE("presenter install propagates aborts from network prompts") { - initializePresenterTestEnvironment(defaultRunnerOutputs()); + const auto zone1970Fixture + = initializePresenterTestEnvironment(defaultRunnerOutputs()); // Inject two synthetic interfaces so PresenterNetwork does not bail // out via fatalMessage("Not enough interfaces!") on single-NIC hosts @@ -2234,7 +2253,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE("presenter install can drive the questionnaire end to end on " "dry-run") { - initializePresenterTestEnvironment(defaultRunnerOutputs(), true); + const auto zone1970Fixture + = initializePresenterTestEnvironment(defaultRunnerOutputs(), true); const auto interfaces = usableHostInterfaces(); if (interfaces.size() < 2) { @@ -2318,7 +2338,8 @@ TEST_SUITE("opencattus::presenter::tui") TEST_CASE("presenter install rewinds to the previous step when Back is " "requested") { - initializePresenterTestEnvironment(defaultRunnerOutputs()); + const auto zone1970Fixture + = initializePresenterTestEnvironment(defaultRunnerOutputs()); const auto interfaces = usableHostInterfaces(); if (interfaces.size() < 2) {