fix: address additional AI findings in util tests

[nogeenharrie]

Summary

• Add exact path content assertions in normalizeFolderPath test
• Move PassStoreGuard to class member for reuse
• Add file existence check after opening in findBinaryInPath test

Summary by CodeRabbit

• Tests
  • Strengthened assertions for folder-path normalization and mixed-separator handling.
  • Added an automatic state-restore guard to ensure original settings are restored after tests.
  • Added immediate existence checks for temporary executables to catch creation failures early.
  • Improved test cleanup and fixture reliability.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 346c39ad-c2a3-4785-ba9a-7efbe7287291

📥 Commits

Reviewing files that changed from the base of the PR and between 947232e and cf84199.

📒 Files selected for processing (1)

• tests/auto/util/tst_util.cpp

---

📝 Walkthrough

Walkthrough

Introduce a PassStoreGuard RAII helper and QFileInfo include in tests/auto/util/tst_util.cpp; strengthen folder-path normalization assertions; refactor getDirWithIndex() to use the class-level guard; and add an existence assertion for a temporary executable in findBinaryInPathTempExecutableInTempDir().

Changes

Cohort / File(s)	Summary
Test file tests/auto/util/tst_util.cpp	Added #include <QFileInfo>, introduced a private PassStoreGuard RAII helper to save/restore QtPassSettings::getPassStore(), replaced a local guard with the class-level guard in getDirWithIndex(), and added an assertion that the temp executable exists after writing.
normalizeFolderPath tests tests/auto/util/tst_util.cpp	Reworked assertions to use a local sep = QDir::separator() and added exact-equality checks for normalized paths (e.g., "test" + sep, "test\\subdir\\"), while keeping suffix/contains checks for mixed-separator cases.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• fix: address AI findings in util tests #879 — Adds PassStoreGuard pattern and similar test changes to tests/auto/util/tst_util.cpp.
• fix: verify temp file cleanup in test #874 — Related edits to findBinaryInPathTempExecutableInTempDir() (temp executable handling and cleanup).
• fix: address 5 AI findings in test files #857 — Modifies getDirWithIndex / related test assertions in the same test file.

Poem

🐇 I tuck the store, then let it go,

Restore it neat, as tests will show,
Paths now tidy, slashes sing,
Temp files checked — a careful thing,
Hoppity hops for tests that flow.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main focus of the pull request—addressing AI findings in utility tests, which aligns with the changes in tests/auto/util/tst_util.cpp.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/ai-findings-util-tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (2)

tests/auto/util/tst_util.cpp (2)

36-41: Consider making PassStoreGuard non-copyable/non-movable.

This RAII guard could be accidentally copied or moved, leading to double-restoration of the original value. Adding move/copy prevention makes the intent clearer and prevents subtle bugs.

♻️ Suggested improvement

 private:
   struct PassStoreGuard {
     QString original;
     explicit PassStoreGuard(const QString &orig) : original(orig) {}
     ~PassStoreGuard() { QtPassSettings::setPassStore(original); }
+    PassStoreGuard(const PassStoreGuard &) = delete;
+    PassStoreGuard &operator=(const PassStoreGuard &) = delete;
   };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/auto/util/tst_util.cpp` around lines 36 - 41, PassStoreGuard can be
accidentally copied/moved causing double-restoration; make it non-copyable and
non-movable by deleting its copy constructor, copy assignment, move constructor
and move assignment (i.e., add deleted declarations for PassStoreGuard(const
PassStoreGuard&) = delete, PassStoreGuard& operator=(const PassStoreGuard&) =
delete, PassStoreGuard(PassStoreGuard&&) = delete, and PassStoreGuard&
operator=(PassStoreGuard&&) = delete) so only a single instance created via the
explicit PassStoreGuard(const QString&) exists and the destructor
(~PassStoreGuard) runs exactly once.

---

594-605: Consider using PassStoreGuard for consistency with getDirWithIndex().

getDirWithIndex() uses PassStoreGuard for RAII-based restoration (lines 626-627), but this test uses manual save/restore. Using the guard here ensures restoration even if an assertion fails between lines 595 and 605.

♻️ Suggested refactor for consistency

   QModelIndex rootIndex = fsm.index(tempDir.path());
   QVERIFY2(rootIndex.isValid(), "Filesystem model root index should be valid");
   const QString originalStore = QtPassSettings::getPassStore();
+  PassStoreGuard passStoreGuard(originalStore);
   QtPassSettings::setPassStore(tempDir.path());
 
   QString result = Util::getDir(QModelIndex(), false, fsm, sm);
   QString expectedDir = QDir(tempDir.path()).absolutePath();
   if (!expectedDir.endsWith(QDir::separator())) {
     expectedDir += QDir::separator();
   }
   QVERIFY2(
       result == expectedDir,
       qPrintable(QString("Expected '%1', got '%2'").arg(expectedDir, result)));
-  QtPassSettings::setPassStore(originalStore);
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/auto/util/tst_util.cpp` around lines 594 - 605, Replace the manual
save/restore of the pass store in the test with the RAII guard used elsewhere:
instead of calling QtPassSettings::getPassStore() and later
QtPassSettings::setPassStore(originalStore) around the Util::getDir(...) call,
construct a PassStoreGuard (as used in getDirWithIndex()) around the tempDir
setup so the original pass store is automatically restored even if assertions
fail; locate the test around Util::getDir(QModelIndex(), false, fsm, sm) in
tests/auto/util/tst_util.cpp and swap the manual QtPassSettings calls for a
PassStoreGuard instance that is initialized with tempDir.path().

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@tests/auto/util/tst_util.cpp`:
- Around line 36-41: PassStoreGuard can be accidentally copied/moved causing
double-restoration; make it non-copyable and non-movable by deleting its copy
constructor, copy assignment, move constructor and move assignment (i.e., add
deleted declarations for PassStoreGuard(const PassStoreGuard&) = delete,
PassStoreGuard& operator=(const PassStoreGuard&) = delete,
PassStoreGuard(PassStoreGuard&&) = delete, and PassStoreGuard&
operator=(PassStoreGuard&&) = delete) so only a single instance created via the
explicit PassStoreGuard(const QString&) exists and the destructor
(~PassStoreGuard) runs exactly once.
- Around line 594-605: Replace the manual save/restore of the pass store in the
test with the RAII guard used elsewhere: instead of calling
QtPassSettings::getPassStore() and later
QtPassSettings::setPassStore(originalStore) around the Util::getDir(...) call,
construct a PassStoreGuard (as used in getDirWithIndex()) around the tempDir
setup so the original pass store is automatically restored even if assertions
fail; locate the test around Util::getDir(QModelIndex(), false, fsm, sm) in
tests/auto/util/tst_util.cpp and swap the manual QtPassSettings calls for a
PassStoreGuard instance that is initialized with tempDir.path().

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 141a0eb5-d562-463b-83d5-f851b5e26d84

📥 Commits

Reviewing files that changed from the base of the PR and between 7cd4782 and 947232e.

📒 Files selected for processing (1)

• tests/auto/util/tst_util.cpp

[coveralls]

coverage: 20.071%. remained the same
when pulling cf84199 on fix/ai-findings-util-tests
into 7cd4782 on main.