From cb8fea286165ffd6ed91e5320f92b934facfbba5 Mon Sep 17 00:00:00 2001 From: Pascal Severin Date: Wed, 16 Sep 2026 16:39:23 +0200 Subject: [PATCH] [M1-DET-02] Fix macOS/Windows CI: replay test fopen C4996; exact tick count under frame budget 1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The M1-DET-02 merge (544a8a4) went red on the two non-Linux P0 jobs, both in the new tests/laige-sim/replay_record_tests.cpp: - Windows x64 (MSVC 2022): three plain std::fopen call sites (the RoundTripBytes and EncodingIsDeterministic raw-file reads) are C4996 (deprecated), fatal under the engine's /WX policy. Follow the logging_tests.cpp / src/laige-sim/replay.cpp precedent: a portable openReplayFile using _fsopen(path, mode, _SH_DENYNO) on MSVC (plain- fopen sharing semantics — fopen_s/_SH_SECURE would deny a same- process read-only re-open) and std::fopen elsewhere. - macOS arm64 + Intel: ReplayEngine.RecordsEmptyFramesPerTick ran the 8-tick bounded run with the default frame budget (kDefaultMaxCatchUp Ticks = 5). The run_headless contract (engine.h) allows the final count to overshoot by up to frameBudgetTicks - 1 when a late frame catches up several due ticks at once — on the macOS runners a paced sleep overshoots a 16.67 ms tick interval, so the run completed 9-10 ticks and the exact-count assertions (stats().ticks == 8, log.frames.size() == 8) failed, even though the frames-per-tick invariant itself held (9 frames for 9 ticks, 10 for 10). Run the same 8-tick bounded run with frame budget 1, the EngineRun.BoundedRunCompletesExactly precedent (engine_tests.cpp): each frame runs at most one tick, so the run lands exactly on 8 under any cadence (a late frame drops, it does not overshoot) and the exact assertions are contract-deterministic on every platform. Verified locally (g++ 16.2 Debug, the linux-gcc job's configuration): warning-free build, ctest 57/57, and the fixed test repeated 3x at exactly ticks=8 (133 ms each). The macOS job itself is verified by the PR's ci:macos label; the Windows job runs on the merge (its fix is the identical, already-Windows-verified _fsopen pattern). --- tests/laige-sim/replay_record_tests.cpp | 37 ++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/tests/laige-sim/replay_record_tests.cpp b/tests/laige-sim/replay_record_tests.cpp index 156d9c6..4d074f4 100644 --- a/tests/laige-sim/replay_record_tests.cpp +++ b/tests/laige-sim/replay_record_tests.cpp @@ -31,6 +31,10 @@ #include #include +#if defined(_MSC_VER) +#include // _SH_DENYNO: plain-fopen sharing for _fsopen +#endif + #include "gtest/gtest.h" #include "laige/errors.h" #include "laige/logging.h" @@ -164,6 +168,23 @@ std::string tempPath(const char* name) { return std::string(::testing::TempDir()) + name; } +// Portable file open (CPP-009 platform boundary, the +// logging_tests.cpp / src/laige-sim/replay.cpp precedent): MSVC's CRT +// deprecates plain `fopen` (C4996, fatal under the engine's /WX +// policy). As in replay.cpp, the MSVC path uses `_fsopen(path, mode, +// _SH_DENYNO)` — plain-`fopen` sharing semantics, so a read-only +// re-open of a file the recorder still holds succeeds (the secure +// `fopen_s` opens with `_SH_SECURE` and would deny it). +#if defined(_MSC_VER) +std::FILE* openReplayFile(const char* path, const char* mode) { + return ::_fsopen(path, mode, _SH_DENYNO); +} +#else +std::FILE* openReplayFile(const char* path, const char* mode) { + return std::fopen(path, mode); +} +#endif + // One log event captured from the facade (the engine_tests.cpp // MemorySink pattern — Warn+ only, rate limiting off). class MemorySink : public laige::log::Sink { @@ -243,7 +264,7 @@ TEST(ReplayFormat, RoundTripBytes) { { std::size_t size = static_cast(std::filesystem::file_size(path)); raw.resize(size); - std::FILE* f = std::fopen(path.c_str(), "rb"); + std::FILE* f = openReplayFile(path.c_str(), "rb"); ASSERT_NE(f, nullptr); ASSERT_EQ(std::fread(raw.data(), 1, raw.size(), f), raw.size()); std::fclose(f); @@ -308,8 +329,8 @@ TEST(ReplayFormat, EncodingIsDeterministic) { std::vector a, b; a.resize(static_cast(std::filesystem::file_size(pathA))); b.resize(static_cast(std::filesystem::file_size(pathB))); - std::FILE* fa = std::fopen(pathA.c_str(), "rb"); - std::FILE* fb = std::fopen(pathB.c_str(), "rb"); + std::FILE* fa = openReplayFile(pathA.c_str(), "rb"); + std::FILE* fb = openReplayFile(pathB.c_str(), "rb"); ASSERT_NE(fa, nullptr); ASSERT_NE(fb, nullptr); ASSERT_EQ(std::fread(a.data(), 1, a.size(), fa), a.size()); @@ -825,7 +846,15 @@ TEST(ReplayEngine, RecordsEmptyFramesPerTick) { EXPECT_TRUE(engine.replayRecordingActive()); EXPECT_EQ(engine.replayBytesWritten(), laige::kReplayHeaderSize); - ASSERT_TRUE(engine.run_headless(8, laige::kDefaultMaxCatchUpTicks).ok()); + // Frame budget 1 (the EngineRun.BoundedRunCompletesExactly + // precedent, engine_tests.cpp): each frame runs AT MOST one tick, so + // the run lands EXACTLY on 8 under any cadence — a late frame drops + // its extra due tick (the M1-LOOP-01 overload behavior), it does not + // overshoot. With a larger budget the run_headless contract (engine.h) + // allows the final count to run up to frameBudgetTicks - 1 over the + // target under overload, which breaks the exact-count assertions + // below on slow runners. + ASSERT_TRUE(engine.run_headless(8, 1).ok()); EXPECT_EQ(engine.stats().ticks, 8u); EXPECT_TRUE(engine.isShutDown());