Split plot-smoke tests into parametrized cases
- Dominant language
- Python
- Stars
- 56
- Forks
- 9
- Avg merge
- 57m
- Merged PRs (30d)
- 3
Description
## Problem
A handful of plot tests are slow and provide poor diagnostic value. They stack many `plot.scatter(...)` calls into a single test body and end with `assert True`, so a failing kwarg combination requires manual bisection to locate.
Worst offenders (durations from a recent local run):
- [`tests/test_multimodelcompare.py::test_mm_scatter`](https://github.com/DHI/modelskill/blob/main/tests/test_multimodelcompare.py) — **4.12 s**, 12 scatter calls, two real assertions
- [`tests/test_multivariable_compare.py::test_mv_mm_scatter`](https://github.com/DHI/modelskill/blob/main/tests/test_multivariable_compare.py) — **1.92 s**, same shape
- [`tests/test_multimodelcompare.py::test_custom_metric_skilltable_mm_scatter`](https://github.com/DHI/modelskill/blob/main/tests/test_multimodelcompare.py) — **0.39 s**, same shape
Together they're ~14 % of total suite runtime (45 s).
## Why the current shape is bad
- **No useful failure signal.** 12 calls in one test → you only know "something broke," not which kwarg combination.
- **Discourages adding coverage.** Every new scatter kwarg makes the test slower instead of expanding the matrix.
- **Hides regressions.** A change that breaks one combo but not the others still surfaces as one red dot.
## Proposed fix
Convert each into `@pytest.mark.parametrize` over the kwarg variations. Each combination becomes its own test case with its own name and its own assertion. Same coverage, parallelisable, individually attributable failures, sub-second per variant.
Drop the trailing `assert True` — if the test only checks "didn't crash," a parametrize with no body still does that.
## Scope
Plot-smoke tests only — not the tests that already have real assertions per call.
Contributor guide
Assessment
This issue has not been assessed yet.