DHI / DHI/modelskill

Split plot-smoke tests into parametrized cases

Open
#660 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.