From a38a92991b228bc2a2b71d65c9fb3a5ffe9b81e8 Mon Sep 17 00:00:00 2001 From: "F.N. Claessen" Date: Mon, 3 Aug 2026 17:59:20 +0200 Subject: [PATCH 1/4] Run the scheduler constraint tests under both backends Since #2364 the schedulers build the same model twice, once through Pyomo and once directly in HiGHS, so a test of scheduler behaviour only means something under one backend if the two agree -- which is the thing that cannot be assumed. Only test_solver.py was parametrized, so most constraint behaviour was verified under the configured default and no other. That is how a fix could pass on the Pyomo path and fail on the direct one, as happened on #2355. A module now opts in with RUN_UNDER_EACH_SOLVER = True and an autouse fixture in conftest does the switching, so no test signature changes. Enabled on test_group_constraints.py and test_operation_modes.py (27 tests, now 54). test_commitments.py and test_storage.py are NOT enabled, and the reason is worth recording: they build assets with fixed names, so running each test twice in one fixture scope violates generic_asset's unique-name constraint. Parametrizing them means making those fixtures unique per parameter first, which is a larger change than this one. Rather than leave that hole silent, test_solver_coverage.py asserts every planning test module either opts in or appears on an EXEMPT list with a reason, and that no EXEMPT entry is stale. A new module is then a decision someone makes on purpose instead of a gap nobody notices. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01X2jLjsQzwztDc7Nxz1ozmQ Signed-off-by: F.N. Claessen --- .../data/models/planning/tests/conftest.py | 32 +++++++++ .../planning/tests/test_group_constraints.py | 3 + .../planning/tests/test_operation_modes.py | 3 + .../planning/tests/test_solver_coverage.py | 67 +++++++++++++++++++ 4 files changed, 105 insertions(+) create mode 100644 flexmeasures/data/models/planning/tests/test_solver_coverage.py diff --git a/flexmeasures/data/models/planning/tests/conftest.py b/flexmeasures/data/models/planning/tests/conftest.py index 17752bdabe..0d1738494a 100644 --- a/flexmeasures/data/models/planning/tests/conftest.py +++ b/flexmeasures/data/models/planning/tests/conftest.py @@ -443,3 +443,35 @@ def add_as_beliefs(db, sensor, values, time_slots, source): for dt, val in zip(time_slots, values) ] db.session.add_all(beliefs) + + +@pytest.fixture(autouse=True) +def solver_backend(request, app): + """Run a test under a specific solver backend, when its module opts in. + + Modules setting ``RUN_UNDER_EACH_SOLVER = True`` have every test run once per + backend (see ``pytest_generate_tests`` below). Everything else is untouched and + keeps running under the configured default. + """ + solver = getattr(request, "param", None) + if solver is None: + yield None + return + original_solver = app.config["FLEXMEASURES_LP_SOLVER"] + app.config["FLEXMEASURES_LP_SOLVER"] = solver + yield solver + app.config["FLEXMEASURES_LP_SOLVER"] = original_solver + + +def pytest_generate_tests(metafunc): + """Parametrize a whole module over the solver backends, if it opts in. + + A module that exercises scheduler behaviour is only meaningful under one backend + if the two agree, which is exactly what we cannot assume: the schedulers build the + same model twice, once through Pyomo and once directly in HiGHS. Opting a module in + costs a signature change nowhere -- the autouse fixture above does the switching. + """ + if getattr(metafunc.module, "RUN_UNDER_EACH_SOLVER", False): + metafunc.parametrize( + "solver_backend", ["appsi_highs", "highspy"], indirect=True + ) diff --git a/flexmeasures/data/models/planning/tests/test_group_constraints.py b/flexmeasures/data/models/planning/tests/test_group_constraints.py index 88f8e119c7..4d90f26531 100644 --- a/flexmeasures/data/models/planning/tests/test_group_constraints.py +++ b/flexmeasures/data/models/planning/tests/test_group_constraints.py @@ -10,6 +10,9 @@ from flexmeasures.data.models.planning.storage import StorageScheduler from flexmeasures.utils.unit_utils import ur +#: Run every test in this module under both scheduler backends (see conftest). +RUN_UNDER_EACH_SOLVER = True + def _unique_name(prefix: str) -> str: return f"{prefix} {uuid.uuid4().hex[:8]}" diff --git a/flexmeasures/data/models/planning/tests/test_operation_modes.py b/flexmeasures/data/models/planning/tests/test_operation_modes.py index aaa6a3a7e4..3ccf64fc8f 100644 --- a/flexmeasures/data/models/planning/tests/test_operation_modes.py +++ b/flexmeasures/data/models/planning/tests/test_operation_modes.py @@ -7,6 +7,9 @@ from flexmeasures.data.models.planning.linear_optimization import device_scheduler from flexmeasures.data.models.planning.utils import initialize_index +#: Run every test in this module under both scheduler backends (see conftest). +RUN_UNDER_EACH_SOLVER = True + def _one_device_setup(stock_target: float): """One storage device charging towards a stock target over 4 hourly steps. diff --git a/flexmeasures/data/models/planning/tests/test_solver_coverage.py b/flexmeasures/data/models/planning/tests/test_solver_coverage.py new file mode 100644 index 0000000000..a2ec72c704 --- /dev/null +++ b/flexmeasures/data/models/planning/tests/test_solver_coverage.py @@ -0,0 +1,67 @@ +"""Keep track of which scheduler tests actually run under both backends. + +The schedulers build the same model twice — once through Pyomo, once directly in HiGHS — +so a test of scheduler behaviour only means something under one backend if the two agree, +which is the very thing that cannot be assumed. +A module that does not opt into the solver matrix is therefore a coverage hole, +and this test exists so that hole is an explicit, reviewed decision rather than an accident. + +To opt a module in, set ``RUN_UNDER_EACH_SOLVER = True`` at its top (see conftest). +""" + +from __future__ import annotations + +import pathlib + +#: Modules that exercise scheduler behaviour but deliberately run under one solver only. +#: Each needs a reason, and the reason should be fixable rather than permanent. +EXEMPT = { + # These build named assets in the database, so running each test twice in the same + # fixture scope violates generic_asset's unique-name constraint. Parametrizing them + # means making their fixtures unique-per-parameter first. + "test_commitments.py": "creates named DB assets; not idempotent across parameters", + "test_storage.py": "creates named DB assets; not idempotent across parameters", + "test_process.py": "ProcessScheduler does not use device_scheduler", + # Covered by the solver matrix through their own fixture instead. + "test_solver.py": "uses the app_with_each_solver fixture directly", + "test_highspy_equivalence.py": "runs both backends explicitly, per scenario", + "test_solver_options.py": "tests option validation, not scheduling", + # No scheduling involved. + "test_device_inventory.py": "no scheduling", + "test_storage_utils.py": "no scheduling", + "test_utils.py": "no scheduling", + "test_utils_fresh_db.py": "no scheduling", +} + +TESTS_DIR = pathlib.Path(__file__).parent + + +def test_scheduler_modules_run_under_each_solver_or_are_exempt(): + """Every planning test module either opts into the solver matrix or is listed as exempt. + + If this fails after adding a module, decide which it is — + do not add it to EXEMPT just to go green. + """ + uncovered = [] + for path in sorted(TESTS_DIR.glob("test_*.py")): + if path.name == pathlib.Path(__file__).name: + continue + opts_in = "RUN_UNDER_EACH_SOLVER = True" in path.read_text() + if not opts_in and path.name not in EXEMPT: + uncovered.append(path.name) + assert not uncovered, ( + "These planning test modules run under one solver only, and are not listed as exempt: " + f"{uncovered}. Either set RUN_UNDER_EACH_SOLVER = True, or add them to EXEMPT with a reason." + ) + + +def test_exempt_list_has_no_stale_entries(): + """An exempt module that no longer exists, or that has since opted in, should be removed.""" + stale = [] + for name in EXEMPT: + path = TESTS_DIR / name + if not path.exists(): + stale.append(f"{name} (gone)") + elif "RUN_UNDER_EACH_SOLVER = True" in path.read_text(): + stale.append(f"{name} (now opts in)") + assert not stale, f"Stale EXEMPT entries: {stale}" From 91adfacd575946eba698ec662ff11472b959bb95 Mon Sep 17 00:00:00 2001 From: "F.N. Claessen" Date: Mon, 3 Aug 2026 23:18:53 +0200 Subject: [PATCH 2/4] review: show the exempt modules pass under the other backend too The exempt modules could not be parametrized in-process, but that left their behaviour under the non-default backend simply unknown, which is a weaker position than it needed to be. Adds a --lp-solver option pinning a whole run to one backend, so those modules can be run again under the other one. (An environment variable would not do: TestingConfig does not read FLEXMEASURES_LP_SOLVER.) Result: test_commitments.py, test_storage.py and test_process.py -- 68 tests -- all pass under appsi_highs, the non-default backend. So the exemption costs per-test granularity, not coverage. Checked the flag actually bites rather than silently doing nothing: running with --lp-solver=definitely_not_a_solver fails with Pyomo's UnknownSolver, so the option does reach solver selection. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01X2jLjsQzwztDc7Nxz1ozmQ Signed-off-by: F.N. Claessen --- .../data/models/planning/tests/conftest.py | 34 +++++++++++++++++++ .../planning/tests/test_solver_coverage.py | 7 ++++ 2 files changed, 41 insertions(+) diff --git a/flexmeasures/data/models/planning/tests/conftest.py b/flexmeasures/data/models/planning/tests/conftest.py index 0d1738494a..e95a7fe693 100644 --- a/flexmeasures/data/models/planning/tests/conftest.py +++ b/flexmeasures/data/models/planning/tests/conftest.py @@ -475,3 +475,37 @@ def pytest_generate_tests(metafunc): metafunc.parametrize( "solver_backend", ["appsi_highs", "highspy"], indirect=True ) + + +def pytest_addoption(parser): + """Allow a whole run to be pinned to one solver backend. + + The modules that cannot be parametrized in-process (see EXEMPT in + test_solver_coverage.py) can still be shown green under the other backend by running + them again with this flag. Note that FLEXMEASURES_LP_SOLVER cannot be set from the + environment for tests, because TestingConfig does not read it. + """ + parser.addoption( + "--lp-solver", + action="store", + default=None, + help="Run every test under this solver backend (e.g. appsi_highs).", + ) + + +@pytest.fixture(autouse=True) +def pinned_solver_backend(request, app): + """Apply --lp-solver, unless the test is already parametrized over backends.""" + solver = request.config.getoption("--lp-solver") + if ( + solver is None + or "solver_backend" in request.fixturenames + and getattr(request.node, "callspec", None) + and "solver_backend" in request.node.callspec.params + ): + yield + return + original_solver = app.config["FLEXMEASURES_LP_SOLVER"] + app.config["FLEXMEASURES_LP_SOLVER"] = solver + yield + app.config["FLEXMEASURES_LP_SOLVER"] = original_solver diff --git a/flexmeasures/data/models/planning/tests/test_solver_coverage.py b/flexmeasures/data/models/planning/tests/test_solver_coverage.py index a2ec72c704..4c72d14bb3 100644 --- a/flexmeasures/data/models/planning/tests/test_solver_coverage.py +++ b/flexmeasures/data/models/planning/tests/test_solver_coverage.py @@ -19,6 +19,13 @@ # These build named assets in the database, so running each test twice in the same # fixture scope violates generic_asset's unique-name constraint. Parametrizing them # means making their fixtures unique-per-parameter first. + # + # They are not unverified, though: a whole run can be pinned to one backend with + # --lp-solver, and all three pass under the non-default one. See the PR description, + # and re-check with: + # pytest flexmeasures/data/models/planning/tests/test_commitments.py \ + # flexmeasures/data/models/planning/tests/test_storage.py \ + # flexmeasures/data/models/planning/tests/test_process.py --lp-solver=appsi_highs "test_commitments.py": "creates named DB assets; not idempotent across parameters", "test_storage.py": "creates named DB assets; not idempotent across parameters", "test_process.py": "ProcessScheduler does not use device_scheduler", From b59d1444e795eaf4a1b7c438894e9bb9b12dda6f Mon Sep 17 00:00:00 2001 From: "F.N. Claessen" Date: Mon, 3 Aug 2026 23:39:49 +0200 Subject: [PATCH 3/4] review: reflow this PR's docstrings and comments Every docstring and comment block added here broke lines mid-phrase, against the convention this repo documents and that #2384 had just restated -- "run once per / backend", "under one backend / if the two agree", "set from the / environment". Reflowed so each physical line ends after punctuation. Text only. Checked mechanically rather than by eye this time: inside a multi-line docstring or comment block, every line but the last must end in punctuation. The only remaining hits in the added lines are the two continuation lines of an example shell command, which end in a backslash by nature. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01X2jLjsQzwztDc7Nxz1ozmQ Signed-off-by: F.N. Claessen --- .../data/models/planning/tests/conftest.py | 21 +++++++++---------- .../planning/tests/test_solver_coverage.py | 12 +++++------ 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/flexmeasures/data/models/planning/tests/conftest.py b/flexmeasures/data/models/planning/tests/conftest.py index e95a7fe693..1d0b54fa2a 100644 --- a/flexmeasures/data/models/planning/tests/conftest.py +++ b/flexmeasures/data/models/planning/tests/conftest.py @@ -449,9 +449,8 @@ def add_as_beliefs(db, sensor, values, time_slots, source): def solver_backend(request, app): """Run a test under a specific solver backend, when its module opts in. - Modules setting ``RUN_UNDER_EACH_SOLVER = True`` have every test run once per - backend (see ``pytest_generate_tests`` below). Everything else is untouched and - keeps running under the configured default. + Modules setting ``RUN_UNDER_EACH_SOLVER = True`` have every test run once per backend (see ``pytest_generate_tests`` below). + Everything else is untouched, and keeps running under the configured default. """ solver = getattr(request, "param", None) if solver is None: @@ -466,10 +465,10 @@ def solver_backend(request, app): def pytest_generate_tests(metafunc): """Parametrize a whole module over the solver backends, if it opts in. - A module that exercises scheduler behaviour is only meaningful under one backend - if the two agree, which is exactly what we cannot assume: the schedulers build the - same model twice, once through Pyomo and once directly in HiGHS. Opting a module in - costs a signature change nowhere -- the autouse fixture above does the switching. + A module that exercises scheduler behaviour is only meaningful under one backend if the two agree, + which is exactly what we cannot assume: + the schedulers build the same model twice, once through Pyomo and once directly in HiGHS. + Opting a module in costs a signature change nowhere -- the autouse fixture above does the switching. """ if getattr(metafunc.module, "RUN_UNDER_EACH_SOLVER", False): metafunc.parametrize( @@ -480,10 +479,10 @@ def pytest_generate_tests(metafunc): def pytest_addoption(parser): """Allow a whole run to be pinned to one solver backend. - The modules that cannot be parametrized in-process (see EXEMPT in - test_solver_coverage.py) can still be shown green under the other backend by running - them again with this flag. Note that FLEXMEASURES_LP_SOLVER cannot be set from the - environment for tests, because TestingConfig does not read it. + The modules that cannot be parametrized in-process (see EXEMPT in test_solver_coverage.py) + can still be shown green under the other backend, by running them again with this flag. + Note that FLEXMEASURES_LP_SOLVER cannot be set from the environment for tests, + because TestingConfig does not read it. """ parser.addoption( "--lp-solver", diff --git a/flexmeasures/data/models/planning/tests/test_solver_coverage.py b/flexmeasures/data/models/planning/tests/test_solver_coverage.py index 4c72d14bb3..f8fe4bd4ee 100644 --- a/flexmeasures/data/models/planning/tests/test_solver_coverage.py +++ b/flexmeasures/data/models/planning/tests/test_solver_coverage.py @@ -16,13 +16,13 @@ #: Modules that exercise scheduler behaviour but deliberately run under one solver only. #: Each needs a reason, and the reason should be fixable rather than permanent. EXEMPT = { - # These build named assets in the database, so running each test twice in the same - # fixture scope violates generic_asset's unique-name constraint. Parametrizing them - # means making their fixtures unique-per-parameter first. + # These build named assets in the database, + # so running each test twice in the same fixture scope violates generic_asset's unique-name constraint. + # Parametrizing them means making their fixtures unique-per-parameter first. # - # They are not unverified, though: a whole run can be pinned to one backend with - # --lp-solver, and all three pass under the non-default one. See the PR description, - # and re-check with: + # They are not unverified, though: + # a whole run can be pinned to one backend with --lp-solver, and all three pass under the non-default one. + # See the PR description, and re-check with: # pytest flexmeasures/data/models/planning/tests/test_commitments.py \ # flexmeasures/data/models/planning/tests/test_storage.py \ # flexmeasures/data/models/planning/tests/test_process.py --lp-solver=appsi_highs From f2236768a84cb8a1881073afd43851298e69422b Mon Sep 17 00:00:00 2001 From: "F.N. Claessen" Date: Tue, 4 Aug 2026 00:12:38 +0200 Subject: [PATCH 4/4] review: drop the comment pointing at the PR description A code comment telling the reader to consult a pull request description ages badly and does not belong in the tree. Replaced with the fact it was pointing at: these modules pass under the other backend when a run is pinned with --lp-solver. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01X2jLjsQzwztDc7Nxz1ozmQ Signed-off-by: F.N. Claessen --- .../models/planning/tests/test_solver_coverage.py | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/flexmeasures/data/models/planning/tests/test_solver_coverage.py b/flexmeasures/data/models/planning/tests/test_solver_coverage.py index f8fe4bd4ee..18bc5ef1ed 100644 --- a/flexmeasures/data/models/planning/tests/test_solver_coverage.py +++ b/flexmeasures/data/models/planning/tests/test_solver_coverage.py @@ -16,16 +16,9 @@ #: Modules that exercise scheduler behaviour but deliberately run under one solver only. #: Each needs a reason, and the reason should be fixable rather than permanent. EXEMPT = { - # These build named assets in the database, - # so running each test twice in the same fixture scope violates generic_asset's unique-name constraint. - # Parametrizing them means making their fixtures unique-per-parameter first. - # - # They are not unverified, though: - # a whole run can be pinned to one backend with --lp-solver, and all three pass under the non-default one. - # See the PR description, and re-check with: - # pytest flexmeasures/data/models/planning/tests/test_commitments.py \ - # flexmeasures/data/models/planning/tests/test_storage.py \ - # flexmeasures/data/models/planning/tests/test_process.py --lp-solver=appsi_highs + # These create assets with hardcoded names inline, + # so running each test twice in one fixture scope violates generic_asset's unique-name constraint. + # They still pass under the other backend when a whole run is pinned to it with --lp-solver. "test_commitments.py": "creates named DB assets; not idempotent across parameters", "test_storage.py": "creates named DB assets; not idempotent across parameters", "test_process.py": "ProcessScheduler does not use device_scheduler",