-
Notifications
You must be signed in to change notification settings - Fork 0
refactor and add unit test #4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request refactors the CI workflow and enhances testability of the rquery-orm-macros crate by simplifying the GitHub Actions CI configuration, improving macro attribute parsing logic, and introducing comprehensive test coverage with test shims for external dependencies.
- Simplified CI workflow by removing external database services and adding code coverage checks
- Refactored attribute parsing in the macro to use match statements for better maintainability
- Added test-only shims for external dependencies to enable procedural macro testing
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| .github/workflows/ci.yml | Simplified CI workflow by removing database services and adding coverage checks |
| rquery-orm-macros/src/lib.rs | Refactored attribute parsing logic and added test shims for external dependencies |
| rquery-orm-macros/src/tests.rs | Added comprehensive test suite for the procedural macro functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| , "max_length" => { | ||
| if let Lit::Int(i) = &nv.lit { max_length = i.base10_parse().ok(); } | ||
| } | ||
| , "min_length" => { | ||
| if let Lit::Int(i) = &nv.lit { min_length = i.base10_parse().ok(); } | ||
| } | ||
| , "regex" => { | ||
| if let Lit::Str(s) = &nv.lit { regex = Some(s.value()); } | ||
| } | ||
| , "error_max_length" => { | ||
| if let Lit::Str(s) = &nv.lit { err_max_length = Some(s.value()); } | ||
| } | ||
| , "error_min_length" => { | ||
| if let Lit::Str(s) = &nv.lit { err_min_length = Some(s.value()); } | ||
| } | ||
| , "error_required" => { | ||
| if let Lit::Str(s) = &nv.lit { err_required = Some(s.value()); } | ||
| } | ||
| , "error_allow_null" => { | ||
| if let Lit::Str(s) = &nv.lit { err_allow_null = Some(s.value()); } | ||
| } | ||
| , "error_allow_empty" => { | ||
| if let Lit::Str(s) = &nv.lit { err_allow_empty = Some(s.value()); } | ||
| } | ||
| , "error_regex" => { | ||
| if let Lit::Str(s) = &nv.lit { err_regex = Some(s.value()); } | ||
| } | ||
| , "allow_empty" => { | ||
| if let Lit::Bool(b) = &nv.lit { allow_empty = b.value; } | ||
| } | ||
| , "required" => { | ||
| if let Lit::Bool(b) = &nv.lit { required = b.value; } | ||
| } | ||
| , "allow_null" => { | ||
| if let Lit::Bool(b) = &nv.lit { allow_null = b.value; } | ||
| } | ||
| , "ignore_in_update" => { | ||
| if let Lit::Bool(b) = &nv.lit { ignore_in_update = b.value; } | ||
| } | ||
| , "ignore_in_insert" => { | ||
| if let Lit::Bool(b) = &nv.lit { ignore_in_insert = b.value; } | ||
| } | ||
| , "ignore_in_delete" => { | ||
| if let Lit::Bool(b) = &nv.lit { ignore_in_delete = b.value; } | ||
| } | ||
| , "ignore" => { | ||
| if let Lit::Bool(b) = &nv.lit { ignore = b.value; } | ||
| } | ||
| , _ => {} |
Copilot
AI
Sep 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra commas before the match arm patterns will cause compilation errors. Remove the leading commas on lines 174, 177, 180, 183, 186, 189, 192, 195, 198, 201, 204, 207, 210, 213, 216, 219, and 222.
| - name: Install PostgreSQL client | ||
| run: sudo apt-get update && sudo apt-get install -y postgresql-client | ||
| - name: Wait for MSSQL | ||
| - uses: actions-rs/toolchain@v1 |
Copilot
AI
Sep 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The actions-rs/toolchain action is deprecated. Use dtolnay/rust-toolchain which was already imported in the original code and is the recommended alternative.
| - uses: actions-rs/toolchain@v1 | |
| - uses: dtolnay/rust-toolchain@v1 |
This pull request refactors the CI workflow and improves the testability of the
rquery-orm-macroscrate. The main changes include simplifying the GitHub Actions CI configuration, making the macro attribute parsing logic more robust, and introducing comprehensive test shims and tests for the procedural macro. These updates enhance code maintainability, reliability, and ensure the macro’s correctness through automated tests.CI/CD Workflow Improvements:
.github/workflows/ci.ymlworkflow by removing the setup and usage of external database services (MSSQL and PostgreSQL), and restructured the test steps to run directly withcargo test. Also added code coverage checks usingcargo-llvm-covand enabled workflow triggers on bothmainandmasterbranches.Macro Logic and Testability Enhancements:
entitymacro to use a match statement on the attribute name string, making the code more maintainable and less repetitive.anyhow,regex,uuid,tiberius,tokio_postgres, and internalrquery_ormmodules) to enable the procedural macro to be tested without external dependencies.tests.rsfile with comprehensive tests that verify the generated metadata, validation logic, SQL generation, and trait implementations for entities derived using the macro.