-
-
Notifications
You must be signed in to change notification settings - Fork 35
✅ increase test coverage #113
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
Conversation
…pedDict is missing
…d connection error handling
|
Warning Rate limit exceeded@techouse has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 20 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis pull request introduces comprehensive unit test coverage for the MySQL to SQLite converter, including three new or significantly expanded test modules: CLI error-path validation, transporter edge cases and fallback behaviours, and type system fallback mechanisms. All changes are test-focused with no modifications to production code or public APIs. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #113 +/- ##
==========================================
+ Coverage 93.50% 99.11% +5.61%
==========================================
Files 8 8
Lines 908 908
==========================================
+ Hits 849 900 +51
+ Misses 59 8 -51 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/unit/test_cli_error_paths.py (1)
68-96: Consider extracting common kwargs into a fixture.The large kwargs dictionary is duplicated across both debug tests. Extracting it into a pytest fixture would improve maintainability.
Example:
@pytest.fixture def debug_cli_kwargs(): return { "sqlite_file": "out.sqlite3", "mysql_user": "user", "prompt_mysql_password": False, "mysql_password": None, "mysql_database": "db", # ... remaining kwargs "debug": True, }Then use it in tests:
def test_debug_reraises_keyboard_interrupt(self, monkeypatch: pytest.MonkeyPatch, debug_cli_kwargs) -> None: # ... setup code with pytest.raises(KeyboardInterrupt): mysql2sqlite.callback(**debug_cli_kwargs)Also applies to: 112-140
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
tests/unit/test_cli_error_paths.py(1 hunks)tests/unit/test_transporter.py(6 hunks)tests/unit/test_types_fallback.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
tests/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use pytest; keep tests under tests/ mirroring src/mysql_to_sqlite3/ structure
Files:
tests/unit/test_cli_error_paths.pytests/unit/test_types_fallback.pytests/unit/test_transporter.py
tests/unit/test_*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
tests/unit/test_*.py: Add unit tests in tests/unit/ for isolated helpers; create targeted files named test_.py
Add at least one unit test covering new flag behavior/validation
Add unit tests for new type mappings in _translate_type_from_mysql_to_sqlite
Add unit tests for new default expression translations
Files:
tests/unit/test_cli_error_paths.pytests/unit/test_types_fallback.pytests/unit/test_transporter.py
{src,tests}/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Adhere to lint suite: Black line length 120 and isort profile=black import ordering for all Python files
Files:
tests/unit/test_cli_error_paths.pytests/unit/test_types_fallback.pytests/unit/test_transporter.py
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
PR: techouse/mysql-to-sqlite3#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-18T21:08:55.973Z
Learning: Applies to src/mysql_to_sqlite3/transporter.py : Extend _translate_type_from_mysql_to_sqlite when adding type mappings and add corresponding tests
Learnt from: CR
PR: techouse/mysql-to-sqlite3#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-18T21:08:55.973Z
Learning: Applies to tests/unit/test_*.py : Add unit tests for new type mappings in _translate_type_from_mysql_to_sqlite
📚 Learning: 2025-10-18T21:08:55.973Z
Learnt from: CR
PR: techouse/mysql-to-sqlite3#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-18T21:08:55.973Z
Learning: Applies to tests/unit/test_*.py : Add unit tests for new type mappings in _translate_type_from_mysql_to_sqlite
Applied to files:
tests/unit/test_types_fallback.pytests/unit/test_transporter.py
📚 Learning: 2025-10-18T21:08:55.973Z
Learnt from: CR
PR: techouse/mysql-to-sqlite3#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-18T21:08:55.973Z
Learning: Applies to src/mysql_to_sqlite3/transporter.py : Extend _translate_type_from_mysql_to_sqlite when adding type mappings and add corresponding tests
Applied to files:
tests/unit/test_transporter.py
📚 Learning: 2025-10-18T21:08:55.973Z
Learnt from: CR
PR: techouse/mysql-to-sqlite3#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-18T21:08:55.973Z
Learning: Applies to src/mysql_to_sqlite3/transporter.py : Implement and modify core transfer logic (schema introspection, batching, type/default translation, reconnection handling) in transporter.py
Applied to files:
tests/unit/test_transporter.py
📚 Learning: 2025-10-18T21:08:55.973Z
Learnt from: CR
PR: techouse/mysql-to-sqlite3#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-18T21:08:55.973Z
Learning: Applies to src/mysql_to_sqlite3/transporter.py : Extend _translate_default_from_mysql_to_sqlite to support new default expressions and add tests
Applied to files:
tests/unit/test_transporter.py
📚 Learning: 2025-10-18T21:08:55.973Z
Learnt from: CR
PR: techouse/mysql-to-sqlite3#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-18T21:08:55.973Z
Learning: Applies to src/mysql_to_sqlite3/transporter.py : Raise ValueError in constructor for invalid configuration
Applied to files:
tests/unit/test_transporter.py
🧬 Code graph analysis (1)
tests/unit/test_transporter.py (3)
src/mysql_to_sqlite3/sqlite_utils.py (1)
CollatingSequences(51-56)src/mysql_to_sqlite3/transporter.py (11)
MySQLtoSQLite(44-1219)_transpile_mysql_expr_to_sqlite(310-325)_transpile_mysql_type_to_sqlite(355-439)_quote_sqlite_identifier(328-347)_translate_default_from_mysql_to_sqlite(442-579)_data_type_collation_sequence(582-620)_get_unique_index_name(629-654)_build_create_table_sql(656-871)_mysql_viewdef_to_sqlite(898-928)_build_create_view_sql(930-993)_create_view(995-1019)tests/conftest.py (4)
MySQLCredentials(154-161)sqlite_database(144-146)mysql_credentials(165-191)mysql_database(275-317)
🪛 Flake8 (7.3.0)
tests/unit/test_transporter.py
[error] 47-47: undefined name 'os'
(F821)
[error] 47-47: undefined name 't'
(F821)
[error] 106-106: undefined name 'os'
(F821)
[error] 106-106: undefined name 't'
(F821)
[error] 613-613: undefined name 'os'
(F821)
[error] 613-613: undefined name 't'
(F821)
[error] 669-669: undefined name 'os'
(F821)
[error] 669-669: undefined name 't'
(F821)
[error] 762-762: undefined name 'os'
(F821)
[error] 762-762: undefined name 't'
(F821)
[error] 776-776: undefined name 'os'
(F821)
[error] 776-776: undefined name 't'
(F821)
[error] 790-790: undefined name 'os'
(F821)
[error] 790-790: undefined name 't'
(F821)
[error] 804-804: undefined name 'os'
(F821)
[error] 804-804: undefined name 't'
(F821)
[error] 824-824: undefined name 'os'
(F821)
[error] 824-824: undefined name 't'
(F821)
🪛 Ruff (0.14.1)
tests/unit/test_cli_error_paths.py
14-14: Avoid specifying long messages outside the exception class
(TRY003)
tests/unit/test_transporter.py
47-47: Undefined name os
(F821)
47-47: Undefined name t
(F821)
61-61: Unused method argument: args
(ARG002)
61-61: Unused method argument: kwargs
(ARG002)
73-73: Unused lambda argument: charset
(ARG005)
80-80: Unused lambda argument: kwargs
(ARG005)
106-106: Undefined name os
(F821)
106-106: Undefined name t
(F821)
120-120: Unused method argument: args
(ARG002)
120-120: Unused method argument: kwargs
(ARG002)
126-126: Unused lambda argument: charset
(ARG005)
133-133: Unused lambda argument: kwargs
(ARG005)
159-159: Unused function argument: args
(ARG001)
159-159: Unused function argument: kwargs
(ARG001)
168-168: Unused function argument: read
(ARG001)
173-173: Unused method argument: dialect
(ARG002)
188-188: Unused method argument: index
(ARG002)
241-241: Unused lambda argument: expr
(ARG005)
251-251: Unused lambda argument: expr
(ARG005)
261-261: Unused lambda argument: expr
(ARG005)
271-271: Unused function argument: cls
(ARG001)
271-271: Unused function argument: column_type
(ARG001)
271-271: Unused function argument: sqlite_json1_extension_enabled
(ARG001)
613-613: Undefined name os
(F821)
613-613: Undefined name t
(F821)
669-669: Undefined name os
(F821)
669-669: Undefined name t
(F821)
762-762: Undefined name os
(F821)
762-762: Undefined name t
(F821)
776-776: Undefined name os
(F821)
776-776: Undefined name t
(F821)
790-790: Unused method argument: sqlite_database
(ARG002)
790-790: Undefined name os
(F821)
790-790: Undefined name t
(F821)
804-804: Undefined name os
(F821)
804-804: Undefined name t
(F821)
824-824: Undefined name os
(F821)
824-824: Undefined name t
(F821)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Test (python3.10, mariadb:10.0, 1, true, 3.10)
- GitHub Check: Test (python3.13, mariadb:5.5, 1, true, 3.13)
- GitHub Check: Test (python3.11, mariadb:10.0, 1, true, 3.11)
- GitHub Check: Test (python3.12, mariadb:5.5, 1, true, 3.12)
- GitHub Check: Test (python3.9, mariadb:5.5, 1, true, 3.9)
- GitHub Check: Test (python3.9, mariadb:10.0, 1, true, 3.9)
- GitHub Check: Test (python3.11, mariadb:5.5, 1, true, 3.11)
- GitHub Check: Test (python3.10, mariadb:5.5, 1, true, 3.10)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (9)
tests/unit/test_cli_error_paths.py (1)
1-142: LGTM! CLI error path coverage is comprehensive.The test coverage for CLI error handling is well-structured. The mocking strategy using
_FakeConverterand_fake_supported_charsetsappropriately isolates the CLI validation logic from the actual converter implementation.tests/unit/test_types_fallback.py (1)
1-30: LGTM! Fallback test follows established patterns.The test correctly exercises the TypedDict fallback path by mocking the typing module and reloading the types module. The cleanup in the finally block ensures proper test isolation.
tests/unit/test_transporter.py (7)
22-43: LGTM! Fallback test correctly exercises typing.Unpack fallback.The test appropriately mocks the typing module to exercise the fallback path when
Unpackis unavailable. The cleanup ensures proper test isolation.
45-102: LGTM! Constructor test verifies UTF-8mb4 collation normalisation.The test correctly verifies that
utf8mb4_0900_ai_cidefaults are downgraded toutf8mb4_unicode_cias expected. The mocking strategy withfake_isinstanceappropriately handles the MySQLConnectionAbstract check.
104-154: LGTM! Connection validation test ensures proper error handling.The test correctly verifies that a
ConnectionErroris raised when the MySQL connection reports as disconnected. This covers an important error path in the constructor.
156-163: LGTM! Helper method tests provide thorough coverage.The tests for
_transpile_mysql_expr_to_sqlite,_transpile_mysql_type_to_sqlite,_quote_sqlite_identifier,_translate_default_from_mysql_to_sqlite, and_data_type_collation_sequencecomprehensively cover edge cases including parse errors, length preservation, non-UTF-8 bytes, charset introducers, and various default value formats.Also applies to: 165-200, 202-206, 208-266, 268-284
286-298: LGTM! Table and view creation tests cover critical error paths.The tests for index name generation, CREATE TABLE warnings, CREATE VIEW fallbacks, and reconnection logic appropriately exercise error handling and edge cases. The mocking strategy isolates the logic under test whilst simulating realistic failure scenarios.
Also applies to: 300-354, 356-388, 390-411
760-840: LGTM! Constructor validation tests ensure proper error messaging.The parameterised approach to testing missing required arguments and mutually exclusive options provides comprehensive coverage of constructor validation logic. These tests align with the learnings about raising ValueError for invalid configuration.
Based on learnings.
874-897: LGTM! Non-subscriptable row fallback test covers edge case.The test correctly exercises the fallback path in
_coerce_rowwhen encountering non-subscriptable row objects, ensuring robust handling of unexpected data structures.
This pull request adds new unit tests to improve coverage of error handling and compatibility in the CLI and types modules. The most important changes are grouped below:
CLI error handling tests
TestCliErrorPathsclass intests/unit/test_cli_error_paths.pyto verify that invalid charset/collation combinations are rejected before transfer starts, and that debug mode properly re-raisesKeyboardInterruptand generic exceptions for easier debugging and development.Types module compatibility tests
tests/unit/test_types_fallback.pyto ensure that the types module correctly falls back to usingtyping_extensions.TypedDictwhentyping.TypedDictis missing, improving compatibility with older Python versions.