Skip to content

refactor(test/mainwindow): use QDir::filePath() for .gpg-id path construction#1491

Merged
annejan merged 3 commits into
mainfrom
test/mainwindow-unit-tests
May 15, 2026
Merged

refactor(test/mainwindow): use QDir::filePath() for .gpg-id path construction#1491
annejan merged 3 commits into
mainfrom
test/mainwindow-unit-tests

Conversation

@nogeenharrie
Copy link
Copy Markdown
Contributor

@nogeenharrie nogeenharrie commented May 15, 2026

Summary

  • Replace QDir::cleanPath(m_storeDir.path() + "/.gpg-id") with QDir(m_storeDir.path()).filePath(".gpg-id") — more idiomatic Qt path construction that handles separator normalisation intrinsically, without manual string concatenation.

Skipped findings

  • Rename tst_mainwindowtst_MainWindow: every other test class in the project uses tst_<lowercase> (tst_storemodel, tst_gpgkeystate, tst_configdialog, …). The rename would be inconsistent with the established convention.
  • Add normalHtml != errorHtml styling assertion: flashText calls setTextColor(Qt::red) then setText(text). For plain text, setText() delegates to setPlainText() which resets the document with default formatting, discarding the char format set by setTextColor(). Both calls produce identical toHtml() output, so the assertion would always fail.

Test plan

  • All 14 tests pass locally with --platform offscreen

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests
    • Improved test handling of store file paths to be more robust across environments.
    • Made minimal test store creation more reliable, reducing flaky behavior in main-window tests.

Review Change Stack

…truction

QDir(dir).filePath(name) handles separator normalisation intrinsically
and is more idiomatic Qt than manual string concatenation + cleanPath.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 15, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

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

Review profile: ASSERTIVE

Plan: Pro

Run ID: bf38c7da-cb2f-4070-a42c-e2b7d328eead

📥 Commits

Reviewing files that changed from the base of the PR and between 3fe3ba8 and bfbbedf.

📒 Files selected for processing (1)
  • tests/auto/mainwindow/tst_mainwindow.cpp

📝 Walkthrough

Walkthrough

The test setup in initTestCase now constructs the .gpg-id path with QDir(m_storeDir.path()).filePath(".gpg-id") (cleaned via QDir::cleanPath) instead of manual string concatenation.

Changes

Test initialization path normalization

Layer / File(s) Summary
GPG-ID file path construction in test setup
tests/auto/mainwindow/tst_mainwindow.cpp
The .gpg-id file path in initTestCase now uses QDir(m_storeDir.path()).filePath(".gpg-id") wrapped with QDir::cleanPath, replacing prior manual concatenation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • IJHack/QtPass#1490: Both PRs modify tests/auto/mainwindow/tst_mainwindow.cpp to normalize how the temporary pass-store .gpg-id path is built.
  • IJHack/QtPass#905: Also updates .gpg-id path construction to use QDir(...).filePath(".gpg-id") in production code.

Suggested labels

size:XS

Poem

🐰 A tiny change, a tidy trail,
QDir hops in to mend the tail,
Paths now clean with every stride,
The test finds home, no slash to hide,
I nibble bytes and bounce with pride.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 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 refactoring change: replacing string concatenation with QDir::filePath() for .gpg-id path construction, directly matching the PR's core objective.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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 test/mainwindow-unit-tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/auto/mainwindow/tst_mainwindow.cpp`:
- Line 68: The change removed the required path normalization: wrap the result
of QDir(m_storeDir.path()).filePath(QStringLiteral(".gpg-id")) with
QDir::cleanPath() before constructing QFile (i.e., compute a cleaned path via
QDir::cleanPath(QDir(m_storeDir.path()).filePath(...))) and pass that to QFile
(affecting the QFile gpgId creation); this restores cross-platform normalization
per project guidelines.
🪄 Autofix (Beta)

✅ Autofix completed


ℹ️ Review info
⚙️ Run configuration

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

Review profile: ASSERTIVE

Plan: Pro

Run ID: ce082132-acc8-41e2-bbaf-276e54843fec

📥 Commits

Reviewing files that changed from the base of the PR and between 87bf346 and 4091d07.

📒 Files selected for processing (1)
  • tests/auto/mainwindow/tst_mainwindow.cpp

Comment thread tests/auto/mainwindow/tst_mainwindow.cpp Outdated
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 15, 2026

Note

Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it.

Fixes Applied Successfully

Fixed 1 file(s) based on 1 unresolved review comment.

Files modified:

  • tests/auto/mainwindow/tst_mainwindow.cpp

Commit: 3fe3ba803d9b7aa7dd76105e1b55d8a9bbbcee3c

The changes have been pushed to the test/mainwindow-unit-tests branch.

Time taken: 1m 24s

Fixed 1 file(s) based on 1 unresolved review comment.

Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
@coveralls
Copy link
Copy Markdown

coveralls commented May 15, 2026

Coverage Status

coverage: 53.836%. remained the same — test/mainwindow-unit-tests into main

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@annejan annejan merged commit 2d4a157 into main May 15, 2026
24 of 25 checks passed
@annejan annejan deleted the test/mainwindow-unit-tests branch May 15, 2026 23:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants