Skip to content

Group the tests by what they need, and give the static checks their own directory - #884

Merged
m-reuter merged 5 commits into
Deep-MI:devfrom
m-reuter:tests
Sep 15, 2026
Merged

m-reuter merged 5 commits into
Deep-MI:devfrom
m-reuter:tests

Conversation

@m-reuter

@m-reuter m-reuter commented Sep 15, 2026

Copy link
Copy Markdown
Member

Tests were spread across three workflows, the style workflow was running pytest, and two of the
three unittest jobs installed the whole project, torch included, to run checks that only read
files. This sorts them by what they actually need.

Measured

job before after
code-style 4 min 30s
test/image did not run, see below 2 min
test/shell inside code-style 22s
shell tests on macOS ~1 min 53s

The image job is the only one that installs the project, and it is the only one above a minute.

What each directory needs

directory needs runtime
test/lint pytest 0.3s
test/shell pytest, runs the shipped scripts 25s
test/image nibabel, numpy, scipy, FastSurferCNN the install dominates
test/quicktest a built docker image its own workflow

Only test/image needs the project installed. Everything else was paying a full
pip install .[quicktest] for nothing, and the install is the entire cost of those jobs.

test/lint

New directory for the checks that only read files and never execute anything:

  • test_bash4_lint.py, moved from test/shell, scans the shipped scripts for bash-4-only
    constructs. Its own docstring already said it "runs in the style job".
  • test_python_version.py, moved from test/config, which dissolves. It asserts that the
    declared python version agrees across pyproject.toml, the Dockerfile and the workflows.
  • test_every_test_dir_runs.py, new, see below.

These are style checks rather than tests of behaviour, so they run in code-style.yml, which
now runs the directory instead of naming files. A new static check lands there automatically.

The static and functional halves of the bash 3.2 coverage are now in different directories, so
the two docstrings that cross-reference each other name the full paths.

unittest.yaml

The matrix splits by what the tests need rather than by directory. The install step branches on
needs-project, so one job installs the project instead of two, and the macOS job runs
test/shell as a directory rather than naming two files.

PYTHON_PATH becomes PYTHONPATH, exported only where the project was not installed: it
precedes site-packages, so setting it unconditionally would make the image job import the working
tree and leave the installed package untested.

timeout-minutes drops from 180 to 20 on both jobs, now that a run takes about a minute. The
headroom is for a cold pip cache, which is what a pull request sees until a run on dev has
populated one.

Two bugs caught in review, one of them by the first CI run

The matrix collapsed to a single job and test/image never ran. GitHub merges an include
entry into every existing combination when none of its keys is a matrix dimension, so two such
entries overwrite each other rather than creating two jobs. The first CI run on this branch
showed it: a 24 second "unit tests" job, which cannot be the image tests, and no second ubuntu
job at all. tests is a real dimension again, which makes each include a filter.

test/lint/test_every_test_dir_runs.py guards both halves: every test/<name> holding tests must
be named by some workflow, expanding test/${{ matrix.tests }} through the matrix, and at least
one include key must be a matrix dimension. Verified against the broken matrix twice: exactly
those two tests fail and nothing else.

PYTHONPATH was shadowing the installed package, as above.

Unrelated CI speedup, found while looking at the timings

code-style.yml ran uv run --project src --extra style ruff check src, which resolved and
installed all 28 project dependencies, torch included, to run a linter that needs none of them.
That was the four minutes. It now uses --no-project --with ruff, the same form
CONTRIBUTING.md already documents. Ruff still reads [tool.ruff] from src/pyproject.toml:
--show-settings reports linter.line_length = 120, which exists nowhere else.

The tests now also run on pushes to dev

Not for coverage, since everything lands through a pull request and that event tests the merge
ref. For the pip cache: a run restores caches from its own branch, the default branch, or a pull
request's base branch. Your upstream default branch is stable and pull requests target dev,
so with no run on dev this workflow's cache only ever existed at pull-request scope, which no
other pull request can read, and every one of them re-downloaded torch. cancel-in-progress is
now limited to pull requests, because a cancelled run saves no cache.

Note for reviewers

The visible check names change, because the matrix values did: test/config is gone and
test/shell is new. Any branch protection rule naming the old checks needs updating.

@m-reuter
m-reuter merged commit 17fa7cb into Deep-MI:dev Sep 15, 2026
5 checks passed
@m-reuter
m-reuter deleted the tests branch September 15, 2026 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant