fix(#23): add tests/ with unit coverage for dialect write-path hooks - #26
Open
Analitiq-Bot wants to merge 2 commits into
Open
fix(#23): add tests/ with unit coverage for dialect write-path hooks#26Analitiq-Bot wants to merge 2 commits into
Analitiq-Bot wants to merge 2 commits into
Conversation
Mirror the mysql layout: tests/conftest.py + tests/test_connector.py, with [tool.pytest.ini_options] in pyproject.toml (testpaths, pythonpath, --import-mode=importlib). Covers all items listed in #23: - stage_table_sql: temp=True/False branches; identifiers go through quote_table, not raw interpolation. - merge_statement_sql: DO UPDATE SET path; DO NOTHING degradation when every landed column is a conflict key (empty SET list is a syntax error). - build_tls_connect_arg: verify-ca/verify-full require a CA bundle; require + CA builds a verifying context (check_hostname=False); the three plaintext modes pass through as strings; any unrecognized or case-variant mode raises ValueError (security property). - Contract pin: max_identifier_length == 63 == sql_capabilities.limits.max_identifier_len. - Contract pin: empty_table_sql renders DELETE not TRUNCATE. 26 tests, all passing. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Read max_identifier_len from connector.json in the contract pin so the test actually cross-checks the dialect against the published contract (hardcoded 63 would stay green even if the two values diverged). - Add test_require_with_empty_string_ca_is_passthrough: empty string is falsy and must not trigger CA verification, same as None. - test_do_nothing_when_all_columns_are_conflict_keys: switch from substring assertions to exact string equality so INSERT/SELECT/ON CONFLICT structure is also pinned. - test_column_order_preserved: replace fragile sql.index() extraction with exact string equality; document both INSERT and SELECT order. - test_identifiers_are_quoted (stage): remove the redundant assert "DROP TABLE x" in sql line (the quoted-form assertions above already prove safe quoting). - Module docstring: drop the "no other automated coverage" qualifier (only what IS covered here can be stated confidently). - Replace non-ASCII arrow -> in comment with ASCII --. - Add comment above parametrize explaining why require is excluded. 27 tests, all passing. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Analitiq-Bot
marked this pull request as ready for review
August 2, 2026 11:56
Collaborator
Author
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds the missing
tests/directory, mirroring the mysql/mariadb/bigquery layout, to close the parity gap raised in #23. The test suite covers the three pure-Python entry points that had zero unit coverage:stage_table_sql— bothtemp=Trueandtemp=Falsebranches; verifies identifiers are quoted viaquote_tableand never interpolated raw.merge_statement_sql— theDO UPDATE SETpath for a normal upsert, and theDO NOTHINGdegradation when every landed column is a conflict key (an emptyDO UPDATE SETis a PostgreSQL syntax error, so this branch is load-bearing).build_tls_connect_arg— all seven mode paths:verify-ca/verify-fullrequire a CA bundle;require+CA builds a verifying context without hostname checking;disable/allow/prefer/require(no CA) pass through as strings; any unrecognized or case-variant mode raisesValueError(security invariant — prevents silent bypass of CA verification).Two contract pins are also added:
max_identifier_length == 63must matchsql_capabilities.limits.max_identifier_lenindefinition/connector.json.empty_table_sqlmust renderDELETE FROM, notTRUNCATE(the CDK conformance kit rejectsTRUNCATEunconditionally).pyproject.tomlgains[tool.pytest.ini_options]withtestpaths = ["tests"],pythonpath = ["."], andaddopts = "--import-mode=importlib".Test plan
26 tests collected, all passing. Every branch in the issue's "Worth covering" list is exercised by at least one test, with parametrized cases for passthrough modes and bad mode strings.
Closes #23