test: expand gpgkeystate and storemodel test suites#1499
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds seven Qt test slots: four tests for parseGpgColonOutput edge cases (empty input, missing uid, orphan sub/ssb, short malformed lines) and three tests for StoreModel behaviors (store path updates, EditRole ChangesTest Coverage Expansion
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/gpgkeystate/tst_gpgkeystate.cpp`:
- Around line 479-489: The test parseGpgColonOutputPubWithoutUid should assert
that the UserInfo.name is populated from the pub record's userid field (field
index 9); update the test in tst_gpgkeystate::parseGpgColonOutputPubWithoutUid
to call parseGpgColonOutput(...), then add a QVERIFY/QCOMPARE verifying
result.first().name == QStringLiteral("Name Only") (or equivalent) to ensure
parseGpgColonOutput sets the name when no uid record exists.
In `@tests/auto/model/tst_storemodel.cpp`:
- Line 50: The test named filterHidesNonGpgFile has a contradictory name vs
behavior (it actually verifies a non-.gpg file is accepted when the regex
matches); rename the test to reflect the asserted behavior (e.g.,
filterAcceptsNonGpgFileWhenRegexMatches) and update the function declaration,
definition, and any references/registrations to use the new name (change the
prototype void filterHidesNonGpgFile(); and its implementation and any test
suite entries to void filterAcceptsNonGpgFileWhenRegexMatches();).
- Around line 530-534: Replace the tautological QCOMPARE assertions with
descriptive QVERIFY2 checks that include explicit failure messages: assert that
sm.getStore() equals tempDir.path() after construction and equals newPath after
calling sm.setStore(newPath), using statements that call QVERIFY2(condition,
"message") where the message names the expectation (e.g. "store should default
to temporary directory: expected <tempDir> but was <actual>" and "store should
update to substore after setStore: expected <newPath> but was <actual>"). Ensure
you reference the same methods/variables from the diff (sm.getStore(),
sm.setStore(newPath), tempDir.path(), newPath) when building the condition and
message.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4fe99ff6-7475-49c9-b0fa-30f3b1a6ef76
📒 Files selected for processing (2)
tests/auto/gpgkeystate/tst_gpgkeystate.cpptests/auto/model/tst_storemodel.cpp
|
coverage: 55.418%. remained the same — test/gpgkeystate-storemodel-tests into main |
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 2 file(s) based on 3 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
There was a problem hiding this comment.
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/gpgkeystate/tst_gpgkeystate.cpp`:
- Around line 479-490: The test
tst_gpgkeystate::parseGpgColonOutputPubWithoutUid uses input where the username
is in the wrong colon field (index 12) while parseGpgColonOutput reads the name
from GPG_FIELD_USERID (field index 9); update the test's input string in
parseGpgColonOutputPubWithoutUid so the "Name Only" token appears in the ninth
colon-delimited field (the userid field) matching parseGpgColonOutput's
expectations, ensuring the QCOMPARE on result.first().name passes.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: d9183b85-ae91-42fe-8d99-59de8519b36a
📒 Files selected for processing (2)
tests/auto/gpgkeystate/tst_gpgkeystate.cpptests/auto/model/tst_storemodel.cpp
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tests/auto/gpgkeystate/tst_gpgkeystate.cpp (1)
479-490:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winField index mismatch persists in test data.
This issue was previously flagged: "Name Only" is placed at field index 10, but the test comment (line 481) and the implementation expect the userid at field index 9 (GPG_FIELD_USERID). The test input should place "Name Only" at field 9.
🔧 Corrected test input
const QString input = - QStringLiteral("pub:u:4096:1:NOUIDKEY1:1774947438:::u:Name Only:\n"); + QStringLiteral("pub:u:4096:1:NOUIDKEY1:1774947438:::u:Name Only::\n");🤖 Prompt for 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. In `@tests/auto/gpgkeystate/tst_gpgkeystate.cpp` around lines 479 - 490, The test tst_gpgkeystate::parseGpgColonOutputPubWithoutUid has the userid placed in the wrong colon field (it's currently at field index 10); update the test input string passed to parseGpgColonOutput so the "Name Only" value appears at GPG_FIELD_USERID (field index 9) in the pub record line (i.e. shift the empty field so the userid occupies the 9th colon-separated slot), keeping the rest of the record intact so the assertions about key_id and name still hold.
🤖 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.
Duplicate comments:
In `@tests/auto/gpgkeystate/tst_gpgkeystate.cpp`:
- Around line 479-490: The test
tst_gpgkeystate::parseGpgColonOutputPubWithoutUid has the userid placed in the
wrong colon field (it's currently at field index 10); update the test input
string passed to parseGpgColonOutput so the "Name Only" value appears at
GPG_FIELD_USERID (field index 9) in the pub record line (i.e. shift the empty
field so the userid occupies the 9th colon-separated slot), keeping the rest of
the record intact so the assertions about key_id and name still hold.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: cc81ace6-7ca0-4dcf-bd80-0029c1d39cea
📒 Files selected for processing (1)
tests/auto/gpgkeystate/tst_gpgkeystate.cpp
There was a problem hiding this comment.
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/model/tst_storemodel.cpp`:
- Around line 548-549: Replace the bare QVERIFY assertions that check file
creation with QVERIFY2 so failures include descriptive messages: change the
assertions that call f.open(QFile::WriteOnly) (variable f using
QFile::WriteOnly) to use QVERIFY2(..., "<descriptive message>") and do the same
for the second occurrence around lines 573-574; update the message to clearly
state what file operation failed (e.g., "Failed to open test file for writing")
to make setup failures explicit.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 08707885-5f05-4756-af26-e7c7a152ad9b
📒 Files selected for processing (1)
tests/auto/model/tst_storemodel.cpp
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
There was a problem hiding this comment.
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/model/tst_storemodel.cpp`:
- Around line 569-571: Update the misleading comment to state that the test is
asserting behavior for the explicit "readme" filter (not a claim about any
default empty regex); specifically, replace the sentence that says the default
empty regex “won’t match” with a clear note that because the test sets the
filter to "readme", a plain text file without a .gpg extension should not pass
the filter unless the filter explicitly matches its basename (i.e., the test
only covers the explicit "readme" filter behavior).
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 56e06083-ae96-4a3f-9df8-ddad65934c6f
📒 Files selected for processing (1)
tests/auto/model/tst_storemodel.cpp
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 2 file(s) based on 3 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Fix clang-format violations in the two new test additions: - tst_storemodel: reflow two over-long qPrintable/QStringLiteral argument lines in setStoreUpdatesPath Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
annejan
force-pushed
the
test/gpgkeystate-storemodel-tests
branch
from
6dc9421 to
a60bec2
Compare
3 participants
Summary
tst_gpgkeystate: 23 → 27 tests — adds edge-case coverage forparseGpgColonOutput: empty input, pub-without-uid, orphan sub/ssb records, and short lines that should be silently skippedtst_storemodel: 31 → 34 tests — addssetStoreUpdatesPath(verifiessetStore()persists the new path),dataEditRoleKeepsGpgExtension(verifies.gpgis only stripped forDisplayRole), andfilterHidesNonGpgFile(documents filter behavior for non-.gpgentries)Test plan
make checkpasses locally (27 gpgkeystate, 36 storemodel tests pass)🤖 Generated with Claude Code
Summary by CodeRabbit