🧪 test: Improve CCCD OCR parsing test coverage#4
Conversation
- Added tests covering happy paths for both Vietnamese (next-line values) and English (same-line values) CCCD formats. - Tested edge cases including empty/missing data and malformed/random text inputs. - Validated default value assignments (e.g., gender, nationality). Co-authored-by: chuanman2707 <29907469+chuanman2707@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a1bb5c96ee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| assert_eq!(info.doc_number, ""); | ||
| assert_eq!(info.full_name, ""); | ||
| assert_eq!(info.dob, ""); | ||
| assert_eq!(info.gender, "Nam"); |
There was a problem hiding this comment.
Reject substring gender match in random-text test
This edge-case test is intended to validate malformed input handling, but asserting "Nam" for the token "Maleish" codifies a false positive (parse_cccd currently matches "Male" as a substring). Keeping this expectation makes it harder to fix the real parsing bug (gender should only be inferred from standalone labels/values) because a correctness improvement would now fail the test suite.
Useful? React with 👍 / 👎.
Replaced the closure inside `sort_by` that calls `.cmp()` with a more concise `sort_by_key` using `std::cmp::Reverse` which is recommended by clippy to resolve the GitHub Actions CI workflow check errors. Co-authored-by: chuanman2707 <29907469+chuanman2707@users.noreply.github.com>
|
The OCR coverage work is useful, but I’m not merging this as-is. The branch includes an unrelated |
I have removed the unrelated |
- Added comprehensive unit tests for `parse_cccd` in `mhm/src-tauri/src/ocr.rs`. - Fixed a false positive in the gender parser where `Maleish` resulted in `Nam`. The gender parser now correctly bounds its searches to whole words (`\\b(Nam|Male)\\b` and `\\b(Nữ|Female)\\b`). - Ensured testing accounts for English/Vietnamese differences and fields ending on the next line or the same line. - Assured empty text edge cases default back to empty strings, save for nationality defaulting to Việt Nam. Co-authored-by: chuanman2707 <29907469+chuanman2707@users.noreply.github.com>
Replaced the closure inside `sort_by` that calls `.cmp()` with a more concise `sort_by_key` using `std::cmp::Reverse` which is recommended by clippy to resolve the GitHub Actions CI workflow check errors. Co-authored-by: chuanman2707 <29907469+chuanman2707@users.noreply.github.com>
🎯 What: Added tests for the CCCD OCR parsing logic to
mhm/src-tauri/src/ocr.rsto address a lack of test coverage for theparse_cccdfunction.📊 Coverage: The new tests cover:
✨ Result: Test suite reliability and coverage for the core OCR extraction functions have been significantly improved.
PR created automatically by Jules for task 1054232453394269589 started by @chuanman2707