Skip to content

Commit 88b0f4f

Browse files
Jgocunhaclaude
andcommitted
fix: resolve CI build failure and address review feedback
- Fix std::pair CTAD failure that broke Linux/macOS builds: a raw function name deduces to a function type (not a pointer), which std::pair cannot hold. Reuse the existing NamedApplier struct instead. - Make TempIniFile's constructor/destructor non-throwing (error_code overload of fs::remove) to avoid std::terminate on cleanup failure - Drop unnecessary fs::remove(defaultWindowParametersFilename) calls; every test uses explicit positive dimensions and its own load(), so the shared default INI is never actually read - Extract the duplicated colorsEqual helper into test_helpers.h - Use TempIniFile's RAII cleanup instead of manual fs::remove in the three tests that still did it by hand Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 75a75f7 commit 88b0f4f

6 files changed

Lines changed: 37 additions & 60 deletions

File tree

imgui-platform-kit/tests/test_helpers.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,14 @@
1414

1515
namespace imgui_kit::testing
1616
{
17+
// Exact (non-approximate) ImVec4 comparison — the values under test are
18+
// always literal source constants or copies of them, so bit-for-bit
19+
// equality is expected and any divergence is a real regression.
20+
inline bool colorsEqual(const ImVec4& a, const ImVec4& b)
21+
{
22+
return a.x == b.x && a.y == b.y && a.z == b.z && a.w == b.w;
23+
}
24+
1725
class ImGuiContextFixture
1826
{
1927
public:

imgui-platform-kit/tests/test_parameters_edge_cases.cpp

Lines changed: 19 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,17 @@ namespace
2121
explicit TempIniFile(const std::string& name, const std::string& content)
2222
: path((fs::temp_directory_path() / name).string())
2323
{
24-
fs::remove(path);
24+
std::error_code ec;
25+
fs::remove(path, ec);
2526
std::ofstream out(path);
2627
out << content;
2728
}
2829

29-
~TempIniFile() { fs::remove(path); }
30+
~TempIniFile()
31+
{
32+
std::error_code ec;
33+
fs::remove(path, ec);
34+
}
3035

3136
TempIniFile(const TempIniFile&) = delete;
3237
TempIniFile& operator=(const TempIniFile&) = delete;
@@ -45,39 +50,34 @@ TEST_CASE("WindowParameters: load throws std::invalid_argument on malformed Widt
4550
// Pins current behavior: load() does not validate numeric fields; a
4651
// corrupt INI throws a raw std::invalid_argument out of std::stoi.
4752
TempIniFile ini("ipk_edge_bad_width.ini", "[Window Parameters]\nWidth=abc\n");
48-
fs::remove(defaultWindowParametersFilename);
4953
WindowParameters p("App", 800, 600);
5054
CHECK_THROWS_AS(p.load(ini.str()), std::invalid_argument);
5155
}
5256

5357
TEST_CASE("WindowParameters: load throws std::invalid_argument on malformed Height", "[parameters][window][io][edge]")
5458
{
5559
TempIniFile ini("ipk_edge_bad_height.ini", "[Window Parameters]\nHeight=xyz\n");
56-
fs::remove(defaultWindowParametersFilename);
5760
WindowParameters p("App", 800, 600);
5861
CHECK_THROWS_AS(p.load(ini.str()), std::invalid_argument);
5962
}
6063

6164
TEST_CASE("WindowParameters: load throws std::invalid_argument on malformed StartPosX", "[parameters][window][io][edge]")
6265
{
6366
TempIniFile ini("ipk_edge_bad_startposx.ini", "[Window Parameters]\nStartPosX=notanumber\n");
64-
fs::remove(defaultWindowParametersFilename);
6567
WindowParameters p("App", 800, 600);
6668
CHECK_THROWS_AS(p.load(ini.str()), std::invalid_argument);
6769
}
6870

6971
TEST_CASE("WindowParameters: load throws std::invalid_argument on malformed StartPosY", "[parameters][window][io][edge]")
7072
{
7173
TempIniFile ini("ipk_edge_bad_startposy.ini", "[Window Parameters]\nStartPosY=??\n");
72-
fs::remove(defaultWindowParametersFilename);
7374
WindowParameters p("App", 800, 600);
7475
CHECK_THROWS_AS(p.load(ini.str()), std::invalid_argument);
7576
}
7677

7778
TEST_CASE("WindowParameters: load throws std::out_of_range on an overflowing value", "[parameters][window][io][edge]")
7879
{
7980
TempIniFile ini("ipk_edge_overflow.ini", "[Window Parameters]\nWidth=99999999999999999999\n");
80-
fs::remove(defaultWindowParametersFilename);
8181
WindowParameters p("App", 800, 600);
8282
CHECK_THROWS_AS(p.load(ini.str()), std::out_of_range);
8383
}
@@ -87,7 +87,6 @@ TEST_CASE("WindowParameters: load throws std::out_of_range on an overflowing val
8787
TEST_CASE("WindowParameters: partial file only updates the keys present", "[parameters][window][io][edge]")
8888
{
8989
TempIniFile ini("ipk_edge_partial.ini", "[Window Parameters]\nTitle=OnlyTitle\n");
90-
fs::remove(defaultWindowParametersFilename);
9190
WindowParameters p("Original", 1280, 720, 5, 6);
9291

9392
p.load(ini.str());
@@ -103,7 +102,6 @@ TEST_CASE("WindowParameters: unknown keys are ignored", "[parameters][window][io
103102
{
104103
TempIniFile ini("ipk_edge_unknown_keys.ini",
105104
"[Window Parameters]\nNonsense=42\nDepth=3\nTitle=KnownTitle\n");
106-
fs::remove(defaultWindowParametersFilename);
107105
WindowParameters p("Original", 1280, 720);
108106

109107
REQUIRE_NOTHROW(p.load(ini.str()));
@@ -115,7 +113,6 @@ TEST_CASE("WindowParameters: unknown keys are ignored", "[parameters][window][io
115113
TEST_CASE("WindowParameters: empty file is a no-op on all fields", "[parameters][window][io][edge]")
116114
{
117115
TempIniFile ini("ipk_edge_empty.ini", "");
118-
fs::remove(defaultWindowParametersFilename);
119116
WindowParameters p("Original", 1280, 720, 5, 6);
120117

121118
p.load(ini.str());
@@ -130,7 +127,6 @@ TEST_CASE("WindowParameters: empty file is a no-op on all fields", "[parameters]
130127
TEST_CASE("WindowParameters: the [Window Parameters] header line is ignored", "[parameters][window][io][edge]")
131128
{
132129
TempIniFile ini("ipk_edge_header_only.ini", "[Window Parameters]\n");
133-
fs::remove(defaultWindowParametersFilename);
134130
WindowParameters p("Original", 1280, 720, 5, 6);
135131

136132
REQUIRE_NOTHROW(p.load(ini.str()));
@@ -142,8 +138,8 @@ TEST_CASE("WindowParameters: the [Window Parameters] header line is ignored", "[
142138

143139
TEST_CASE("WindowParameters: title containing spaces and '=' round-trips exactly", "[parameters][window][io][edge]")
144140
{
145-
const std::string path = (fs::temp_directory_path() / "ipk_edge_weird_title.ini").string();
146-
fs::remove(path);
141+
TempIniFile ini("ipk_edge_weird_title.ini", "");
142+
const std::string& path = ini.str();
147143

148144
const WindowParameters saved("My = Weird == Title", 1280, 720, 1, 2);
149145
saved.save(path);
@@ -152,8 +148,6 @@ TEST_CASE("WindowParameters: title containing spaces and '=' round-trips exactly
152148
loaded.load(path);
153149

154150
CHECK(loaded.title == "My = Weird == Title");
155-
156-
fs::remove(path);
157151
}
158152

159153
// ─── negative/zero values bypass validation on load() ──────────────────────────
@@ -163,7 +157,6 @@ TEST_CASE("WindowParameters: load stores negative values without validation", "[
163157
// The width/height > 0 validation only runs in the constructor's
164158
// monitor-fallback logic, not in load() itself — load() bypasses it.
165159
TempIniFile ini("ipk_edge_negative.ini", "[Window Parameters]\nWidth=-500\nHeight=-1\n");
166-
fs::remove(defaultWindowParametersFilename);
167160
WindowParameters p("App", 800, 600);
168161

169162
REQUIRE_NOTHROW(p.load(ini.str()));
@@ -181,32 +174,25 @@ static std::string readFile(const std::string& path)
181174

182175
TEST_CASE("WindowParameters: save() is deterministic and writes exactly 6 lines", "[parameters][window][io][edge]")
183176
{
184-
const std::string pathA = (fs::temp_directory_path() / "ipk_edge_det_a.ini").string();
185-
const std::string pathB = (fs::temp_directory_path() / "ipk_edge_det_b.ini").string();
186-
fs::remove(pathA);
187-
fs::remove(pathB);
177+
TempIniFile iniA("ipk_edge_det_a.ini", "");
178+
TempIniFile iniB("ipk_edge_det_b.ini", "");
188179

189-
fs::remove(defaultWindowParametersFilename);
190180
const WindowParameters p("Deterministic", 1024, 768, 3, 4);
191-
p.save(pathA);
192-
p.save(pathB);
181+
p.save(iniA.str());
182+
p.save(iniB.str());
193183

194-
const std::string contentA = readFile(pathA);
195-
const std::string contentB = readFile(pathB);
184+
const std::string contentA = readFile(iniA.str());
185+
const std::string contentB = readFile(iniB.str());
196186
CHECK(contentA == contentB);
197187

198188
const auto lineCount = std::count(contentA.begin(), contentA.end(), '\n');
199189
CHECK(lineCount == 6);
200-
201-
fs::remove(pathA);
202-
fs::remove(pathB);
203190
}
204191

205192
// ─── save() throws with message on unopenable path ────────────────────────────
206193

207194
TEST_CASE("WindowParameters: save() throws with a descriptive message on an unopenable path", "[parameters][window][io][edge]")
208195
{
209-
fs::remove(defaultWindowParametersFilename);
210196
const WindowParameters p("App", 800, 600);
211197
CHECK_THROWS_WITH(
212198
p.save("this/directory/does/not/exist/file.ini"),
@@ -217,16 +203,13 @@ TEST_CASE("WindowParameters: save() throws with a descriptive message on an unop
217203

218204
TEST_CASE("UserInterfaceParameters: save() with a custom filename delegates to WindowParameters::save()", "[parameters][io][edge]")
219205
{
220-
const std::string path = (fs::temp_directory_path() / "ipk_edge_uiparams.ini").string();
221-
fs::remove(path);
206+
TempIniFile ini("ipk_edge_uiparams.ini", "");
222207

223208
const UserInterfaceParameters params(WindowParameters("Delegated", 800, 600, 0, 0));
224-
params.save(path);
209+
params.save(ini.str());
225210

226-
REQUIRE(fs::exists(path));
227-
const std::string content = readFile(path);
211+
REQUIRE(fs::exists(ini.str()));
212+
const std::string content = readFile(ini.str());
228213
CHECK(content.rfind("[Window Parameters]", 0) == 0);
229214
CHECK(content.find("Title=Delegated") != std::string::npos);
230-
231-
fs::remove(path);
232215
}

imgui-platform-kit/tests/test_style_parameters_apply.cpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,6 @@ static const std::vector<Theme> ALL_THEMES = {
2020
Theme::LedSynthmaster,
2121
};
2222

23-
static bool colorsEqual(const ImVec4& a, const ImVec4& b)
24-
{
25-
return a.x == b.x && a.y == b.y && a.z == b.z && a.w == b.w;
26-
}
27-
2823
// ─── apply() dispatches to the matching themes[] entry ───────────────────────
2924

3025
TEST_CASE("StyleParameters: apply() applies the selected theme for every Theme value", "[parameters][apply]")

imgui-platform-kit/tests/test_template_window.cpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,6 @@ namespace
2020
return std::make_unique<TemplateWindow>();
2121
}
2222

23-
bool colorsEqual(const ImVec4& a, const ImVec4& b)
24-
{
25-
return a.x == b.x && a.y == b.y && a.z == b.z && a.w == b.w;
26-
}
27-
2823
// template_window.cpp unconditionally calls ImPlot::ShowDemoWindow(), so
2924
// every test needs a live ImPlot context alongside the ImGui one. Created
3025
// after the ImGui context, destroyed before it dies.

imgui-platform-kit/tests/test_theme_selector.cpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,6 @@ using namespace imgui_kit::testing;
1313
// therefore cover every line reachable without that interaction: the window
1414
// wrapper, the combo header, and the closed-combo path.
1515

16-
static bool colorsEqual(const ImVec4& a, const ImVec4& b)
17-
{
18-
return a.x == b.x && a.y == b.y && a.z == b.z && a.w == b.w;
19-
}
20-
2116
TEST_CASE("ThemeSelector: renders with nullptr open flag", "[themeselector][render]")
2217
{
2318
ImGuiFrameFixture fixture;

imgui-platform-kit/tests/test_themes_apply.cpp

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,6 @@ static const std::vector<NamedApplier> ALL_APPLIERS = {
5959

6060
static constexpr int EXPECTED_APPLIER_COUNT = 36;
6161

62-
static bool colorsEqual(const ImVec4& a, const ImVec4& b)
63-
{
64-
return a.x == b.x && a.y == b.y && a.z == b.z && a.w == b.w;
65-
}
66-
6762
static bool styleEqual(const ImGuiStyle& a, const ImGuiStyle& b)
6863
{
6964
for (int i = 0; i < ImGuiCol_COUNT; ++i)
@@ -210,12 +205,18 @@ TEST_CASE("Themes: applying a theme is idempotent", "[themes][apply]")
210205
{
211206
ImGuiContextFixture fixture;
212207

213-
for (const auto& applier : {std::pair{"Cherry", applyCherryTheme}, std::pair{"Darcula", applyDarculaTheme}, std::pair{"Gold", applyGoldTheme}})
208+
const std::vector<NamedApplier> subset = {
209+
{"Cherry", applyCherryTheme},
210+
{"Darcula", applyDarculaTheme},
211+
{"Gold", applyGoldTheme},
212+
};
213+
214+
for (const auto& applier : subset)
214215
{
215-
INFO("Theme: " << applier.first);
216-
applier.second();
216+
INFO("Theme: " << applier.name);
217+
applier.fn();
217218
const ImGuiStyle first = ImGui::GetStyle();
218-
applier.second();
219+
applier.fn();
219220
const ImGuiStyle second = ImGui::GetStyle();
220221
CHECK(styleEqual(first, second));
221222
}

0 commit comments

Comments
 (0)