test: make the Python suite green on Windows - #68
Merged
Conversation
Local runs on Windows had a standing 7-failure baseline that was green in CI. It was recorded as one problem — the missing .exe suffix — but probing each failure showed three unrelated causes, only one of which was the suffix. test_quoted_columns (5 failures) — the .exe suffix, and the only genuine instance. Note this bites solely because the file guards with `exists()`; the ~35 other files that hardcode a suffixless path work anyway, because Windows CreateProcess appends .exe itself. Fixed with the same idiom test_in_between_operators.py and tests/comparison/engines.py already carry. test_unnest — same omission, worse symptom, and not previously noticed because nothing failed: the release `exists()` check fails, the file falls through to its `target/debug` fallback, and CreateProcess then resolves that to sql-cli.exe. The tests passed while exercising the DEBUG binary. test_sql_comments (1) — not the suffix at all. NamedTemporaryFile(mode='w') writes in the platform's preferred encoding, cp1252 here, so the tests carrying scientific notation (a0, pi, epsilon0, hbar) raised UnicodeEncodeError before the CLI was invoked. Fixed in the test with encoding='utf-8' rather than by requiring PYTHONUTF8=1 in the runner. test_web_cte_advanced (1) — also not the suffix. --query-plan prints the AST and then executes the query anyway, so the test attempts a fetch it believes it is avoiding. A refused localhost connection is instant on Linux and ~2.4s on Windows, against a timeout=2. Timeout raised to 15s and the real cause recorded in place: --query-plan should not execute. Local suite now 534 passed, 0 failed, with no env-var prefix needed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBEUeckCwmWXoWTpQUTDqP
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
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.
Local runs on Windows had a standing 7-failure baseline that was green in CI. It was recorded as one problem — the missing .exe suffix — but probing each failure showed three unrelated causes, only one of which was the suffix.
test_quoted_columns (5 failures) — the .exe suffix, and the only genuine instance. Note this bites solely because the file guards with
exists(); the ~35 other files that hardcode a suffixless path work anyway, because Windows CreateProcess appends .exe itself. Fixed with the same idiom test_in_between_operators.py and tests/comparison/engines.py already carry.test_unnest — same omission, worse symptom, and not previously noticed because nothing failed: the release
exists()check fails, the file falls through to itstarget/debugfallback, and CreateProcess then resolves that to sql-cli.exe. The tests passed while exercising the DEBUG binary.test_sql_comments (1) — not the suffix at all. NamedTemporaryFile(mode='w') writes in the platform's preferred encoding, cp1252 here, so the tests carrying scientific notation (a0, pi, epsilon0, hbar) raised UnicodeEncodeError before the CLI was invoked. Fixed in the test with encoding='utf-8' rather than by requiring PYTHONUTF8=1 in the runner.
test_web_cte_advanced (1) — also not the suffix. --query-plan prints the AST and then executes the query anyway, so the test attempts a fetch it believes it is avoiding. A refused localhost connection is instant on Linux and ~2.4s on Windows, against a timeout=2. Timeout raised to 15s and the real cause recorded in place: --query-plan should not execute.
Local suite now 534 passed, 0 failed, with no env-var prefix needed.
Claude-Session: https://claude.ai/code/session_01JBEUeckCwmWXoWTpQUTDqP