From 5a78298fc5585b343d21a1730153006503a7e731 Mon Sep 17 00:00:00 2001 From: TimelordUK Date: Sun, 6 Sep 2026 11:25:02 +0100 Subject: [PATCH] test: make the Python suite green on Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Claude-Session: https://claude.ai/code/session_01JBEUeckCwmWXoWTpQUTDqP --- tests/python_tests/test_quoted_columns.py | 21 +++++++++++++++------ tests/python_tests/test_sql_comments.py | 13 ++++++++++--- tests/python_tests/test_unnest.py | 12 +++++++++--- tests/python_tests/test_web_cte_advanced.py | 13 ++++++++++--- 4 files changed, 44 insertions(+), 15 deletions(-) diff --git a/tests/python_tests/test_quoted_columns.py b/tests/python_tests/test_quoted_columns.py index 327b1c9..4b68d6d 100644 --- a/tests/python_tests/test_quoted_columns.py +++ b/tests/python_tests/test_quoted_columns.py @@ -9,10 +9,22 @@ from pathlib import Path from io import StringIO +def _sql_cli_binary(): + """Path to the release binary, with the .exe suffix Windows needs. + + Without the suffix the `exists()` check below raised FileNotFoundError and + every test in this file silently never ran on a Windows box, while passing + in CI. Same fix as tests/comparison/engines.py and + test_in_between_operators.py already carry. + """ + base_dir = Path(__file__).parent.parent.parent + suffix = ".exe" if sys.platform == "win32" else "" + return base_dir / "target" / "release" / f"sql-cli{suffix}" + + def run_query(query, data_file=None): """Execute a query and return the results.""" - base_dir = Path(__file__).parent.parent.parent - sql_cli = base_dir / "target" / "release" / "sql-cli" + sql_cli = _sql_cli_binary() if not sql_cli.exists(): raise FileNotFoundError(f"sql-cli not found at {sql_cli}") @@ -37,10 +49,7 @@ def run_query(query, data_file=None): def format_query(query): """Format a SQL query.""" - base_dir = Path(__file__).parent.parent.parent - sql_cli = base_dir / "target" / "release" / "sql-cli" - - cmd = [str(sql_cli), "--format", "-"] + cmd = [str(_sql_cli_binary()), "--format", "-"] result = subprocess.run(cmd, input=query, capture_output=True, text=True) if result.returncode != 0: diff --git a/tests/python_tests/test_sql_comments.py b/tests/python_tests/test_sql_comments.py index 049fe7f..d4a4611 100644 --- a/tests/python_tests/test_sql_comments.py +++ b/tests/python_tests/test_sql_comments.py @@ -14,7 +14,7 @@ def run_query(query, output_format="json"): """Run a SQL query and return the result""" cmd = [str(SQL_CLI), str(TEST_DATA), "-q", query, "-o", output_format] - result = subprocess.run(cmd, capture_output=True, text=True) + result = subprocess.run(cmd, capture_output=True, text=True, encoding='utf-8') if result.returncode != 0: print(f"Command failed: {' '.join(cmd)}") @@ -34,13 +34,20 @@ def run_query(query, output_format="json"): def run_query_file(sql_file_content, output_format="json"): """Run a SQL query from a file""" - with tempfile.NamedTemporaryFile(mode='w', suffix='.sql', delete=False) as f: + # encoding='utf-8' is required, not cosmetic: without it Python writes the + # temp file in the platform's preferred encoding, which is cp1252 on + # Windows, and the scientific-notation tests below (a₀, π, ε₀, ℏ) raise + # UnicodeEncodeError before the CLI is ever invoked. The CLI reads .sql + # files as UTF-8 on every platform, so this also makes the two agree. + with tempfile.NamedTemporaryFile( + mode='w', suffix='.sql', delete=False, encoding='utf-8' + ) as f: f.write(sql_file_content) temp_path = f.name try: cmd = [str(SQL_CLI), str(TEST_DATA), "-f", temp_path, "-o", output_format] - result = subprocess.run(cmd, capture_output=True, text=True) + result = subprocess.run(cmd, capture_output=True, text=True, encoding='utf-8') if result.returncode != 0: print(f"Command failed: {' '.join(cmd)}") diff --git a/tests/python_tests/test_unnest.py b/tests/python_tests/test_unnest.py index 683c135..7607219 100644 --- a/tests/python_tests/test_unnest.py +++ b/tests/python_tests/test_unnest.py @@ -7,16 +7,22 @@ import json import csv import os +import sys import tempfile import pytest from pathlib import Path -# Find the sql-cli binary +# Find the sql-cli binary. The .exe suffix is required on Windows: without it +# the release `exists()` check fails, this file quietly falls through to the +# debug path, and Windows then resolves `sql-cli` -> `sql-cli.exe` anyway — so +# the tests pass while exercising the DEBUG binary. That is worse than the +# outright failure the same omission caused in test_quoted_columns.py. PROJECT_ROOT = Path(__file__).parent.parent.parent -SQL_CLI = PROJECT_ROOT / "target" / "release" / "sql-cli" +_SUFFIX = ".exe" if sys.platform == "win32" else "" +SQL_CLI = PROJECT_ROOT / "target" / "release" / f"sql-cli{_SUFFIX}" if not SQL_CLI.exists(): - SQL_CLI = PROJECT_ROOT / "target" / "debug" / "sql-cli" + SQL_CLI = PROJECT_ROOT / "target" / "debug" / f"sql-cli{_SUFFIX}" def run_query(csv_file, query): diff --git a/tests/python_tests/test_web_cte_advanced.py b/tests/python_tests/test_web_cte_advanced.py index ec584bb..17ba916 100755 --- a/tests/python_tests/test_web_cte_advanced.py +++ b/tests/python_tests/test_web_cte_advanced.py @@ -46,13 +46,20 @@ def test_method_parsing(): ) SELECT 1 as test """ - # Just check that it parses without error by using --query-plan - # Don't actually execute the query + # Check that it parses, via --query-plan. + # + # NOTE: --query-plan prints the AST and then runs the query anyway, so + # despite the intent above this DOES attempt the fetch. On Linux the + # refused connection to a closed localhost port returns instantly; on + # Windows it takes ~2.4s, so the original timeout=2 made this the one + # test in the file that failed locally while passing in CI. The timeout + # is raised rather than tightened because the delay is the CLI's, not + # this test's. The real fix is for --query-plan not to execute. result = subprocess.run( [SQL_CLI, "-q", query, "--query-plan"], capture_output=True, text=True, - timeout=2 # Short timeout for localhost + timeout=15 ) # Check that parsing succeeded - should show the AST with method assert "WebCTESpec" in result.stdout or "method:" in result.stdout.lower(), \