diff --git a/.agents/skills/sdk-ci-triage/SKILL.md b/.agents/skills/sdk-ci-triage/SKILL.md index 626ad0323..a0286d05a 100644 --- a/.agents/skills/sdk-ci-triage/SKILL.md +++ b/.agents/skills/sdk-ci-triage/SKILL.md @@ -39,7 +39,7 @@ Read when relevant: - `nox`: provider and core test matrix, sharded through `py/scripts/nox-matrix.py` - `adk-py`: reusable workflow for ADK coverage - `langchain-py`: reusable workflow for LangChain coverage -- `upload-wheel`: build wheel sanity check +- `upload-wheel`: builds the wheel and runs the `test_wheel` smoke tests against it The most common failure source is still the `nox` matrix job, but `pylint` and `test_types` failures now surface through `static_checks`, not through `nox`. @@ -204,12 +204,15 @@ Reproduce from `py/`: ```bash cd py -make install-build-deps +make install-dev make build +make test-wheel ``` Remember that build rewrites `py/src/braintrust/version.py` temporarily. +`make test-wheel` runs the `test_wheel` nox session, which runs `py/tests/wheel/test_wheel.py` against the wheel in `dist/`. It is a non-default session (`nox -l` shows it with `- `), so a bare `nox`/`make test` and the sharded `nox` matrix skip it, and it fails if `dist/` has no wheel or more than one. Colocated tests are kept out of the wheel by `py/_build_hooks.py`. If `test_does_not_ship_tests` fails, a new test-only module or data file needs excluding there or in `[tool.setuptools.*]` in `py/pyproject.toml`. If `test_all_modules_import` fails, a shipped module imports a file or module that is missing from the wheel. `braintrust.test_helpers` is deliberately shipped because downstream test suites import it. + ### Reusable workflows (`adk-py`, `langchain-py`) Read the reusable workflow file and map it back to the exact nox or make command it runs. Then reproduce that command locally rather than guessing from the job name alone. diff --git a/.agents/skills/sdk-vcr-workflows/SKILL.md b/.agents/skills/sdk-vcr-workflows/SKILL.md index 5be3298df..962fb3980 100644 --- a/.agents/skills/sdk-vcr-workflows/SKILL.md +++ b/.agents/skills/sdk-vcr-workflows/SKILL.md @@ -53,7 +53,6 @@ Current defaults: - local default: `record_mode="once"` - CI default: `record_mode="none"` -- wheel mode skips VCR-marked tests - fixtures inject dummy API keys and reset global state ### Per-version cassette directories diff --git a/.github/workflows/checks.yaml b/.github/workflows/checks.yaml index 9dbde48e0..0870fbee7 100644 --- a/.github/workflows/checks.yaml +++ b/.github/workflows/checks.yaml @@ -176,9 +176,9 @@ jobs: with: cache: true experimental: true - - name: Install build dependencies and build wheel + - name: Install build dependencies, build and test wheel run: | - mise exec -- make -C py install-build-deps build + mise exec -- make -C py install-dev build test-wheel - name: Upload wheel as artifact uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: diff --git a/AGENTS.md b/AGENTS.md index bb592589d..8da3d3dff 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -149,7 +149,7 @@ Before changing provider/integration behavior: - CI runs `pylint` and `test_types` via the dedicated `static_checks` workflow job on Ubuntu across the configured Python matrix, not inside the sharded `nox` job. - The sharded `nox` workflow excludes `pylint` and `test_types`; use `py/scripts/nox-matrix.py --exclude-session ...` when reproducing shard membership locally. - wrapper coverage is split across dedicated nox sessions by provider/version. -- `test-wheel` is a wheel sanity check and requires a built wheel first. +- `test-wheel` runs the `test_wheel` nox session (`py/tests/wheel/`) against the built wheel and requires `make build` first. Colocated tests are excluded from the wheel by `py/_build_hooks.py`. ## Type Tests @@ -204,7 +204,6 @@ Behavior from `py/src/braintrust/conftest.py`: - local default: `record_mode="once"` - CI default: `record_mode="none"` -- wheel mode skips VCR-marked tests - fixtures inject dummy API keys and reset global state Common commands: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c743c3015..8358af9eb 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -156,7 +156,6 @@ Common ones include: - dummy API key setup for VCR-backed tests - Braintrust global state reset between tests -- wheel-mode skipping for VCR tests The `memory_logger` fixture from `braintrust.test_helpers` is useful for asserting on logged spans without a real Braintrust backend. diff --git a/py/MANIFEST.in b/py/MANIFEST.in new file mode 100644 index 000000000..f937ef459 --- /dev/null +++ b/py/MANIFEST.in @@ -0,0 +1 @@ +include _build_hooks.py diff --git a/py/Makefile b/py/Makefile index 7effb88d0..3167e6fa1 100644 --- a/py/Makefile +++ b/py/Makefile @@ -19,13 +19,11 @@ test: uv run nox -x test-wheel: - # This target runs a small set of sanity checks before we release our - # build artifact to pypi. We skip running the full test suite because - # (a) theoretically it should have been run before (b) it has - # integration tests and we don't want to block the release because - # some service is down, etc. This decision could be revisited anytime. + # Sanity checks (tests/wheel) against the built wheel before we release it + # to pypi. The full test suite runs against source in CI; tests are not + # shipped in the wheel. # Note: Caller must run 'make build' first. - uv run nox -s test_core -- --wheel + uv run nox -s test_wheel test-core: uv run nox -s test_core diff --git a/py/_build_hooks.py b/py/_build_hooks.py new file mode 100644 index 000000000..3bf2ee684 --- /dev/null +++ b/py/_build_hooks.py @@ -0,0 +1,27 @@ +"""In-tree PEP 517 backend: setuptools, minus colocated tests in the wheel. + +pyproject.toml loads this module via ``backend-path``, which requires the +backend itself to live in the tree, so it re-exports ``setuptools.build_meta``. +""" + +import fnmatch + +from setuptools.build_meta import * # noqa: F403 +from setuptools.command.build_py import build_py as _build_py + + +_TEST_MODULE_PATTERNS = ("test_*", "_test_*", "conftest") +# Public and used by downstream test suites (e.g. with_memory_logger). +_SHIPPED_MODULES = ("test_helpers",) + + +def _is_test_module(module): + if module in _SHIPPED_MODULES: + return False + return any(fnmatch.fnmatch(module, pattern) for pattern in _TEST_MODULE_PATTERNS) + + +class BuildPy(_build_py): + def find_package_modules(self, package, package_dir): + modules = super().find_package_modules(package, package_dir) + return [(pkg, mod, path) for pkg, mod, path in modules if not _is_test_module(mod)] diff --git a/py/noxfile.py b/py/noxfile.py index b7a1beb5a..d2289b7d4 100644 --- a/py/noxfile.py +++ b/py/noxfile.py @@ -6,7 +6,6 @@ nox -l List all sessions. nox -s Run a specific session. nox ... -- --disable-vcr Run tests without vcrpy. - nox ... -- --wheel Run tests against the wheel in dist. nox -h Get help. """ @@ -241,7 +240,7 @@ def _pinned_python_version(): "linux_arm64": "6741466bc12e75544338292ab2c1c02c02f3c626568230b5548fffc53e5a87ff", } ERROR_CODES = tuple(range(1, 256)) -INTERNAL_TEST_FLAGS = {"--wheel", "--disable-vcr"} +INTERNAL_TEST_FLAGS = {"--disable-vcr"} GENERATED_LINT_EXCLUDES = ( "src/braintrust/_generated_types.py", "src/braintrust/generated_types.py", @@ -830,6 +829,20 @@ def is_importable(name): _run_core_tests(session) +# Not a default session: it needs a wheel from `make build`, so a bare `nox` +# (`make test`) must not pick it up. +@nox.session(default=False) +def test_wheel(session): + """Sanity check the built wheel in dist/ before release. Run `make build` first.""" + _session_install(session, _get_braintrust_wheel()) + _install_group_locked(session, "test") + _check_installed_deps(session) + test_dir = os.path.abspath("tests/wheel") + # Run from a temp dir so nothing from the source tree is importable. + with tempfile.TemporaryDirectory() as tmp, session.chdir(tmp): + session.run("pytest", test_dir, *session.posargs) + + @nox.session() def test_api_codegen(session): """Test the pinned OpenAPI validator and deterministic model generator.""" @@ -981,9 +994,6 @@ def pylint(session): def _install_test_deps(session, *groups): - # Choose the way we'll install braintrust ... wheel or source. - install_wheel = "--wheel" in session.posargs - # Install braintrust itself. Source installs are editable so that # site-packages resolves to src/ instead of holding a copy. A copy goes # stale as soon as the session venv is reused (``nox -R``), and anything @@ -991,18 +1001,17 @@ def _install_test_deps(session, *groups): # spawned by ``verify_autoinstrument_script`` -- picks up site-packages # rather than the source tree, so it would silently exercise the code from # whenever the venv was last built. - _session_install(session, *([_get_braintrust_wheel()] if install_wheel else ["-e", "."])) + _session_install(session, "-e", ".") # Install base test deps (pytest, pytest-asyncio, pytest-vcr) from the # lockfile so transitive deps are pinned and reproducible. _install_group_locked(session, "test", *groups) - # Sanity check braintrust imports from where this mode expects it: - # site-packages for a wheel, the source tree for an editable install. + # Sanity check braintrust imports from the source tree. lines = [ "import sys, braintrust as b", "print(f'Using braintrust from: {b.__file__}')", - f"sys.exit(0 if {install_wheel} == ('site-packages' in b.__file__) else 1)", + "sys.exit(0 if 'site-packages' not in b.__file__ else 1)", ] session.run("python", "-c", ";".join(lines)) @@ -1060,63 +1069,34 @@ def _run_tests( version=None, run_from_temp_dir=False, ): - """Run tests against a wheel or the source code. Paths should be relative and start with braintrust.""" + """Run tests against the source code. Paths should be relative and start with braintrust.""" _check_installed_deps(session) env = env.copy() if env else {} if version: env["BRAINTRUST_TEST_PACKAGE_VERSION"] = version - wheel_flag = "--wheel" in session.posargs common_args = ["--disable-vcr"] if "--disable-vcr" in session.posargs else [] pytest_posargs = [arg for arg in session.posargs if arg not in INTERNAL_TEST_FLAGS] test_paths = [test_path] if isinstance(test_path, str) else list(test_path) paths_to_ignore = ignore_paths or [] - if not wheel_flag: - # Run the tests in the src directory. - source_test_paths = [f"src/{path}" for path in test_paths] - source_ignore_paths = [f"src/{path}" for path in paths_to_ignore] - if run_from_temp_dir: - source_test_paths = [os.path.abspath(path) for path in source_test_paths] - source_ignore_paths = [os.path.abspath(path) for path in source_ignore_paths] - test_args = [ - "pytest", - # Disable the braintrust pytest plugin (registered via pytest11 entry - # point) to avoid ImportPathMismatchError when the installed package - # and the source tree both contain braintrust/conftest.py. - "-p", - "no:braintrust", - *source_test_paths, - ] - test_args.extend(f"--ignore={path}" for path in source_ignore_paths) - if run_from_temp_dir: - with tempfile.TemporaryDirectory() as tmp, session.chdir(tmp): - session.run(*test_args, *common_args, *pytest_posargs, env=env) - else: + source_test_paths = [f"src/{path}" for path in test_paths] + source_ignore_paths = [f"src/{path}" for path in paths_to_ignore] + if run_from_temp_dir: + source_test_paths = [os.path.abspath(path) for path in source_test_paths] + source_ignore_paths = [os.path.abspath(path) for path in source_ignore_paths] + test_args = [ + "pytest", + # Disable the braintrust pytest plugin (registered via pytest11 entry + # point) to avoid ImportPathMismatchError when the installed package + # and the source tree both contain braintrust/conftest.py. + "-p", + "no:braintrust", + *source_test_paths, + ] + test_args.extend(f"--ignore={path}" for path in source_ignore_paths) + if run_from_temp_dir: + with tempfile.TemporaryDirectory() as tmp, session.chdir(tmp): session.run(*test_args, *common_args, *pytest_posargs, env=env) - return - - # Running the tests from the wheel involves a bit of gymnastics to ensure we don't import - # local modules from the source directory. - # First, we need to absolute paths to all the binaries and libs in our venv that we'll see. - py = os.path.join(session.bin, "python") - site_packages = session.run(py, "-c", "import site; print(site.getsitepackages()[0])", silent=True).strip() - abs_test_paths = [os.path.abspath(os.path.join(site_packages, path)) for path in test_paths] - pytest_path = os.path.join(session.bin, "pytest") - - ignore_args = [] - for path in paths_to_ignore: - abs_ignore_path = os.path.abspath(os.path.join(site_packages, path)) - ignore_args.append(f"--ignore={abs_ignore_path}") - - # Lastly, change to a different directory to ensure we don't install local stuff. - with tempfile.TemporaryDirectory() as tmp: - with session.chdir(tmp): - # This env var is used to detect if we're running from the wheel. - # It proved very helpful because it's very easy - # to accidentally import local modules from the source directory. - env["BRAINTRUST_TESTING_WHEEL"] = "1" - session.run(pytest_path, *abs_test_paths, *ignore_args, *common_args, *pytest_posargs, env=env) - - # And a final note ... if it's not clear from above, we include test files in our wheel, which - # is perhaps not ideal? + else: + session.run(*test_args, *common_args, *pytest_posargs, env=env) diff --git a/py/pyproject.toml b/py/pyproject.toml index e5d6b2d1d..07858f944 100644 --- a/py/pyproject.toml +++ b/py/pyproject.toml @@ -1,6 +1,7 @@ [build-system] requires = ["setuptools>=82.0.1"] -build-backend = "setuptools.build_meta" +build-backend = "_build_hooks" +backend-path = ["."] [project] name = "braintrust" @@ -84,11 +85,18 @@ all = [ [tool.setuptools.dynamic] version = {attr = "braintrust.version.VERSION"} +[tool.setuptools.cmdclass] +build_py = "_build_hooks.BuildPy" + [tool.setuptools.packages.find] where = ["src"] +exclude = ["braintrust.type_tests*", "braintrust.fixtures*"] [tool.setuptools.package-data] -braintrust = ["py.typed", "fixtures/test-image.png"] +braintrust = ["py.typed"] + +[tool.setuptools.exclude-package-data] +braintrust = ["fixtures/*", "type_tests/*"] # --------------------------------------------------------------------------- # Dependency groups (PEP 735) diff --git a/py/src/braintrust/conftest.py b/py/src/braintrust/conftest.py index 922af68ed..ffb5887b6 100644 --- a/py/src/braintrust/conftest.py +++ b/py/src/braintrust/conftest.py @@ -254,21 +254,6 @@ def reset_braintrust_state(): logger._state = logger.BraintrustState() -@pytest.fixture(autouse=True) -def skip_vcr_tests_in_wheel_mode(request): - """Skip VCR tests when running from an installed wheel. - - Wheel mode (BRAINTRUST_TESTING_WHEEL=1) is a pre-release sanity check - that verifies the built package installs and runs correctly. It's not - intended to be a full test suite - VCR cassettes are not included in - the wheel, so we skip those tests here. The full test suite with VCR - tests runs against source code during normal CI. - """ - if os.environ.get("BRAINTRUST_TESTING_WHEEL") == "1": - if request.node.get_closest_marker("vcr"): - pytest.skip("VCR tests skipped in wheel mode (pre-release sanity check only)") - - _SENSITIVE_RESPONSE_HEADERS = { "openai-organization", "openai-project", diff --git a/py/src/braintrust/test_cassette_usage.py b/py/src/braintrust/test_cassette_usage.py index 90aa1dc26..dd2b29c1a 100644 --- a/py/src/braintrust/test_cassette_usage.py +++ b/py/src/braintrust/test_cassette_usage.py @@ -114,8 +114,6 @@ def test_recorder_is_off_without_env_var(tmp_path): @pytest.fixture def checker(tmp_path, monkeypatch): - if not _SCRIPT.exists(): - pytest.skip("scripts/ is not available (wheel mode)") spec = importlib.util.spec_from_file_location("check_unused_cassettes", _SCRIPT) module = importlib.util.module_from_spec(spec) spec.loader.exec_module(module) diff --git a/py/src/braintrust/test_nox_matrix.py b/py/src/braintrust/test_nox_matrix.py index f64cbb32e..0bb89379e 100644 --- a/py/src/braintrust/test_nox_matrix.py +++ b/py/src/braintrust/test_nox_matrix.py @@ -16,8 +16,6 @@ @pytest.mark.parametrize("returncode", [0, 1]) def test_measured_durations_use_nox_report(tmp_path, monkeypatch, returncode): - if not _SCRIPT.exists(): - pytest.skip("scripts/ is not available (wheel mode)") spec = importlib.util.spec_from_file_location("nox_matrix", _SCRIPT) matrix = importlib.util.module_from_spec(spec) spec.loader.exec_module(matrix) diff --git a/py/src/braintrust/test_version.py b/py/src/braintrust/test_version.py index 27a5a520c..e1d2ffdab 100644 --- a/py/src/braintrust/test_version.py +++ b/py/src/braintrust/test_version.py @@ -1,31 +1,9 @@ -import os - from braintrust import version def test_version(): - """Test the version module and verify templating works correctly.""" - # Detect wheel testing environment in multiple ways for robustness. - # The env variable makes sure we're not running from source - wheel_env = os.environ.get("BRAINTRUST_TESTING_WHEEL") == "1" - is_wheel_in_path = "site-packages" in version.__file__ - is_from_wheel = wheel_env or is_wheel_in_path - - # Basic assertions that should always pass + """Test the version module from source. Templating is checked against the wheel in tests/wheel.""" assert version.VERSION - assert version.GIT_COMMIT - assert version.RELEASE_CHANNEL assert isinstance(version.VERSION, str) - assert isinstance(version.GIT_COMMIT, str) - assert isinstance(version.RELEASE_CHANNEL, str) - assert len(version.VERSION) > 0 - assert len(version.GIT_COMMIT) > 0 - assert len(version.RELEASE_CHANNEL) > 0 - if is_from_wheel: - # When testing from the wheel, GIT_COMMIT - # should be the actual commit hash, not the placeholder - assert version.GIT_COMMIT != "__GIT_COMMIT__" - else: - # When testing from source directly, we expect to see the placeholder - assert version.GIT_COMMIT == "__GIT_COMMIT__" - assert version.RELEASE_CHANNEL == "source" + assert version.GIT_COMMIT == "__GIT_COMMIT__" + assert version.RELEASE_CHANNEL == "source" diff --git a/py/tests/wheel/test_wheel.py b/py/tests/wheel/test_wheel.py new file mode 100644 index 000000000..dac1685f4 --- /dev/null +++ b/py/tests/wheel/test_wheel.py @@ -0,0 +1,101 @@ +"""Sanity checks for the built braintrust wheel. + +Run by the ``test_wheel`` nox session against the wheel installed from +``dist/``, before it is published. These tests live outside the package +because colocated tests are not shipped in the wheel. +""" + +import importlib +import importlib.util +import pkgutil +import shutil +import subprocess +import sysconfig +from importlib.metadata import distribution, entry_points + +import braintrust +import pytest +from braintrust import version + + +DIST = distribution("braintrust") +WHEEL_FILES = [str(path) for path in DIST.files] + + +def test_imports_from_installed_wheel(): + assert "site-packages" in braintrust.__file__, braintrust.__file__ + + +def test_version_is_templated(): + assert version.VERSION == DIST.version + assert version.GIT_COMMIT and version.GIT_COMMIT != "__GIT_COMMIT__" + assert version.RELEASE_CHANNEL + + +def test_ships_py_typed(): + assert "braintrust/py.typed" in WHEEL_FILES + + +def test_does_not_ship_tests(): + def is_test_file(path): + name = path.rsplit("/", 1)[-1] + return ( + (name.startswith(("test_", "_test_")) and name != "test_helpers.py") + or name == "conftest.py" + or any(part in path for part in ("/cassettes/", "/type_tests/", "/fixtures/")) + ) + + assert [path for path in WHEEL_FILES if is_test_file(path)] == [] + + +def test_ships_public_test_helpers(): + # Downstream test suites import fixtures from braintrust.test_helpers. + from braintrust.test_helpers import init_test_logger, with_memory_logger # noqa: F401 + + +def test_all_modules_import(): + # Optional third-party dependencies are not installed, so ImportErrors for + # them are expected. A missing braintrust module means the wheel is broken. + missing = [] + for module in pkgutil.walk_packages(braintrust.__path__, "braintrust."): + try: + importlib.import_module(module.name) + except ModuleNotFoundError as e: + if e.name and e.name.split(".")[0] == "braintrust": + missing.append(f"{module.name}: {e}") + except ImportError: + pass + assert missing == [] + + +def test_pytest_plugin_entry_point_loads(): + (plugin,) = entry_points(group="pytest11", name="braintrust") + assert hasattr(plugin.load(), "pytest_addoption") + + +@pytest.mark.parametrize("group", ["console_scripts", "harbor.plugins"]) +def test_entry_points_resolve(group): + eps = [ep for ep in entry_points(group=group) if ep.dist and ep.dist.name == "braintrust"] + assert eps + for ep in eps: + assert importlib.util.find_spec(ep.module) is not None, ep.value + + +def test_cli_help(): + cli = shutil.which("braintrust", path=sysconfig.get_path("scripts")) + assert cli is not None + result = subprocess.run([cli, "--help"], capture_output=True, text=True, check=False) + assert result.returncode == 0, result.stderr + assert "eval" in result.stdout + + +def test_local_eval(): + result = braintrust.Eval( + "wheel-smoke", + data=[{"input": 1, "expected": 2}, {"input": 2, "expected": 4}], + task=lambda input: input * 2, + scores=[lambda input, output, expected: float(output == expected)], + no_send_logs=True, + ) + assert [r.output for r in result.results] == [2, 4] + assert [list(r.scores.values()) for r in result.results] == [[1.0], [1.0]]