Add test for TapCard ident file name prefix validation

[DengreSarthak]

Summary

Platform Coverage

• Tested on iOS device
• Tested on Android device
• Tested on iOS simulator
• Tested on Android simulator
• Not tested

Checklist

• I have read CONTRIBUTING.md
• I have read ARCHITECTURE.md

Summary by CodeRabbit

No user-facing changes in this release. This update contains internal testing improvements only.

[coderabbitai]

Important

Review skipped

No new commits to review since the last review.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f6568006-ac66-4d4c-91f1-fdc4f913b2a5

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

A new test case is added to validate TapSigner::ident_file_name_prefix() behavior using a known card identifier. The test verifies that the prefix is correctly derived as lowercase, dash-free, and that the parsed URL is identified as a TapSigner variant.

Changes

Cohort / File(s)	Summary
Test Coverage rust/crates/cove-tap-card/src/parse.rs	Added test case for ident_file_name_prefix() method with validation of prefix generation and TapSigner URL variant identification.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Possibly related PRs

• Add SATSCARD parser regression tests #686: Also modifies tests in rust/crates/cove-tap-card/src/parse.rs with TapSigner-related parsing test additions.

Suggested reviewers

• praveenperera

Poem

🐰 A test hops in to verify the way,
TapSigner prefixes work each day,
With dashes gone and lowercase true,
The card's identifier shines anew! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is largely incomplete; it lacks the required Summary section explaining what changed and why, and the Testing section is missing details about how the test was validated.	Add a Summary section describing the test addition and its purpose, and complete the Testing section with details on how the test was run or why testing approach was chosen.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: adding a test for TapCard ident file name prefix validation.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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 unit tests (beta)

• Create PR with 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[greptile-apps]

Greptile Summary

This PR adds a single test, test_ident_file_name_prefix, that validates the TapSigner::ident_file_name_prefix() method — confirming it strips dashes and lowercases the full card identifier string.

• New test: Parses a known TapSigner URL, calls ident_file_name_prefix(), and asserts the result equals \"xufc52swy2px24qizc7w\" (derived from \"XUFC5-2SWY2-PX24Q-IZC7W\" with dashes removed and lowercased).
• No production code changed: The PR is test-only; ident_file_name_prefix() itself is untouched.

Confidence Score: 5/5

Test-only change with no production code modifications; safe to merge.

The change adds one new test function that exercises an existing, already-validated method. No production code is altered, the test logic is correct, and the expected value is consistent with what the existing test_tap_signer_readable_ident_string test already pins.

No files require special attention.

Important Files Changed

Filename	Overview
rust/crates/cove-tap-card/src/parse.rs	Adds test_ident_file_name_prefix test that correctly validates the ident_file_name_prefix() method produces a dash-free, lowercase string; test logic is sound and implementation matches expectations.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["test_ident_file_name_prefix()"] --> B["TapCard::parse(url)"]
    B --> C{"Ok?"}
    C -- No --> D["panic!"]
    C -- Yes --> E["Extract TapSigner via let-else"]
    E --> F["ts.ident_file_name_prefix()"]
    F --> G["full_card_ident() → XUFC5-2SWY2-PX24Q-IZC7W"]
    G --> H["replace('-','') + to_ascii_lowercase()"]
    H --> I["prefix = 'xufc52swy2px24qizc7w'"]
    I --> J["assert_eq! prefix == 'xufc52swy2px24qizc7w'"]
    J --> K["assert! no dashes"]
    K --> L["assert! is lowercase"]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["test_ident_file_name_prefix()"] --> B["TapCard::parse(url)"]
    B --> C{"Ok?"}
    C -- No --> D["panic!"]
    C -- Yes --> E["Extract TapSigner via let-else"]
    E --> F["ts.ident_file_name_prefix()"]
    F --> G["full_card_ident() → XUFC5-2SWY2-PX24Q-IZC7W"]
    G --> H["replace('-','') + to_ascii_lowercase()"]
    H --> I["prefix = 'xufc52swy2px24qizc7w'"]
    I --> J["assert_eq! prefix == 'xufc52swy2px24qizc7w'"]
    J --> K["assert! no dashes"]
    K --> L["assert! is lowercase"]

Loading

_{Reviews (2): Last reviewed commit: "Add test for TapCard ident file name pre..." | Re-trigger Greptile}

[greptile-apps]

Redundant property assertions

The two follow-up assertions test properties of the hardcoded literal "xufc52swy2px24qizc7w" rather than independent function behaviour. Since the first assert_eq! already pins the exact output, any value that satisfies it also trivially passes the dash-free and lowercase checks. If the intent is to document the contract, consider a dedicated test that asserts these properties independently of the fixed value.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[coderabbitai]

🧹 Nitpick comments (1)

rust/crates/cove-tap-card/src/parse.rs (1)

480-482: Redundant assertions after exact-match check.

Once line 480 asserts prefix == "xufc52swy2px24qizc7w" (a dash-free, already-lowercase literal), the follow-up checks on lines 481–482 are tautological — they can never fail independently of line 480. If the intent is to guard against the expected literal being mistakenly edited to include a dash or uppercase later, a brief comment clarifying that intent would help; otherwise these two lines can be dropped to keep the test focused.

♻️ Optional simplification

         let prefix = ts.ident_file_name_prefix();
         // Derived from full_card_ident "XUFC5-2SWY2-PX24Q-IZC7W" with dashes removed and lowercased
         assert_eq!(prefix, "xufc52swy2px24qizc7w");
-        assert!(!prefix.contains('-'), "file name prefix must not contain dashes");
-        assert_eq!(prefix, prefix.to_lowercase(), "file name prefix must be lowercase");
     }

🤖 Prompt for AI Agents

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

In `@rust/crates/cove-tap-card/src/parse.rs` around lines 480 - 482, The test
currently asserts prefix == "xufc52swy2px24qizc7w" and then redundantly checks
assert!(!prefix.contains('-')) and assert_eq!(prefix, prefix.to_lowercase());
remove the two redundant assertions referencing prefix (or, if you intended to
document the literal's properties, replace them with a single concise comment
explaining that the canonical literal is dash-free and lowercase) so the test
stays focused; locate the checks against the variable prefix in parse.rs and
either delete the assert!(!prefix.contains('-')) and assert_eq!(prefix,
prefix.to_lowercase()) lines or convert them into an explanatory comment.

🤖 Prompt for all review comments with AI agents

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

Nitpick comments:
In `@rust/crates/cove-tap-card/src/parse.rs`:
- Around line 480-482: The test currently asserts prefix ==
"xufc52swy2px24qizc7w" and then redundantly checks
assert!(!prefix.contains('-')) and assert_eq!(prefix, prefix.to_lowercase());
remove the two redundant assertions referencing prefix (or, if you intended to
document the literal's properties, replace them with a single concise comment
explaining that the canonical literal is dash-free and lowercase) so the test
stays focused; locate the checks against the variable prefix in parse.rs and
either delete the assert!(!prefix.contains('-')) and assert_eq!(prefix,
prefix.to_lowercase()) lines or convert them into an explanatory comment.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 7b6121aa-91df-4553-a0ed-9f2aa6ede3c5

📥 Commits

Reviewing files that changed from the base of the PR and between fc4f6d3 and 75a8e89.

📒 Files selected for processing (1)

• rust/crates/cove-tap-card/src/parse.rs

[Vineet1101]

Hey @DengreSarthak can you add summary for this pr and why was it required

[praveenperera]

@coderabbitai review
@greptileai

[coderabbitai]

I'll kick off a review of this PR right away!

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.