What the developer wrote
Authored by selsta
tests: fix Windows unit test failures
Preserve line endings for test fixtures so file hashes remain stable
across platforms.
Temporarily release the wallet keys lock while inspecting the keys file,
then reacquire it and verify the lock state. This lets the test read the
file on Windows, where the lock prevents a concurrent read open.
Avoid passing possibly negative char values to std::isprint(), which is
undefined behavior except for EOF.
What changed, and why it matters
This commit fixes three test-only issues that caused unit tests to fail on Windows. It ensures test data files keep consistent line endings so their cryptographic hashes match, lets a test briefly release a file lock so it can read a wallet keys file on Windows, and avoids a minor undefined-behavior case when checking whether characters are printable. None of these changes affect the Monero wallet or node code that users rely on; they only make the test suite more reliable.
No security action required. Treat as a normal test-fix commit. Reviewers may verify that the unlock/lock pattern in the test does not mask a real product issue, but the change is clearly scoped to the test.
Security signals we found
Undefined behavior in test code removed: negative char values no longer passed to std::isprint()
File locking behavior in unit test adjusted to accommodate Windows semantics
No changes to production cryptographic, consensus, or wallet-handling code
Evidence from the diff
The patch modifies .gitattributes to mark files under tests/data/** as -text, preventing Git from converting line endings so test fixture hashes stay stable across platforms. In tests/unit_tests/wallet_storage.cpp, it changes std::isprint(c) to std::isprint(static_cast
Changed components
.gitattributestests/unit_tests/wallet_storage.cppInspect captured patch +9 / −4
diff --git a/.gitattributes b/.gitattributes
index f75097e..da492ba 100644
--- a/.gitattributes
+++ b/.gitattributes
@@ -1,2 +1,3 @@
.git* export-ignore
version.cmake export-subst
+tests/data/** -text
diff --git a/tests/unit_tests/wallet_storage.cpp b/tests/unit_tests/wallet_storage.cpp
index 6d25686..60e2f63 100644
--- a/tests/unit_tests/wallet_storage.cpp
+++ b/tests/unit_tests/wallet_storage.cpp
@@ -302,7 +302,7 @@ TEST(wallet_storage, gen_ascii_format)
ASSERT_TRUE(epee::file_io_utils::load_file_to_string(target_wallet_file.string() + ".keys", key_file_contents));
EXPECT_NE(std::string::npos, key_file_contents.find(WALLET2_ASCII_OUTPUT_MAGIC));
for (const char c : key_file_contents)
- ASSERT_TRUE(std::isprint(c) || c == '\n' || c == '\r');
+ ASSERT_TRUE(std::isprint(static_cast<unsigned char>(c)) || c == '\n' || c == '\r');
}
{
@@ -340,12 +340,16 @@ TEST(wallet_storage, change_export_format)
// Assert that we initially store keys in binary format
{
std::string key_file_contents;
- ASSERT_TRUE(epee::file_io_utils::load_file_to_string(target_wallet_file.string() + ".keys", key_file_contents));
+ ASSERT_TRUE(w.unlock_keys_file());
+ const bool loaded = epee::file_io_utils::load_file_to_string(target_wallet_file.string() + ".keys", key_file_contents);
+ ASSERT_TRUE(w.lock_keys_file());
+ ASSERT_TRUE(w.is_keys_file_locked());
+ ASSERT_TRUE(loaded);
EXPECT_EQ(std::string::npos, key_file_contents.find(WALLET2_ASCII_OUTPUT_MAGIC));
bool only_printable = true;
for (const char c : key_file_contents)
{
- if (!std::isprint(c) && c != '\n' && c != '\r')
+ if (!std::isprint(static_cast<unsigned char>(c)) && c != '\n' && c != '\r')
{
only_printable = false;
break;
@@ -369,7 +373,7 @@ TEST(wallet_storage, change_export_format)
ASSERT_TRUE(epee::file_io_utils::load_file_to_string(target_wallet_file.string() + ".keys", key_file_contents));
EXPECT_NE(std::string::npos, key_file_contents.find(WALLET2_ASCII_OUTPUT_MAGIC));
for (const char c : key_file_contents)
- ASSERT_TRUE(std::isprint(c) || c == '\n' || c == '\r');
+ ASSERT_TRUE(std::isprint(static_cast<unsigned char>(c)) || c == '\n' || c == '\r');
}
{
Why this scored 19/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.