diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 581b612ba4..de63b7a6a3 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -189,7 +189,7 @@ jobs: # Verify installation works across platforms after publishing test-installation: - needs: publish + needs: [build, publish] runs-on: ${{ matrix.os }} strategy: fail-fast: false @@ -214,15 +214,21 @@ jobs: - name: Test installation from PyPI if: github.event.inputs.test_pypi != 'true' + shell: bash + env: + BENCHBOX_VERSION: ${{ needs.build.outputs.version }} run: | - pip install benchbox + pip install "benchbox==${BENCHBOX_VERSION}" python -c "import importlib.util; import benchbox; assert importlib.util.find_spec('pandas') is None, 'published core wheel must not pull pandas'; print(f'BenchBox {benchbox.__version__} installed successfully (minimal import surface)')" benchbox --help - name: Test installation from Test PyPI if: github.event.inputs.test_pypi == 'true' + shell: bash + env: + BENCHBOX_VERSION: ${{ needs.build.outputs.version }} run: | - pip install -i https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple/ benchbox + pip install -i https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple/ "benchbox==${BENCHBOX_VERSION}" python -c "import importlib.util; import benchbox; assert importlib.util.find_spec('pandas') is None, 'published core wheel must not pull pandas'; print(f'BenchBox {benchbox.__version__} installed successfully (minimal import surface)')" benchbox --help diff --git a/_project/TODO/results-labels-funding/planning/explorer-pipeline-frontend-handoff.yaml b/_project/TODO/results-labels-funding/planning/explorer-pipeline-frontend-handoff.yaml index 9795ad0cae..4af10efdf8 100644 --- a/_project/TODO/results-labels-funding/planning/explorer-pipeline-frontend-handoff.yaml +++ b/_project/TODO/results-labels-funding/planning/explorer-pipeline-frontend-handoff.yaml @@ -61,7 +61,7 @@ work: (bare-column projection, G-11 rule), and add funding to the matching EXPECTED_VIEWS set in test_duckdb_browser_contract.py. No new base-table column, so no read_model_version bump. - notes: >- + DONE in PR #1080: r.funding added to result_detail_metrics in duckdb_builder.py + browser-duckdb-schema.sql; EXPECTED_VIEWS guards it. needs: [w1] diff --git a/_project/config/fast_test_lane_policy.json b/_project/config/fast_test_lane_policy.json index e9d4c7ab71..de4d50f626 100644 --- a/_project/config/fast_test_lane_policy.json +++ b/_project/config/fast_test_lane_policy.json @@ -1,7 +1,7 @@ { "enabled": true, - "max_fast_tests": 25190, - "_ceiling_note": "Bumped 13200 -> 22000 on 2026-04-13 to reflect grown fast-lane (20,184 collected). Bumped 22000 -> 22100 on 2026-05-10 for joinorder-canonical-foundation Phase-1 (data_fetch + scale-factor + surface-field tests; 22038 collected). Bumped 22100 -> 22400 on 2026-05-11 for joinorder-canonical-cutover canonical SQL/DataFrame, MCP, publishing, and corpus-inventory guards; 22317 collected. Bumped 22400 -> 22500 on 2026-05-13 for UAT enabled-platform remediation coverage; pr-preflight collected 22419. Bumped 22500 -> 22510 on 2026-05-15 after rebasing onto develop at 36ab01058; pr-preflight collected 22501 before this branch's new public theme contract tests, which are medium-marked to avoid fast-lane growth. Bumped 22510 -> 22530 on 2026-05-14 for pr-review-followup batch coverage (joinorder dataframe family, pg_mooncake adapter, landing quickstart validation, explorer read-model retry); pr-preflight collected 22520. Bumped 22530 -> 22540 on 2026-05-15 for results-explorer-post-theme-reconcile count-aware Home/script coverage; CI collected 22537. Bumped 22540 -> 22543 on 2026-05-16 for pr-review-followups coverage on UAT CLI dispatch, UAT release-gate envelope scoping, and develop-post-merge orphan ordering; ci-lint collected 22543. Bumped 22543 -> 22550 on 2026-05-16 for prompts landing Phase 1 CLI hygiene validator/prompt-shape coverage; pr-preflight collected 22550. Bumped 22550 -> 22553 on 2026-05-16 for prompts landing Phase 2 cost-class and registry prompt-safety coverage; timing policy collected 22553. Bumped 22553 -> 22558 on 2026-05-16 for prompts landing Phase 3 platform-option and TPC-DS dsdgen validator coverage; timing_policy_check collected 22558. Bumped 22558 -> 22561 on 2026-05-16 for prompts landing Phase 4 provenance template and capture-plan footer coverage; timing_policy_check collected 22561. Bumped 22561 -> 22563 on 2026-05-16 for prompts landing review fixes covering MCP cost-gating parity and compare summary discipline; targeted lane count increased by two fast tests. Bumped 22563 -> 22568 on 2026-05-16 for prompts landing Phase 5 MCP parity prompt-surface coverage; timing_policy_check collected 22568. Bumped 22568 -> 22570 on 2026-05-16 for prompts landing MCP real-tool parity and platform-option gap follow-up coverage; timing_policy_check collected 22570. Bumped 22570 -> 22572 on 2026-05-19 for pr-review-followups prompt regressions covering credential-before-smoke ordering and smoke-scale dsdgen warning behavior; timing_policy_check collected 22572. Set 22572 -> 25000 on 2026-05-19 by maintainer direction during pr-review-followups cleanup. Ratchet down if the lane contracts; never raise without justification. Bumped 25000 -> 25050 on 2026-07-16 for tuning-mode-vocabulary-and-facet-implementation-20260712 (ADR-2 canonical_mode/tuned-fallback/official-refusal coverage, the PACKAGED_RESOURCE composition pin added when merging develop's #1188 packaged-template tier, and the physical_mechanisms unknown-vs-empty ingest-pipeline regression tests); develop alone collected 24995, this branch's merge collected 25022. Bumped 25050 -> 25060 on 2026-07-17 for tuning-capability-registry-coverage-20260716 (alias-resolution invariant tests and derived generator-coverage drift guards for the 9 newly-registered platforms); CI collected 25052 on the merge ref. Bumped 25060 -> 25080 on 2026-07-17 composing #1198's registry-coverage guards with #1176's provenance/hash export coverage (test_tuning_provenance_export.py + test_requested_config_hash.py); both branches independently bumped from 25050 (#1198: 25052 collected; #1176 pre-compose: 25062 collected); composed merge tree collects 25077. Bumped 25080 -> 25140 on 2026-07-17 for tuning-from-config-forwarding-sweep-20260716 (test_tuning_config_forwarding.py's registry-driven parametrized test, one case per registered platform adapter, verifying from_config forwards tuning_enabled/tuning_config/unified_tuning_configuration/tuning_source/tuning_source_file); timing_policy_check collected 25128. Bumped 25140 -> 25180 on 2026-07-18 for the todo-db-tracker local-SQLite spike (tests/unit/scripts/test_todo_db.py: 49 fast unit tests covering the enforced lifecycle invariants, archive import, and concurrency guards); develop collected 25128, this branch collects 25177. Bumped 25180 -> 25190 on 2026-07-18 composing the todo-db spike branch with develop's post-25128 growth: the branch alone collects 25177 (49 fast tracker tests; the 11 wrapper tests are medium-marked and collect 0 under -m fast), but the PR merge ref collects 25187.", + "max_fast_tests": 25220, + "_ceiling_note": "Bumped 13200 -> 22000 on 2026-04-13 to reflect grown fast-lane (20,184 collected). Bumped 22000 -> 22100 on 2026-05-10 for joinorder-canonical-foundation Phase-1 (data_fetch + scale-factor + surface-field tests; 22038 collected). Bumped 22100 -> 22400 on 2026-05-11 for joinorder-canonical-cutover canonical SQL/DataFrame, MCP, publishing, and corpus-inventory guards; 22317 collected. Bumped 22400 -> 22500 on 2026-05-13 for UAT enabled-platform remediation coverage; pr-preflight collected 22419. Bumped 22500 -> 22510 on 2026-05-15 after rebasing onto develop at 36ab01058; pr-preflight collected 22501 before this branch's new public theme contract tests, which are medium-marked to avoid fast-lane growth. Bumped 22510 -> 22530 on 2026-05-14 for pr-review-followup batch coverage (joinorder dataframe family, pg_mooncake adapter, landing quickstart validation, explorer read-model retry); pr-preflight collected 22520. Bumped 22530 -> 22540 on 2026-05-15 for results-explorer-post-theme-reconcile count-aware Home/script coverage; CI collected 22537. Bumped 22540 -> 22543 on 2026-05-16 for pr-review-followups coverage on UAT CLI dispatch, UAT release-gate envelope scoping, and develop-post-merge orphan ordering; ci-lint collected 22543. Bumped 22543 -> 22550 on 2026-05-16 for prompts landing Phase 1 CLI hygiene validator/prompt-shape coverage; pr-preflight collected 22550. Bumped 22550 -> 22553 on 2026-05-16 for prompts landing Phase 2 cost-class and registry prompt-safety coverage; timing policy collected 22553. Bumped 22553 -> 22558 on 2026-05-16 for prompts landing Phase 3 platform-option and TPC-DS dsdgen validator coverage; timing_policy_check collected 22558. Bumped 22558 -> 22561 on 2026-05-16 for prompts landing Phase 4 provenance template and capture-plan footer coverage; timing_policy_check collected 22561. Bumped 22561 -> 22563 on 2026-05-16 for prompts landing review fixes covering MCP cost-gating parity and compare summary discipline; targeted lane count increased by two fast tests. Bumped 22563 -> 22568 on 2026-05-16 for prompts landing Phase 5 MCP parity prompt-surface coverage; timing_policy_check collected 22568. Bumped 22568 -> 22570 on 2026-05-16 for prompts landing MCP real-tool parity and platform-option gap follow-up coverage; timing_policy_check collected 22570. Bumped 22570 -> 22572 on 2026-05-19 for pr-review-followups prompt regressions covering credential-before-smoke ordering and smoke-scale dsdgen warning behavior; timing_policy_check collected 22572. Set 22572 -> 25000 on 2026-05-19 by maintainer direction during pr-review-followups cleanup. Ratchet down if the lane contracts; never raise without justification. Bumped 25000 -> 25050 on 2026-07-16 for tuning-mode-vocabulary-and-facet-implementation-20260712 (ADR-2 canonical_mode/tuned-fallback/official-refusal coverage, the PACKAGED_RESOURCE composition pin added when merging develop's #1188 packaged-template tier, and the physical_mechanisms unknown-vs-empty ingest-pipeline regression tests); develop alone collected 24995, this branch's merge collected 25022. Bumped 25050 -> 25060 on 2026-07-17 for tuning-capability-registry-coverage-20260716 (alias-resolution invariant tests and derived generator-coverage drift guards for the 9 newly-registered platforms); CI collected 25052 on the merge ref. Bumped 25060 -> 25080 on 2026-07-17 composing #1198's registry-coverage guards with #1176's provenance/hash export coverage (test_tuning_provenance_export.py + test_requested_config_hash.py); both branches independently bumped from 25050 (#1198: 25052 collected; #1176 pre-compose: 25062 collected); composed merge tree collects 25077. Bumped 25080 -> 25140 on 2026-07-17 for tuning-from-config-forwarding-sweep-20260716 (test_tuning_config_forwarding.py's registry-driven parametrized test, one case per registered platform adapter, verifying from_config forwards tuning_enabled/tuning_config/unified_tuning_configuration/tuning_source/tuning_source_file); timing_policy_check collected 25128. Bumped 25140 -> 25180 on 2026-07-18 for the todo-db-tracker local-SQLite spike (tests/unit/scripts/test_todo_db.py: 49 fast unit tests covering the enforced lifecycle invariants, archive import, and concurrency guards); develop collected 25128, this branch collects 25177. Bumped 25180 -> 25190 on 2026-07-18 composing the todo-db spike branch with develop's post-25128 growth: the branch alone collects 25177 (49 fast tracker tests; the 11 wrapper tests are medium-marked and collect 0 under -m fast), but the PR merge ref collects 25187. Bumped 25190 -> 25220 on 2026-07-18 for the post-CODEOWNERS-retirement merge queue, composed once so the queued branches carry byte-identical policy content instead of five conflicting bumps (#1142 seed-validation exclusion coverage +10; #1116 bot-finding sweep regressions +4; #1202 capability-registry/report regressions ~+8; #1206 object-schema loader regressions ~+5; #1114 branch-rename test moves +0): develop alone collects 25187; the queue's final merge ref is expected to collect ~25214.", "forbidden_marker_expressions": [ "resource_heavy", "stress", diff --git a/_project/scripts/build_joinorder_data.py b/_project/scripts/build_joinorder_data.py index 6b511f42f6..100dc0fc49 100644 --- a/_project/scripts/build_joinorder_data.py +++ b/_project/scripts/build_joinorder_data.py @@ -37,7 +37,10 @@ from pathlib import Path from typing import Any -import tomllib +try: + import tomllib +except ModuleNotFoundError: # Python 3.10: stdlib tomllib is 3.11+ + import tomli as tomllib # type: ignore[no-redef] # Shared, single-source-of-truth logical-hash algorithm. The build script runs # under the repo-root uv environment, so it imports the same primitives the diff --git a/benchbox/cli/commands/run_official.py b/benchbox/cli/commands/run_official.py index 37ad18a797..59db03a743 100644 --- a/benchbox/cli/commands/run_official.py +++ b/benchbox/cli/commands/run_official.py @@ -97,6 +97,18 @@ def run_official(ctx, benchmark, platform, scale, phases, streams, seed, output_ console.print(f"[red]Error: --streams must be a non-negative integer, got: {streams}[/red]") sys.exit(1) + # Reject an explicit --streams 1 outright rather than letting it silently + # become 2: _resolve_requested_stream_count() (benchbox/platforms/base/ + # execution.py) floors any resolved value to the TPC throughput minimum + # of 2 because there is no wire-level way to distinguish "explicitly + # requested 1" from the schema's own default of 1 at that boundary. A + # user who explicitly typed --streams 1 here would otherwise see their + # own value silently overridden with no error, so catch it at the one + # place we still know it was explicit. + if streams == 1: + console.print("[red]Error: --streams must be >= 2 (TPC throughput minimum); got: 1[/red]") + sys.exit(1) + console.print("[bold blue]TPC-Compliant Official Benchmark Run[/bold blue]") for label, value in [ ("Benchmark", f"TPC-{benchmark.upper()}"), diff --git a/benchbox/core/tpch/generator.py b/benchbox/core/tpch/generator.py index d826aa2058..2186819317 100644 --- a/benchbox/core/tpch/generator.py +++ b/benchbox/core/tpch/generator.py @@ -45,6 +45,87 @@ def _load_generator_specs() -> dict[str, Any]: _TPCH_TABLE_CODES = dict(_GENERATOR_SPECS["table_codes"]) _TPCH_BASE_ROW_COUNTS = dict(_GENERATOR_SPECS["base_row_counts"]) +# Chunk size for the streaming trailing-delimiter rewrite (bytes). +_NORMALIZE_CHUNK_SIZE = 1 << 20 + + +def _has_trailing_delimiter(path: Path) -> bool: + """Probe the last bytes of ``path`` for a classic dbgen trailing delimiter. + + dbgen row framing is decided at compile time (EOL_HANDLING), so every row + in a file shares the same framing; inspecting the file tail is sufficient + to classify the whole file. + """ + size = path.stat().st_size + if size == 0: + return False + with path.open("rb") as handle: + handle.seek(max(0, size - 2)) + tail = handle.read() + return tail.endswith((b"|\n", b"|")) + + +def normalize_tbl_trailing_delimiters(path: Path) -> bool: + """Strip exactly one trailing ``|`` before each newline, rewriting in place. + + The bundled dbgen binaries are all compiled with -DEOL_HANDLING (#1069) + and emit no trailing delimiter, so this is normally an O(1) tail-probe + no-op. It remains a safety net for classic dbgen file-mode output (a + stale locally-built binary predating that CFLAGS fix, or any source + compile on a platform outside the precompiled matrix): TPCCompiler's + ``needs_compilation()`` only checks whether a binary file already exists, + with no staleness/CFLAGS check against current source, so a pre-existing + ``_sources/tpc-h/dbgen/dbgen`` is reused as-is and never recompiled. + + Only the delimiter immediately before the line terminator is removed; + interior empty fields are preserved. In ``.tbl`` format a trailing empty + field is a delimiter artifact, not a NULL (see + :func:`benchbox.utils.file_format.get_data_extension`). + + The rewrite streams fixed-size chunks through a temporary file, so large + scale-factor outputs are never loaded into memory. Files that are already + clean are detected with an O(1) tail probe and left untouched, which also + makes the operation idempotent. + + Args: + path: Uncompressed ``.tbl`` (or ``.tbl.N`` chunk) file to normalize. + + Returns: + True if the file was rewritten, False if it was already clean. + """ + if not _has_trailing_delimiter(path): + return False + + tmp_path = path.with_name(path.name + ".normalize.tmp") + try: + with path.open("rb") as src, tmp_path.open("wb") as dst: + carry = b"" + while True: + chunk = src.read(_NORMALIZE_CHUNK_SIZE) + if not chunk: + break + data = carry + chunk + # Hold back a chunk-final "|": it may be followed by "\n" in + # the next chunk and must be stripped together with it. + if data.endswith(b"|"): + carry = b"|" + data = data[:-1] + else: + carry = b"" + # "\n" only occurs at row boundaries, so "|\n" is always the + # trailing delimiter; str.replace removes exactly one "|" per + # newline and never touches interior empty fields. + dst.write(data.replace(b"|\n", b"\n")) + # A held-back "|" at EOF means the final row is unterminated but + # still carries the trailing delimiter; drop it. + with contextlib.suppress(OSError): + shutil.copystat(path, tmp_path) + os.replace(tmp_path, path) + finally: + with contextlib.suppress(OSError): + tmp_path.unlink(missing_ok=True) + return True + class TPCHDataGenerator(CompressionMixin, CloudStorageGeneratorMixin, VerbosityMixin): """TPC-H data generator. @@ -974,6 +1055,16 @@ def _finalize_generation(self, target_dir: Path) -> dict[str, Path | list[Path]] """ table_paths, precompressed_tables = self._gather_generated_table_paths(target_dir) + # Classic (non -z) dbgen writes a trailing field delimiter on every + # row. Normalize file-mode output before any compression or data + # organization so results match the streaming path byte-for-byte. + # The bundled binaries are clean (#1069), so this is normally an O(1) + # no-op; it remains a safety net for a stale locally-built dbgen that + # predates -DEOL_HANDLING (needs_compilation() has no staleness + # check, so an existing binary is never recompiled - see + # normalize_tbl_trailing_delimiters's docstring). + self._normalize_table_delimiters(table_paths, precompressed_tables) + # Data organization is a post-generation output mode. When configured, # prefer organized outputs over raw compression artifacts. if self._data_organization_config is not None: @@ -995,6 +1086,25 @@ def _finalize_generation(self, target_dir: Path) -> dict[str, Path | list[Path]] self._write_manifest(target_dir, table_paths) return table_paths + def _normalize_table_delimiters( + self, + table_paths: dict[str, Path | list[Path]], + precompressed_tables: set[str], + ) -> None: + """Strip classic dbgen trailing row delimiters from uncompressed outputs. + + Covers both single-file mode (``customer.tbl``) and parallel chunked + mode (``customer.tbl.1`` ... ``customer.tbl.N``). Pre-compressed + tables are skipped: they come from the streaming (-z) path or a + previous run and are already normalized. + """ + for table_name, paths in table_paths.items(): + if table_name in precompressed_tables: + continue + for file_path in paths if isinstance(paths, list) else [paths]: + if normalize_tbl_trailing_delimiters(file_path): + self.log_verbose(f"Stripped trailing row delimiters from {file_path.name}") + def _apply_data_organization( self, target_dir: Path, diff --git a/benchbox/experimental/__init__.py b/benchbox/experimental/__init__.py index 96a3d4761e..40bfce03a3 100644 --- a/benchbox/experimental/__init__.py +++ b/benchbox/experimental/__init__.py @@ -11,5 +11,5 @@ - ``aiml_functions`` - AI/ML SQL function benchmarks - ``multiregion`` - Multi-region latency and orchestration - ``gpu`` - GPU detection, metrics, and benchmark primitives -- ``concurrency`` - Concurrent load testing patterns +- ``load_testing`` - Concurrent load testing patterns """ diff --git a/benchbox/platforms/ducklake.py b/benchbox/platforms/ducklake.py index b402c96104..a7ac0744b1 100644 --- a/benchbox/platforms/ducklake.py +++ b/benchbox/platforms/ducklake.py @@ -246,8 +246,8 @@ def _resolve_option(key: str, default: Any = None) -> Any: adapter_config: dict[str, Any] = {} - metadata_path = config.get("ducklake_metadata_path") or config.get("metadata_path") - data_path = config.get("ducklake_data_path") or config.get("data_path") + metadata_path = config.get("ducklake_metadata_path") or _resolve_option("metadata_path") + data_path = config.get("ducklake_data_path") or _resolve_option("data_path") if not metadata_path or not data_path: if config.get("output_dir"): @@ -280,7 +280,13 @@ def _resolve_option(key: str, default: Any = None) -> Any: # Pass through DuckDB-family configuration (base DuckDB connection # settings for the "shell" connection the DuckLake catalog attaches to). adapter_config["memory_limit"] = config.get("memory_limit", "4GB") - adapter_config["force_recreate"] = config.get("force", False) + # "force" (flat) is the legacy config_utils.py build_config() shape; + # "force_recreate" (flat or nested in config["options"]) is what the + # real --force CLI flag actually threads through as via the modern + # PlatformHookRegistry default config builder (see _resolve_option's + # docstring above for why nested lookups are needed at all). Check + # both so a real --force request is never silently dropped to False. + adapter_config["force_recreate"] = bool(config.get("force") or _resolve_option("force_recreate", False)) for key in [ "tuning_config", "tuning_enabled", diff --git a/tests/integration/test_ducklake_integration.py b/tests/integration/test_ducklake_integration.py index 33c6f9f6a7..904f8edd02 100644 --- a/tests/integration/test_ducklake_integration.py +++ b/tests/integration/test_ducklake_integration.py @@ -61,15 +61,22 @@ # Extension-availability probes # ============================================================================= # Mirrors tests/integration/test_open_table_formats.py's TestDuckLakeFormatSmoke -# guard (and tests/unit/platforms/test_ducklake_adapter.py's DUCKLAKE_AVAILABLE -# probe): INSTALL/LOAD an extension so the tests below can skip cleanly - never +# guard: INSTALL/LOAD an extension so the tests below can skip cleanly - never # fail - when the environment cannot reach the DuckDB extension repository or # lacks a given extension. # -# Only the extensions the default lane actually needs (ducklake, sqlite) are -# probed at import time. The postgres/httpfs probes are deferred into their -# live_integration classes, which are deselected by default - probing them -# eagerly would spend a network INSTALL attempt on every collection. +# The probe must stay LAZY (called from setup_method, never at module level): +# pytest imports every collected module to discover markers/tests regardless +# of which markers are later deselected (module-level pytestmark = [slow] only +# excludes these tests from EXECUTION, not from collection/import). A +# module-level probe call, or a `@pytest.mark.skipif(not , ...)` +# class decorator whose condition captures that eager flag, would therefore +# still spend a real, potentially-networked INSTALL/LOAD attempt on every test +# collection - including a fast/offline lane that will never run any test in +# this file (#1082 review: this is exactly what test_ducklake_adapter.py's own +# hermetic unit module avoids by never touching a live DuckDB connection). +# setup_method only runs for a test pytest has already decided to execute, so +# deferring the probe there means an excluded/deselected run never pays for it. @cache @@ -94,25 +101,25 @@ def _skip_unless_extensions(*names: str) -> None: pytest.skip(f"DuckDB extension(s) not available: {', '.join(missing)}") -DUCKLAKE_AVAILABLE = _probe_extension("ducklake") -SQLITE_EXTENSION_AVAILABLE = _probe_extension("sqlite") - - # ============================================================================= # 1. Live connection - in-process DuckDB-file catalog ATTACH # ============================================================================= @pytest.mark.live_integration -@pytest.mark.skipif(not DUCKLAKE_AVAILABLE, reason="DuckLake extension not available (needs DuckDB>=1.3 + network)") class TestDuckLakeLiveConnection: """End-to-end tests exercising a real INSTALL/LOAD/ATTACH of the ducklake extension. Marked live_integration (deselected from the default/fast lane) because they perform a real, potentially-networked INSTALL ducklake; they also skip - entirely when the extension cannot be installed/loaded. + entirely when the extension cannot be installed/loaded (checked in + setup_method, not a collection-time skipif - see the probe docstring + above for why). """ + def setup_method(self) -> None: + _skip_unless_extensions("ducklake") + def test_cursor_defaults_to_lake_catalog(self, tmp_path): # Regression for the w8 validation bug: framework seams (e.g. # phase_tracking._validate_data_integrity) probe tables via @@ -291,13 +298,12 @@ def test_sqlite_catalog_end_to_end(self, tmp_path): # ============================================================================= -@pytest.mark.skipif( - not (DUCKLAKE_AVAILABLE and SQLITE_EXTENSION_AVAILABLE), - reason="ducklake/sqlite DuckDB extensions not available", -) class TestDuckLakeSqliteCatalogLive: """Real end-to-end TPC-H run through the DuckLake adapter with catalog=sqlite.""" + def setup_method(self) -> None: + _skip_unless_extensions("ducklake", "sqlite") + def test_tpch_sf001_sqlite_catalog_end_to_end(self, tmp_path: Path) -> None: """Run TPC-H SF 0.01 via `benchbox run` with --platform-option catalog=sqlite. diff --git a/tests/integration/test_throughput_real_duckdb.py b/tests/integration/test_throughput_real_duckdb.py index 7fb03041ff..2b701accae 100644 --- a/tests/integration/test_throughput_real_duckdb.py +++ b/tests/integration/test_throughput_real_duckdb.py @@ -36,7 +36,7 @@ from __future__ import annotations -from collections import defaultdict +from collections import Counter, defaultdict from collections.abc import Generator from pathlib import Path from typing import Any @@ -44,6 +44,7 @@ import pytest from benchbox.core.tpcds.benchmark import TPCDSBenchmark +from benchbox.core.tpcds.streams import MULTI_PART_QUERY_IDS from benchbox.core.tpch.benchmark import TPCHBenchmark from benchbox.core.tpch.streams import TPCHStreams from benchbox.platforms.duckdb import DuckDBAdapter @@ -72,6 +73,20 @@ # from this test's own run output), so it can safely serve as an independent # expected-count oracle. _TPCDS_QUERIES_PER_STREAM = 103 +# The exact multiset of query_id strings every stream must produce, derived +# independently from the same seed-invariant properties as +# _TPCDS_QUERIES_PER_STREAM above (query range 1-99, MULTI_PART_QUERY_IDS +# expand into "a"/"b" instead of the plain "" - see +# benchbox/core/tpcds/throughput_test.py's query_display_id construction). +# A stream that duplicates one query while omitting another, or collapses a +# variant pair like "14a"/"14b" into the same id, keeps the same length but +# changes this multiset - which len(...) == _TPCDS_QUERIES_PER_STREAM alone +# cannot detect. +_TPCDS_EXPECTED_QUERY_IDS = Counter( + f"{query_id}{variant}" if variant else str(query_id) + for query_id in range(1, 100) + for variant in (["a", "b"] if query_id in MULTI_PART_QUERY_IDS else [None]) +) # Shared base seed for both throughput runs below. _BASE_SEED = 7 @@ -342,13 +357,20 @@ def test_no_cross_stream_result_bleed(self, tpcds_throughput_run: tuple[list[dic assert set(by_stream.keys()) == set(range(_TPCDS_NUM_STREAMS)) - # Each stream must have run the expected, seed-invariant TPC-DS query - # count - independent of what the flattened rows/stream_results - # themselves report. + # Each stream must have run the exact expected, seed-invariant TPC-DS + # query multiset - not just the right count. A stream that duplicates + # one query while omitting another, or collapses a variant pair like + # "14a"/"14b" into the same id, keeps len(query_ids) == 103 but + # changes which ids are present; Counter equality catches that. for stream_id, query_ids in by_stream.items(): assert len(query_ids) == _TPCDS_QUERIES_PER_STREAM, ( f"stream {stream_id} ran {len(query_ids)} queries, expected {_TPCDS_QUERIES_PER_STREAM}" ) + assert Counter(query_ids) == _TPCDS_EXPECTED_QUERY_IDS, ( + f"stream {stream_id} ran the wrong query/variant multiset: " + f"missing={_TPCDS_EXPECTED_QUERY_IDS - Counter(query_ids)}, " + f"unexpected={Counter(query_ids) - _TPCDS_EXPECTED_QUERY_IDS}" + ) # Structured stream_results must agree with the flattened adapter # rows on the expected per-stream count too. diff --git a/tests/integration/test_throughput_stream_count.py b/tests/integration/test_throughput_stream_count.py index d5bddd95f4..56f6834fa7 100644 --- a/tests/integration/test_throughput_stream_count.py +++ b/tests/integration/test_throughput_stream_count.py @@ -290,4 +290,25 @@ def test_run_official_forward_requested_streams_noop_when_not_requested(): original = BenchmarkOrchestrator.execute_benchmark with _forward_requested_streams(None): assert BenchmarkOrchestrator.execute_benchmark is original - assert BenchmarkOrchestrator.execute_benchmark is original + + +def test_run_official_rejects_explicit_streams_one(): + """#1077 review: an explicit --streams 1 must error, not be silently + floored to 2. _resolve_requested_stream_count() floors any resolved + value to the TPC throughput minimum because it cannot distinguish an + explicit user request of 1 from the schema's own default of 1 at that + deep boundary -- so run-official (the one place the request is still + known to be explicit) must reject it up front instead of accepting it + and silently running a different stream count than requested.""" + from click.testing import CliRunner + + from benchbox.cli.commands.run_official import run_official + + runner = CliRunner() + result = runner.invoke( + run_official, + ["tpch", "--platform", "duckdb", "--scale", "1", "--phases", "throughput", "--streams", "1", "--seed", "42"], + ) + + assert result.exit_code != 0 + assert "streams must be >= 2" in result.output diff --git a/tests/uat/container_cleanup.py b/tests/uat/container_cleanup.py index 2d28656c45..52acaaa4e6 100644 --- a/tests/uat/container_cleanup.py +++ b/tests/uat/container_cleanup.py @@ -369,10 +369,16 @@ def _list_images(run: ContainerRunner, project_prefix: str) -> list[ContainerRes rows = _run_json_array(run, [CONTAINER_BIN, "image", "ls", "--format", "json"]) out: list[ContainerResource] = [] for row in rows: + row = row if isinstance(row, dict) else {} config = row.get("configuration") if isinstance(row, dict) else None config = config if isinstance(config, dict) else {} - name = str(config.get("name") or "") - identifier = str(row.get("id") or "") + # Current `container image ls --format json` renders ImageResource rows + # with the reference at the top-level `displayReference` field (see + # upstream ImageList.swift); `configuration.name` is empty on those + # rows. Fall back to `configuration.name` for older/other CLI versions + # that still populate it there. + name = str(row.get("displayReference") or config.get("name") or "") + identifier = str(row.get("id") or row.get("digest") or "") created_at = str(config.get("creationDate") or "") category: ContainerCategory if _BUILDER_IMAGE_MARKER in name: diff --git a/tests/uat/test_container_cleanup.py b/tests/uat/test_container_cleanup.py index 3b3e9ae065..3ab8dcf7ef 100644 --- a/tests/uat/test_container_cleanup.py +++ b/tests/uat/test_container_cleanup.py @@ -208,6 +208,78 @@ def test_report_renders_mode_and_store_path(): assert "Retained (widen --mode to reclaim):" in rendered +def test_custom_project_prefix_reaches_classification(): + """#1065 review: --prefix must actually classify resources, not just be + echoed in the report. _inventory_resources/_list_images/_list_containers/ + _list_volumes previously ignored the caller's project_prefix and always + classified against the hard-coded DEFAULT_UAT_PROJECT_PREFIX.""" + custom_images = [ + {"id": "img-custom", "configuration": {"name": "foo-tpc-h:latest", "creationDate": "2026-07-08T00:00:00Z"}}, + ] + custom_containers = [ + { + "id": "custom-leftover", + "configuration": { + "id": "custom-leftover", + "image": {"reference": "postgres:18"}, + "labels": {"com.docker.compose.project": "foo-smoke-postgresql"}, + }, + "status": {"state": "stopped"}, + }, + ] + + def fake(argv, **kwargs): + argv_tuple = tuple(argv) + if argv_tuple[:4] == ("container", "image", "ls", "--format"): + return docker_assets.DockerCommandResult(argv_tuple, 0, json.dumps(custom_images), "") + if argv_tuple[:3] == ("container", "ls", "-a"): + return docker_assets.DockerCommandResult(argv_tuple, 0, json.dumps(custom_containers), "") + if argv_tuple[:4] == ("container", "volume", "ls", "--format"): + return docker_assets.DockerCommandResult(argv_tuple, 0, "[]", "") + if argv_tuple == ("container", "system", "df"): + return docker_assets.DockerCommandResult(argv_tuple, 0, _SYSTEM_DF, "") + return docker_assets.DockerCommandResult(argv_tuple, 0, "", "") + + report = container_cleanup.reclaim_container_usage(mode="owned", apply=False, project_prefix="foo", runner=fake) + assert _targets_by_kind(report) == { + "image": {"foo-tpc-h:latest"}, + "container": {"custom-leftover"}, + } + + +def test_image_reference_read_from_display_reference_field(): + """#1065 review: current `container image ls --format json` renders + ImageResource rows with the reference at the top-level `displayReference` + field, not `configuration.name` (which is empty on those rows).""" + images = [ + { + "id": "sha256:abc123", + "displayReference": "benchbox/tpc-h-linux-arm64:latest", + "configuration": {"descriptor": {"digest": "sha256:abc123"}}, + }, + { + "id": "sha256:def456", + "displayReference": "postgres:18", + "configuration": {"descriptor": {"digest": "sha256:def456"}}, + }, + ] + + def fake(argv, **kwargs): + argv_tuple = tuple(argv) + if argv_tuple[:4] == ("container", "image", "ls", "--format"): + return docker_assets.DockerCommandResult(argv_tuple, 0, json.dumps(images), "") + if argv_tuple[:3] == ("container", "ls", "-a"): + return docker_assets.DockerCommandResult(argv_tuple, 0, "[]", "") + if argv_tuple[:4] == ("container", "volume", "ls", "--format"): + return docker_assets.DockerCommandResult(argv_tuple, 0, "[]", "") + if argv_tuple == ("container", "system", "df"): + return docker_assets.DockerCommandResult(argv_tuple, 0, _SYSTEM_DF, "") + return docker_assets.DockerCommandResult(argv_tuple, 0, "", "") + + report = container_cleanup.reclaim_container_usage(mode="owned", apply=False, runner=fake) + assert _targets_by_kind(report) == {"image": {"benchbox/tpc-h-linux-arm64:latest"}} + + # -------------------------------------------------------------------------- # uat-container-engine-routing w4/w5/w6 regression coverage # -------------------------------------------------------------------------- diff --git a/tests/unit/cli/test_cli_package_exports.py b/tests/unit/cli/test_cli_package_exports.py index c2c1260899..eed5fec16c 100644 --- a/tests/unit/cli/test_cli_package_exports.py +++ b/tests/unit/cli/test_cli_package_exports.py @@ -39,7 +39,7 @@ def test_public_reexports_accessible_from_benchbox(symbol: str, expected_type: s assert symbol in benchbox.__all__ -_EXPERIMENTAL_SUBSYSTEMS = {"nl2sql", "aiml_functions", "multiregion", "gpu", "concurrency"} +_EXPERIMENTAL_SUBSYSTEMS = {"nl2sql", "aiml_functions", "multiregion", "gpu", "load_testing"} def test_experimental_modules_not_in_benchmark_registry() -> None: diff --git a/tests/unit/core/tpch/test_tpch_trailing_delimiter_normalization.py b/tests/unit/core/tpch/test_tpch_trailing_delimiter_normalization.py new file mode 100644 index 0000000000..f94beb2d48 --- /dev/null +++ b/tests/unit/core/tpch/test_tpch_trailing_delimiter_normalization.py @@ -0,0 +1,108 @@ +"""Fast guard: TPC-H file-mode output has its trailing delimiter stripped. + +The bundled dbgen binaries are compiled with -DEOL_HANDLING (#1069) and emit +no trailing field separator, but TPCCompiler.needs_compilation() only checks +whether a binary file already exists -- it has no staleness/CFLAGS check +against the current source -- so a pre-existing, locally-built +``_sources/tpc-h/dbgen/dbgen`` predating that fix (or a source compile on a +platform outside the precompiled matrix) is reused as-is and would silently +reintroduce the trailing pipe without this normalization safety net (#1069 +review). See ``normalize_tbl_trailing_delimiters`` in generator.py. + +Copyright 2026 Joe Harris / BenchBox Project + +Licensed under the MIT License. See LICENSE file in the project root for details. +""" + +from __future__ import annotations + +from pathlib import Path +from unittest.mock import MagicMock + +import pytest + +from benchbox.core.tpch.generator import TPCHDataGenerator, normalize_tbl_trailing_delimiters + +pytestmark = [ + pytest.mark.unit, + pytest.mark.fast, +] + + +def test_strips_exactly_one_trailing_pipe_per_row(tmp_path: Path) -> None: + path = tmp_path / "customer.tbl" + path.write_text("1|Customer#1|BUILDING|comment.|\n2|Customer#2|AUTOMOBILE|comment2.|\n", encoding="utf-8") + + rewritten = normalize_tbl_trailing_delimiters(path) + + assert rewritten is True + assert path.read_text(encoding="utf-8") == "1|Customer#1|BUILDING|comment.\n2|Customer#2|AUTOMOBILE|comment2.\n" + + +def test_interior_empty_fields_are_preserved(tmp_path: Path) -> None: + path = tmp_path / "orders.tbl" + path.write_text("1||BUILDING||comment.|\n", encoding="utf-8") + + normalize_tbl_trailing_delimiters(path) + + assert path.read_text(encoding="utf-8") == "1||BUILDING||comment.\n" + + +def test_already_clean_file_is_left_untouched(tmp_path: Path) -> None: + path = tmp_path / "nation.tbl" + original = "0|ALGERIA|0|comment.\n1|ARGENTINA|1|comment2.\n" + path.write_text(original, encoding="utf-8") + mtime_before = path.stat().st_mtime_ns + + rewritten = normalize_tbl_trailing_delimiters(path) + + assert rewritten is False + assert path.read_text(encoding="utf-8") == original + assert path.stat().st_mtime_ns == mtime_before + + +def test_idempotent_on_repeated_calls(tmp_path: Path) -> None: + path = tmp_path / "region.tbl" + path.write_text("0|AFRICA|comment.|\n", encoding="utf-8") + + first = normalize_tbl_trailing_delimiters(path) + second = normalize_tbl_trailing_delimiters(path) + + assert first is True + assert second is False + assert path.read_text(encoding="utf-8") == "0|AFRICA|comment.\n" + + +def test_chunk_boundary_carry_does_not_corrupt_rows(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """A trailing '|' that lands exactly at a chunk boundary must still be + stripped, not left dangling or merged into the next row.""" + import benchbox.core.tpch.generator as generator_module + + monkeypatch.setattr(generator_module, "_NORMALIZE_CHUNK_SIZE", 5) + path = tmp_path / "supplier.tbl.1" + path.write_text("1|AB|\n2|CD|\n", encoding="utf-8") + + normalize_tbl_trailing_delimiters(path) + + assert path.read_text(encoding="utf-8") == "1|AB\n2|CD\n" + + +def test_finalize_generation_normalizes_fallback_file_mode_output(tmp_path: Path) -> None: + """#1069 review: _finalize_generation must still normalize file-mode + output for the fallback path (a stale, non-bundled dbgen build that + predates -DEOL_HANDLING), not just trust that all reachable binaries are + already clean.""" + dirty_path = tmp_path / "customer.tbl" + dirty_path.write_text("1|Customer#1|BUILDING|comment.|\n", encoding="utf-8") + + gen = TPCHDataGenerator.__new__(TPCHDataGenerator) + gen._data_organization_config = None + gen.verbose_enabled = False + gen.should_use_compression = MagicMock(return_value=False) + gen._write_manifest = MagicMock() + gen._gather_generated_table_paths = MagicMock(return_value=({"customer": dirty_path}, set())) + gen.log_verbose = MagicMock() + + gen._finalize_generation(tmp_path) + + assert dirty_path.read_text(encoding="utf-8") == "1|Customer#1|BUILDING|comment.\n" diff --git a/tests/unit/platforms/test_ducklake_adapter.py b/tests/unit/platforms/test_ducklake_adapter.py index 737475c513..e59cd6e466 100644 --- a/tests/unit/platforms/test_ducklake_adapter.py +++ b/tests/unit/platforms/test_ducklake_adapter.py @@ -127,6 +127,26 @@ def test_argparse_style_key_takes_precedence_over_normalized(self, tmp_path): assert adapter.metadata_path == preferred + def test_honors_metadata_and_data_path_from_nested_options(self, tmp_path): + """#1082 review: benchbox run --platform ducklake --platform-option + metadata_path=... --platform-option data_path=... nests those values + under config["options"] (DuckLake has no registered PlatformHookRegistry + config builder to promote them to top-level flat keys), but the old + code only ever read the flat config.get("metadata_path")/ + ("data_path"), so a real CLI-supplied catalog/data location was + silently ignored and the generated-default paths were used instead.""" + metadata_path = tmp_path / "nested" / "catalog.ducklake" + data_path = tmp_path / "nested" / "data" + config = { + "benchmark": "tpch", + "scale_factor": 0.01, + "options": {"metadata_path": str(metadata_path), "data_path": str(data_path)}, + } + adapter = DuckLakeAdapter.from_config(config) + + assert adapter.metadata_path == metadata_path + assert adapter.data_path == data_path + class TestDuckLakeAdapterBasics: """Test basic adapter properties and construction.""" @@ -498,6 +518,40 @@ def test_resolves_s3_creds_from_nested_options(self, tmp_path): assert adapter.s3_secret == "shh" assert adapter.s3_region == "us-east-1" + def test_resolves_force_recreate_from_nested_options(self, tmp_path): + """#1082 review: the real --force CLI flag threads through as + force_recreate nested in config["options"] (via PlatformHookRegistry's + default config builder), not as a flat "force" key. The old code only + read config.get("force", False), so a real --force request was + silently reset to False and an existing catalog would be reused + instead of wiped for a clean rebuild.""" + config = { + "benchmark": "tpch", + "scale_factor": 0.01, + "output_dir": str(tmp_path), + "options": {"force_recreate": True}, + } + adapter = DuckLakeAdapter.from_config(config) + assert adapter.force_recreate is True + + def test_resolves_force_recreate_from_legacy_flat_force_key(self, tmp_path): + """Back-compat: the legacy config_utils.py build_config() path passes + a flat "force" key (not "force_recreate") - must keep working + alongside the nested force_recreate resolution above.""" + config = { + "benchmark": "tpch", + "scale_factor": 0.01, + "output_dir": str(tmp_path), + "force": True, + } + adapter = DuckLakeAdapter.from_config(config) + assert adapter.force_recreate is True + + def test_force_recreate_defaults_to_false(self, tmp_path): + config = {"benchmark": "tpch", "scale_factor": 0.01, "output_dir": str(tmp_path)} + adapter = DuckLakeAdapter.from_config(config) + assert adapter.force_recreate is False + class TestDuckLakeS3Routing: """Test S3/cloud DATA_PATH routing and secret-SQL construction (w3).""" diff --git a/tests/unit/scripts/test_build_joinorder_data.py b/tests/unit/scripts/test_build_joinorder_data.py index ffa75be5b3..2fba6e7695 100644 --- a/tests/unit/scripts/test_build_joinorder_data.py +++ b/tests/unit/scripts/test_build_joinorder_data.py @@ -50,6 +50,19 @@ class FakeDatetimeModule: assert build_joinorder_data.utc_now_iso() == "2026-05-11T03:22:11Z" +def test_tomllib_import_has_python310_fallback() -> None: + """#1076 review: this script requires-python >=3.10 (repo-wide), but + stdlib tomllib is 3.11+. An unconditional `import tomllib` raises + ModuleNotFoundError on 3.10; it must fall back to the tomli backport + (already a declared dependency; python_version < '3.11'), matching the + pattern used by benchbox/core/data_fetch/manifest.py and friends.""" + script_path = REPO_ROOT / "_project" / "scripts" / "build_joinorder_data.py" + text = script_path.read_text(encoding="utf-8") + assert "import tomli as tomllib" in text, ( + "build_joinorder_data.py must fall back to tomli on Python 3.10 (stdlib tomllib is 3.11+)" + ) + + def test_container_cli_defaults_to_docker(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.delenv("BENCHBOX_CONTAINER_CLI", raising=False) assert build_joinorder_data.container_cli() == "docker" diff --git a/tests/unit/test_release_infrastructure.py b/tests/unit/test_release_infrastructure.py index eab9dbd892..ee6657932b 100644 --- a/tests/unit/test_release_infrastructure.py +++ b/tests/unit/test_release_infrastructure.py @@ -245,6 +245,34 @@ def test_release_workflow_configuration(self): assert "id-token" in publish_job["permissions"] assert publish_job["permissions"]["id-token"] == "write" + def test_release_smoke_test_pins_installed_version(self): + """#1072 review: the post-publish smoke test must install the exact + version this workflow run just built, not an unqualified `benchbox` + - PyPI/Test PyPI propagation lag or a pre-existing Test PyPI version + could otherwise mask a broken current wheel.""" + jobs = _workflow("release.yml")["jobs"] + test_installation_job = jobs["test-installation"] + + needs = test_installation_job["needs"] + needs = [needs] if isinstance(needs, str) else needs + assert "build" in needs, "test-installation must depend on build to reach needs.build.outputs.version" + + release_workflow_text = (REPO_ROOT / ".github" / "workflows" / "release.yml").read_text(encoding="utf-8") + assert "pip install benchbox" not in release_workflow_text, "smoke test must not install an unpinned benchbox" + assert release_workflow_text.count("needs.build.outputs.version") >= 3, ( + "the build/publish version pin, plus both PyPI and Test PyPI install steps, must reference it" + ) + + steps = test_installation_job["steps"] + install_steps = [s for s in steps if s.get("name", "").startswith("Test installation from")] + assert len(install_steps) == 2, "expected exactly the PyPI and Test PyPI install steps" + for step in install_steps: + assert step.get("shell") == "bash", ( + f"{step['name']!r} must pin shell: bash - the matrix includes windows-latest, whose " + "default runner shell is PowerShell and does not understand ${BENCHBOX_VERSION} bash " + "syntax, so the Windows leg would silently install an unversioned/empty package name" + ) + def test_release_required_result_contract(self): """Test the main-PR release-required umbrella check shape.""" jobs = _workflow("test.yml")["jobs"]