Skip to content

test: add edge-case tests for simpletransaction and filecontent#1493

Merged
annejan merged 2 commits into
mainfrom
test/more-edge-case-tests
May 16, 2026
Merged

test: add edge-case tests for simpletransaction and filecontent#1493
annejan merged 2 commits into
mainfrom
test/more-edge-case-tests

Conversation

@nogeenharrie
Copy link
Copy Markdown
Contributor

@nogeenharrie nogeenharrie commented May 16, 2026

Summary

  • simpletransaction (6 → 14 tests): covers transactionIsOver with wrong id, empty queue, transactionEnd without transactionStart, start+end without add, result process differing from trigger process, and deeply nested transactions
  • filecontent (19 → 26 tests): covers allFields=true mode, URL-line exclusion in allFields, CRLF named-value trimming, multiple otpauth:// lines hidden from display, and case-insensitive OTPAUTH:// hiding

Test plan

  • make check passes locally for both suites (14 + 26 tests, 0 failures)
  • clang-format --style=file applied

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests
    • Expanded file-content parsing tests: improved coverage for extracting name:value fields, excluding URL-like lines, normalizing CRLF line endings, and preserving/hiding protocol-specific lines from display while retaining them in remaining data.
    • Extended transaction tests: added coverage for invalid/empty-queue states, no-op end/start scenarios, result identity vs. trigger, and deeply nested transaction behavior.

Review Change Stack

simpletransaction (6 → 14 tests):
- transactionIsOver with wrong id returns INVALID
- transactionIsOver on empty queue returns INVALID
- transactionEnd without prior transactionStart is a no-op
- transactionStart + transactionEnd without transactionAdd queues nothing
- transactionEnd result process differs from transactionAdd trigger
- deeply nested transaction: inner add overwrites outer, outer end wins

filecontent (19 → 26 tests):
- allFields=true includes unlabelled name:value pairs
- allFields=true excludes bare URL lines (http://... parsed as colon-name)
- CRLF line endings: value.trimmed() strips the trailing \r
- multiple otpauth:// lines are all kept in getRemainingData but all
  hidden from getRemainingDataForDisplay
- OTPAUTH:// (uppercase) is also hidden (case-insensitive check)

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

coderabbitai Bot commented May 16, 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: f34f6f0c-f242-4593-b458-b7e45e600c87

📥 Commits

Reviewing files that changed from the base of the PR and between caf51de and d29bc99.

📒 Files selected for processing (1)
  • tests/auto/simpletransaction/tst_simpletransaction.cpp

📝 Walkthrough

Walkthrough

Adds unit tests: FileContent parsing tests for all-fields extraction, URL-line exclusion, CRLF trimming, and otpauth display/remaining-data behavior; SimpleTransaction tests for invalid/empty transaction states, start/end/add semantics, and nested-transaction selection.

Changes

FileContent Parsing Test Coverage

Layer / File(s) Summary
FileContent parsing edge cases
tests/auto/filecontent/tst_filecontent.cpp
Five new test slots verify FileContent::parse behavior: all-fields mode extracts name:value pairs into NamedValues, URL-like lines stay in getRemainingData, CRLF-trimmed named values remove trailing \r, multiple otpauth:// lines are kept in remaining data but hidden from getRemainingDataForDisplay, and OTPAUTH:// is treated case-insensitively for the same hiding behavior.

SimpleTransaction Transaction Edge Case Coverage

Layer / File(s) Summary
SimpleTransaction transaction validation
tests/auto/simpletransaction/tst_simpletransaction.cpp
Six new test slots assert transactionIsOver() returns Enums::INVALID for wrong IDs and empty queues; verify transactionEnd() is a no-op when called without transactionStart() or without intervening transactionAdd(); confirm transactionEnd(expectedId) associates the result with the expected ID; and validate deeply nested transactions use the last queued operation ID.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • IJHack/QtPass#1122: Modifies tst_simpletransaction tests around transactionAdd/transactionIsOver/transactionEnd.
  • IJHack/QtPass#983: Strengthens tst_filecontent assertions for named-value parsing in all-fields mode.
  • IJHack/QtPass#981: Adds tst_filecontent tests for otpauth:// remaining vs. display behavior.

Suggested labels

size:M

Poem

🐰 New tests hop through code and cheer,
Parsing lines and transactions clear.
CRLF trimmed, otpauth tucked away,
Nested ends ensure the right display.
A rabbit claps—CI green today.

🚥 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 directly and accurately summarizes the main change: adding edge-case unit tests for two modules (simpletransaction and filecontent).
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/more-edge-case-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/simpletransaction/tst_simpletransaction.cpp`:
- Around line 90-131: Replace the QCOMPARE assertions in the test methods
transactionIsOverWrongIdReturnsInvalid,
transactionIsOverEmptyQueueReturnsInvalid, transactionEndWithoutStartIsNoop,
transactionStartEndWithoutAddIsNoop, transactionEndResultDiffersFromAdd, and
deeplyNestedTransactionUsesLastAdd with QVERIFY2 checks that include explicit
failure messages; each message should state the intent (e.g., "Expected
transactionIsOver to return INVALID when wrong id provided" for
transactionIsOverWrongIdReturnsInvalid) and reference the expected Enums value
and the operation that produced it (use names like transactionAdd,
transactionStart, transactionEnd, transactionIsOver and the specific Enums
values such as PASS_SHOW, PASS_INSERT, GIT_ADD, GIT_COMMIT, GIT_PUSH) so
failures clearly indicate which operation and expected enum failed.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

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

Review profile: ASSERTIVE

Plan: Pro

Run ID: f086dd95-9522-4073-9110-f640bee5dcd4

📥 Commits

Reviewing files that changed from the base of the PR and between f2f8f41 and caf51de.

📒 Files selected for processing (2)
  • tests/auto/filecontent/tst_filecontent.cpp
  • tests/auto/simpletransaction/tst_simpletransaction.cpp

Comment thread tests/auto/simpletransaction/tst_simpletransaction.cpp
Each failure message now names the operation (transactionAdd,
transactionStart, transactionEnd, transactionIsOver), the specific
Enums values involved (PASS_SHOW, PASS_INSERT, GIT_ADD, GIT_COMMIT,
GIT_PUSH, INVALID), and the causal chain so a failure output is
self-explanatory.

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

coveralls commented May 16, 2026

Coverage Status

Coverage is 53.917%test/more-edge-case-tests into main. No base build found for main.

@annejan annejan merged commit c392215 into main May 16, 2026
24 of 25 checks passed
@annejan annejan deleted the test/more-edge-case-tests branch May 16, 2026 01:08
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