diff --git a/.github/actions/load-shared-vars/action.yml b/.github/actions/load-shared-vars/action.yml index abc2c2e47..dc2abb91b 100644 --- a/.github/actions/load-shared-vars/action.yml +++ b/.github/actions/load-shared-vars/action.yml @@ -19,6 +19,9 @@ outputs: min-deps-matrix: description: "Matrix object for the minimum dependency test job" value: ${{ steps.load.outputs.min-deps-matrix }} + min-deps-python-default: + description: "The min-deps cell which runs the steps only one cell needs" + value: ${{ steps.load.outputs.min-deps-python-default }} network-os-matrix: description: "OS list for the network test job" value: ${{ steps.load.outputs.network-os-matrix }} @@ -37,6 +40,9 @@ runs: # matrix (each entry there costs one environment cache per OS). full_py='["3.12","3.13","3.14"]' min_deps_py='["3.12","3.14"]' + # The one min-deps cell that runs the doctests: they take minutes and + # say the same thing on every version. + min_deps_py_default="3.12" # What a pull request runs. Every python version and every OS is # still covered; only the redundant cross product is dropped. The @@ -60,4 +66,5 @@ runs: echo "python-default=$python_default" >> "$GITHUB_OUTPUT" echo "test-matrix=$test_matrix" >> "$GITHUB_OUTPUT" echo "min-deps-matrix=$min_deps_matrix" >> "$GITHUB_OUTPUT" + echo "min-deps-python-default=$min_deps_py_default" >> "$GITHUB_OUTPUT" echo "network-os-matrix=$network_os" >> "$GITHUB_OUTPUT" diff --git a/.github/test_code.sh b/.github/test_code.sh index d752ff8af..956cc5906 100755 --- a/.github/test_code.sh +++ b/.github/test_code.sh @@ -1,22 +1,39 @@ #!/bin/bash -# Script to run tests to account for wonkiness of periodic mac failures. -args=(tests -m "not network" -s --cov dascore --cov-append --cov-report=xml) +# Runs one flavor of the test suite. Coverage is written to a data file, not +# xml: each CI cell keeps its own file and the coverage_gate job combines +# them, because no single OS covers every line (see runtests.yml). + +# sysmon is coverage's sys.monitoring core, ~1.11x the no-coverage runtime +# against ~1.67x for the C tracer. It needs python >= 3.12 (the floor) and +# does not support branch coverage, which is off here. +export COVERAGE_CORE=sysmon + +# -n logical rather than -n auto: xdist's auto asks psutil for *physical* +# cores, which is 2 on the SMT-enabled runners; logical gives all 3-4. +parallel=(-n logical --dist loadfile) +cov_args=(--cov dascore --cov-append --cov-report=) + +args=(tests -m "not network" "${parallel[@]}" "${cov_args[@]}") if [[ "$1" == "network" ]]; then - args=(tests -m network -s --cov dascore --cov-append --cov-report=xml) + args=(tests -m network "${parallel[@]}" "${cov_args[@]}") fi if [[ "$1" == "doctest" ]]; then args=(dascore --doctest-modules) fi if [[ "$1" == "profile" ]]; then + # No xdist: codspeed measures this process. args=(benchmarks --codspeed) fi -exit_code=0 +python -c " +import os +try: + import psutil + physical = psutil.cpu_count(logical=False) +except ImportError: + physical = None +print(f'cpus: logical={os.cpu_count()} physical={physical}') +" -python -m pytest "${args[@]}" || exit_code=$? - -# Check the exit code is related to sporadic failures on mac, see #312 -if [ $exit_code -ne 132 ] && [ $exit_code -ne 0 ]; then - exit $exit_code -fi +python -m pytest "${args[@]}" diff --git a/.github/workflows/get_coverage.yml b/.github/workflows/get_coverage.yml deleted file mode 100644 index 7c3952373..000000000 --- a/.github/workflows/get_coverage.yml +++ /dev/null @@ -1,47 +0,0 @@ -# Calculates new coverage for the base branch and uploads to codecov -name: Coverage -on: - push: - branches: - - master - -permissions: - contents: read - -env: - # Where the test-data cache is restored; pooch reads files from here. - # Must match the path used in .github/actions/prime-test-data-cache. - DFS_DATA_DIR: ${{ github.workspace }}/.test_data_cache - -jobs: - calc_coverage: - runs-on: ubuntu-latest - - steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - with: - persist-credentials: false - fetch-tags: "true" - fetch-depth: '0' - - - uses: ./.github/actions/load-shared-vars - id: shared-vars - - - uses: ./.github/actions/install-dascore - with: - # Defined in .github/actions/load-shared-vars/action.yml - python-version: ${{ steps.shared-vars.outputs.python-default }} - prepare-test-data: "true" - - - name: run test suite - shell: bash - run: | - pytest -s --cov dascore --cov-report=xml - - - uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f # v7.0.0 - with: - fail_ci_if_error: true - files: ./coverage.xml - flags: unittests # optional - name: master_tests # optional - token: ${{ secrets.CODECOV_TOKEN }} # required diff --git a/.github/workflows/run_min_dep_tests.yml b/.github/workflows/run_min_dep_tests.yml index 2162ffbe0..bf94e0517 100644 --- a/.github/workflows/run_min_dep_tests.yml +++ b/.github/workflows/run_min_dep_tests.yml @@ -43,6 +43,7 @@ jobs: outputs: # Shared values live in .github/actions/load-shared-vars/action.yml min-deps-matrix: ${{ steps.load-vars.outputs.min-deps-matrix }} + min-deps-python-default: ${{ steps.load-vars.outputs.min-deps-python-default }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: @@ -97,9 +98,12 @@ jobs: shell: bash run: ./.github/test_code.sh - # Runs examples in docstrings + # Runs examples in docstrings. One cell: without numba the jit kernels' + # examples run in pure python, which costs minutes on every OS and + # version alike. - name: test docstrings id: run_docstrings + if: matrix.os == 'ubuntu-latest' && matrix.python-version == needs.setup.outputs.min-deps-python-default continue-on-error: ${{ env.debug_enabled == 'true' }} shell: bash run: ./.github/test_code.sh doctest diff --git a/.github/workflows/runtests.yml b/.github/workflows/runtests.yml index e5a38afab..3f452ded7 100644 --- a/.github/workflows/runtests.yml +++ b/.github/workflows/runtests.yml @@ -81,6 +81,8 @@ jobs: env: debug_enabled: ${{ github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'debug') }} PYTEST_ADDOPTS: ${{ github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'debug') && '-vv --durations=100' || '' }} + # One data file per cell; the coverage_gate job combines them all. + COVERAGE_FILE: .coverage.${{ matrix.os }}-${{ matrix.python-version }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -94,8 +96,14 @@ jobs: python-version: ${{ matrix.python-version }} prepare-test-data: "true" + # The generated docs tests and the doctests are the same on every cell + # and cost minutes, so one cell runs each. The generated tests run + # inside the measured suite below, so their coverage reaches the gate; + # the doctests are not measured, and counting them would let a line + # reachable only from a docstring example satisfy the gate. - name: generate qmd docs tests id: generate_qmd_tests + if: matrix.os == 'ubuntu-latest' && matrix.python-version == needs.setup.outputs.python-default shell: bash run: python scripts/generate_doc_code_tests.py @@ -110,6 +118,7 @@ jobs: # Runs examples in docstrings - name: test docstrings id: run_docstrings + if: matrix.os == 'ubuntu-latest' && matrix.python-version == needs.setup.outputs.python-default continue-on-error: ${{ env.debug_enabled == 'true' }} shell: bash run: ./.github/test_code.sh doctest @@ -129,19 +138,80 @@ jobs: with: limit-access-to-actor: true - # Upload coverage files + # The cell's coverage data, for the coverage_gate job to combine. + # Uploaded even when the tests failed, so a partial report is still + # readable; the gate then fails on the missing lines rather than on a + # missing artifact. + - name: upload coverage data + if: ${{ !cancelled() }} + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: coverage-${{ matrix.os }}-${{ matrix.python-version }} + # A coverage data file starts with a dot. + path: ${{ env.COVERAGE_FILE }} + include-hidden-files: true + if-no-files-found: error + + - name: fail job after debug session if tests failed + if: steps.generate_qmd_tests.outcome == 'failure' || steps.run_test_suite.outcome == 'failure' || steps.run_docstrings.outcome == 'failure' + shell: bash + run: exit 1 + + # Where the 100% coverage threshold is enforced. It has to run on the + # combined data rather than in any one cell: a handful of lines are + # reachable only on a case-insensitive filesystem (macOS, Windows), and + # the generated docs tests run on one cell. + coverage_gate: + needs: [setup, test_code] + timeout-minutes: 20 + runs-on: ubuntu-latest + + # Report on whatever the cells produced, including when one of them + # failed; do not wait on network_tests, which is allowed to fail. + if: ${{ !cancelled() && (github.event_name == 'push' || !contains(github.event.pull_request.labels.*.name, 'no_ci')) }} + + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + # Coverage alone: this job reads the source tree, it does not import + # dascore, so the dependency install is not worth its minute. + - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + with: + python-version: ${{ needs.setup.outputs.python-default }} + + - name: install coverage + shell: bash + run: python -m pip install "coverage>=7.4,<8" + + - uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 # v7.0.0 + with: + pattern: coverage-* + merge-multiple: true + + # [tool.coverage.paths] in pyproject.toml is what lets the three + # operating systems' data files combine into one set of files. + - name: combine coverage + shell: bash + run: | + coverage combine + coverage report --show-missing + coverage xml + - uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f # v7.0.0 + if: ${{ !cancelled() }} with: - fail_ci_if_error: false + fail_ci_if_error: true files: ./coverage.xml flags: unittests - name: PR_tests + name: combined_tests token: ${{ secrets.CODECOV_TOKEN }} - - name: fail job after debug session if tests failed - if: steps.generate_qmd_tests.outcome == 'failure' || steps.run_test_suite.outcome == 'failure' || steps.run_docstrings.outcome == 'failure' + # Last, so the report is uploaded whether or not this passes. + - name: require full coverage shell: bash - run: exit 1 + run: coverage report --fail-under=100 network_tests: needs: setup @@ -157,6 +227,9 @@ jobs: # Keep remote-IO coverage visible without blocking unrelated changes. if: github.event_name == 'push' || !contains(github.event.pull_request.labels.*.name, 'no_ci') + env: + COVERAGE_FILE: .coverage.network-${{ matrix.os }} + steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: @@ -173,7 +246,19 @@ jobs: shell: bash run: ./.github/test_code.sh network + # One network report per run, from one OS: the codecov comment waits + # for a fixed number of uploads (codecov.yml), and the three operating + # systems exercise the same remote code. + # !cancelled(), not the implicit success(): the tests here are allowed + # to fail, and skipping the xml would leave codecov waiting forever for + # the second of the two uploads it counts. + - name: write coverage xml + if: ${{ matrix.os == 'ubuntu-latest' && !cancelled() }} + shell: bash + run: coverage xml + - uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f # v7.0.0 + if: ${{ matrix.os == 'ubuntu-latest' && !cancelled() }} with: fail_ci_if_error: false files: ./coverage.xml @@ -209,14 +294,21 @@ jobs: - uses: ./.github/actions/mamba-install-dascore with: python-version: ${{ needs.setup.outputs.python-default }} - # [test] rather than the [dev] default: pip would otherwise layer - # ~30 PyPI distributions over the conda environment, which is the - # opposite of what this job is checking. - install-group-str: "[test]" + # Installed below with --no-deps instead: letting pip resolve the + # dependencies would layer PyPI distributions over the conda + # environment and so hide whatever environment.yml is missing. + install-package: "false" prepare-test-data: "true" - # No coverage upload: the uv jobs already cover this suite, and this job - # exists to prove environment.yml still solves and dascore works in it. - - name: run test suite + - name: install dascore on the conda environment alone + shell: bash -el {0} + run: | + pip install --no-deps -e . + pip check + + # A smoke test, not the suite: every test already runs on the uv + # matrix, and what this job answers is whether environment.yml still + # solves and dascore reads a file in the environment it describes. + - name: run a subset of the test suite shell: bash -el {0} - run: python -m pytest tests -m "not network" -q + run: python -m pytest tests/test_io/test_dasdae tests/test_compat.py -q diff --git a/.github/workflows/test_free_threaded.yml b/.github/workflows/test_free_threaded.yml index a9c4e77e0..b682ffe81 100644 --- a/.github/workflows/test_free_threaded.yml +++ b/.github/workflows/test_free_threaded.yml @@ -60,7 +60,7 @@ jobs: - name: install dascore run: | python -m pip install --upgrade pip - pip install -e . pytest pytest-timeout + pip install -e . pytest pytest-timeout pytest-xdist - name: confirm the GIL is disabled run: | @@ -74,4 +74,4 @@ jobs: uses: ./.github/actions/prime-test-data-cache - name: run test suite without the GIL - run: python -m pytest tests -m "not network" -q --timeout=900 + run: python -m pytest tests -m "not network" -q --timeout=900 -n logical --dist loadfile diff --git a/codecov.yml b/codecov.yml index a425a208a..aa7d5af07 100644 --- a/codecov.yml +++ b/codecov.yml @@ -1,9 +1,12 @@ +# Two uploads reach codecov per run: one `unittests` report from the +# coverage_gate job (every test cell combined) and one `network` report from +# the ubuntu network job. codecov: notify: - after_n_builds: 7 + after_n_builds: 2 comment: - after_n_builds: 7 + after_n_builds: 2 # Require 100% coverage on the unittests flag. coverage: diff --git a/docs/contributing/testing.qmd b/docs/contributing/testing.qmd index 0314ea732..6d4d99e08 100644 --- a/docs/contributing/testing.qmd +++ b/docs/contributing/testing.qmd @@ -20,6 +20,18 @@ introduce large blocks of dead code. pytest tests --cov dascore --cov-report term-missing ``` +The suite is parallel safe, and CI runs it that way. Locally it is the +quickest way to run the whole thing: + + +```bash +pytest tests -n logical --dist loadfile +``` + +CI requires 100% coverage of the combined data from every operating system +rather than from any one run, so a Linux-only run can report a line or two +missing which macOS and Windows cover. + If you would like to test the IO modules it can be done like so: diff --git a/environment.yml b/environment.yml index f50a38ce7..0d82b678f 100644 --- a/environment.yml +++ b/environment.yml @@ -1,23 +1,31 @@ name: dascore channels: - conda-forge +# Every dependency dascore requires belongs here: the conda_env job installs +# dascore with --no-deps and runs pip check, so a package missing from this +# file fails that job rather than being quietly filled in from PyPI. dependencies: - python>=3.12 - pytest - pytest-timeout + - array-api-compat>=1.9 - numpy>=2.3.3 - - pydantic>=2.1 + - packaging + - pydantic>2.1 - pip - pandas>=3.0 - pooch>=1.3 + - pyyaml + - rich + - universal_pathlib - xarray - pre-commit - h5py - - matplotlib>=3.5 + - matplotlib>=3.10 - scipy>=1.15.0 - findiff - pyarrow - jupyter - nbformat - - pint + - pint>=0.24.4 - typing_extensions>=4.12 diff --git a/pyproject.toml b/pyproject.toml index 342b40576..035e5b25c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -114,6 +114,7 @@ test = [ "pytest", "pytest-timeout", "pytest-codeblocks", + "pytest-xdist", "s3fs", "starlette", "twine", @@ -279,6 +280,15 @@ convention = "numpy" # which ruff cannot see, and its autofix would delete the import. "*.ipynb" = ["D103", "F401"] +# Coverage is measured on three operating systems and combined in one job +# (the coverage_gate job in runtests.yml), so the data files have to name the +# same files: relative paths, with the separator differences mapped away. +[tool.coverage.run] +relative_files = true + +[tool.coverage.paths] +source = ["dascore/", "*/dascore/"] + [tool.pytest.ini_options] norecursedirs = [ "*.egg", diff --git a/tests/conftest.py b/tests/conftest.py index 27157ef58..71a9cbfa4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -7,7 +7,7 @@ import shutil import threading import warnings -from contextlib import contextmanager, suppress +from contextlib import contextmanager from pathlib import Path import h5py @@ -42,6 +42,31 @@ # they get issued every time so tests around warning behavior aren't flaky. warnings.filterwarnings("default", category=UserWarning) + +# A filesystem which has neither kind of link raises pathlib's +# UnsupportedOperation (a NotImplementedError) rather than an OSError; +# emscripten is one. +_NO_LINK = (OSError, NotImplementedError) + + +def _link_or_copy(source: Path, dest: Path) -> None: + """Populate one file path using the cheapest available local copy.""" + dest.parent.mkdir(parents=True, exist_ok=True) + if dest.exists(): + return + try: + dest.hardlink_to(source) + return + except _NO_LINK: + pass + try: + dest.symlink_to(source) + return + except _NO_LINK: + pass + shutil.copy2(source, dest) + + # --- Pytest configuration @@ -643,17 +668,19 @@ def memory_spool_dim_1_patches(): @pytest.fixture(scope="class") @register_func(SPOOL_FIXTURES) -def all_examples_spool(terra15_das_example_path): - """Create a spool from all the examples.""" - parent = terra15_das_example_path.parent - spool = dc.spool(parent) - try: - spool = spool.update() - except Exception: - with suppress(FileNotFoundError): - spool.indexer.index_path.unlink() # delete index if problems found - spool = spool.update() # then re-index - return spool +def all_examples_spool(tmp_path_factory, terra15_das_example_path): + """Create a spool from all the example files.""" + # Indexing the example files where they sit would write an index into the + # download cache, which every test process shares. Links cost nothing and + # give the index a directory of its own. + source = terra15_das_example_path.parent + directory = Path(tmp_path_factory.mktemp("all_examples")) + for path in source.rglob("*"): + # Skip the index (and anything else hidden) a previous run may have + # left in the cache: a hard link to it is that same file. + if path.is_file() and not path.name.startswith("."): + _link_or_copy(path, directory / path.relative_to(source)) + return dc.spool(directory).update() @pytest.fixture(scope="class") diff --git a/tests/test_io/conftest.py b/tests/test_io/conftest.py index eea228674..1417b38d5 100644 --- a/tests/test_io/conftest.py +++ b/tests/test_io/conftest.py @@ -9,7 +9,6 @@ from __future__ import annotations import os -import shutil import socketserver import threading import time @@ -26,6 +25,7 @@ from dascore.compat import UPath from dascore.utils.downloader import fetch +from tests.conftest import _link_or_copy from tests.test_io._common_io_test_utils import skip_on_timeout @@ -165,24 +165,6 @@ def _parse_range_header(header: str, size: int) -> tuple[int | None, int | None] return (start, min(end, size - 1)) -def _link_or_copy(source: Path, dest: Path) -> None: - """Populate one served file path using the cheapest available local copy.""" - dest.parent.mkdir(parents=True, exist_ok=True) - if dest.exists(): - return - try: - dest.hardlink_to(source) - return - except OSError: - pass - try: - dest.symlink_to(source) - return - except OSError: - pass - shutil.copy2(source, dest) - - def _prime_http_test_tree( ensure_file: Callable[[str, str | Path | None], Path], ) -> None: diff --git a/tests/test_io/test_index/test_schema.py b/tests/test_io/test_index/test_schema.py index c296c5983..62948dfd6 100644 --- a/tests/test_io/test_index/test_schema.py +++ b/tests/test_io/test_index/test_schema.py @@ -2,7 +2,9 @@ from __future__ import annotations +import contextlib import sqlite3 +import time from concurrent.futures import ThreadPoolExecutor from threading import Barrier @@ -87,17 +89,35 @@ class TestConcurrentInitialization: def test_concurrent_open(self, tmp_path): """Connections racing to create one index all open successfully.""" path = tmp_path / "shared.sqlite3" - barrier = Barrier(4) + # Hold the write lock on the empty file. Every opener then gets past + # its "no tables yet" read and piles up waiting to create them, so + # exactly one wins and the rest take the re-check branch. Left to + # chance, a loaded machine runs the four openers one after another + # and that branch is never reached. + gate = sqlite3.connect(path, isolation_level=None) + gate.execute("BEGIN IMMEDIATE") + ready = Barrier(5) def open_index(_): - barrier.wait() + # The same read the backend makes first: proof this thread + # reached the gate while the database was still schema-less. + with contextlib.closing(sqlite3.connect(path)) as con: + assert not con.execute("SELECT name FROM sqlite_master").fetchall() + ready.wait(timeout=60) backend = get_backend(path) metadata = backend.get_metadata() backend.close() return metadata["index_version"] with ThreadPoolExecutor(max_workers=4) as pool: - versions = list(pool.map(open_index, range(4))) + futures = [pool.submit(open_index, num) for num in range(4)] + ready.wait(timeout=60) + # The openers only have a connect and a BEGIN left to run; give + # them that before the lock they are queueing for is released. + time.sleep(0.05) + gate.execute("ROLLBACK") + gate.close() + versions = [future.result() for future in futures] assert versions == [INDEX_VERSION] * 4 backend = get_backend(path) assert len(backend._fetch_df("SELECT * FROM meta_data")) == 1 diff --git a/tests/test_workflow/test_serialize.py b/tests/test_workflow/test_serialize.py index 8bceb35fc..a0489ff1a 100644 --- a/tests/test_workflow/test_serialize.py +++ b/tests/test_workflow/test_serialize.py @@ -464,8 +464,13 @@ def test_named_callable_has_no_source(self): def test_sourceless_callable(self): """A callable whose source cannot be read is still encodable.""" - # Built by eval, so there is no file holding the text of it. - assert isinstance(digest(eval("lambda x: x")), str) + # Compiled under a filename nothing will ever hold the text of. + # A plain eval() is not enough: something else in the process (an + # xdist worker's bootstrap, for one) can leave "" in the + # linecache, and then the source is readable after all. + func = eval(compile("lambda x: x", "", "eval")) + assert encode(func)["$callable"]["source"] is None + assert isinstance(digest(func), str) def test_partial(self): """A partial is what it wraps and what it wraps it with."""