From 154c41a68205687ff7d3799cc6b78fc05eca39ec Mon Sep 17 00:00:00 2001 From: Aryan Date: Sun, 16 Aug 2026 10:58:55 +0530 Subject: [PATCH] fix(api): treat incomplete sample runs as error, not pass Completed runs that only report a subset of configured regression tests were marked pass whenever that fragment was green. Compare reported TestResult IDs against the run's expected sample set (same rule as summary totals) and return error when any are missing. --- mod_api/routes/runs.py | 18 ++++++------ mod_api/services/status.py | 48 +++++++++++++++++++++++++++---- tests/api/test_services_status.py | 19 ++++++++++++ 3 files changed, 69 insertions(+), 16 deletions(-) diff --git a/mod_api/routes/runs.py b/mod_api/routes/runs.py index 1ff75a91..1fce1d87 100644 --- a/mod_api/routes/runs.py +++ b/mod_api/routes/runs.py @@ -28,7 +28,8 @@ RunSchema, RunSummarySchema) from mod_api.services.error_service import derive_errors_for_run from mod_api.services.status import (batch_get_run_data, derive_run_status, - derive_sample_status) + derive_sample_status, + expected_regression_ids) from mod_api.utils import get_sort_column, paginated_response, single_response from mod_auth.models import Role from mod_customized.models import CustomizedTest @@ -434,16 +435,13 @@ def get_run(run_id): def _run_regression_ids(test): """Regression test IDs that belong to this run. - Uses the customized selection when present; otherwise falls back to - every ACTIVE regression test, mirroring create_run's default. (The - model's get_customized_regressiontests() falls back to all tests - including inactive ones, which inflates total_samples/skipped_count - with tests the run could never execute.) + Delegates to expected_regression_ids so summary totals and run-status + completeness checks share one rule. (The model's + get_customized_regressiontests() falls back to all tests including + inactive ones, which inflates total_samples/skipped_count with tests + the run could never execute.) """ - if test.customized_tests: - return [ct.regression_id for ct in test.customized_tests] - return [rt.id for rt in - RegressionTest.query.filter_by(active=True).all()] + return expected_regression_ids(test) def _aggregate_run_statistics( diff --git a/mod_api/services/status.py b/mod_api/services/status.py index adaf6227..01948e93 100644 --- a/mod_api/services/status.py +++ b/mod_api/services/status.py @@ -17,15 +17,27 @@ """ from collections import defaultdict -from typing import List, Optional +from typing import List, Optional, Set from sqlalchemy.orm import joinedload -from mod_regression.models import RegressionTestOutput +from mod_regression.models import RegressionTest, RegressionTestOutput from mod_test.models import (Test, TestProgress, TestResult, TestResultFile, TestStatus) +def expected_regression_ids(test: Test) -> List[int]: + """Regression test IDs this run was configured to execute. + + Uses the customized selection when present; otherwise every ACTIVE + regression test — same rule as create_run / run summary totals. + """ + if test.customized_tests: + return [ct.regression_id for ct in test.customized_tests] + return [rt.id for rt in + RegressionTest.query.filter_by(active=True).all()] + + def derive_run_status(test: Test) -> str: """ Map the raw model state to one of the 7 normalized run statuses. @@ -160,12 +172,20 @@ def _check_completed_run_status( t_id, results_by_test, files_by_test_and_rt, - expected_outputs_by_rt): + expected_outputs_by_rt, + expected_rt_ids: Optional[Set[int]] = None): results = results_by_test.get(t_id, []) if not results: # A run marked completed that produced zero TestResult rows is not # a pass — the worker finished without reporting anything. return 'error' + # Samples with no TestResult row are invisible to the loop below. If the + # run was configured for more samples than it reported, treat it as an + # error (same class as zero results) so a green fragment cannot pass. + if expected_rt_ids is not None: + reported_ids = {r.regression_test_id for r in results} + if not expected_rt_ids.issubset(reported_ids): + return 'error' for r in results: r_files = files_by_test_and_rt.get((t_id, r.regression_test_id), []) expected = expected_outputs_by_rt.get( @@ -181,7 +201,8 @@ def _compute_run_status( results_by_test, files_by_test_and_rt, t_id, - expected_outputs_by_rt=None): + expected_outputs_by_rt=None, + expected_rt_ids: Optional[Set[int]] = None): if not t_prog: return 'queued' @@ -196,7 +217,8 @@ def _compute_run_status( t_id, results_by_test, files_by_test_and_rt, - expected_outputs_by_rt) + expected_outputs_by_rt, + expected_rt_ids=expected_rt_ids) return 'incomplete' @@ -254,6 +276,19 @@ def batch_get_run_data(tests: list) -> tuple: for rto in all_expected: expected_outputs_by_rt[rto.regression_id].append(rto) + # Expected sample set per run (customized selection or all active RTs) + active_rt_ids = { + rt.id for rt in RegressionTest.query.filter_by(active=True).all() + } + expected_rt_ids_by_test = {} + for t in tests: + if t.customized_tests: + expected_rt_ids_by_test[t.id] = { + ct.regression_id for ct in t.customized_tests + } + else: + expected_rt_ids_by_test[t.id] = active_rt_ids + statuses = {} timestamps_dict = {} @@ -262,6 +297,7 @@ def batch_get_run_data(tests: list) -> tuple: timestamps_dict[t.id] = _compute_run_timestamps(t_prog) statuses[t.id] = _compute_run_status( t_prog, results_by_test, files_by_test_and_rt, t.id, - expected_outputs_by_rt=expected_outputs_by_rt) + expected_outputs_by_rt=expected_outputs_by_rt, + expected_rt_ids=expected_rt_ids_by_test[t.id]) return statuses, timestamps_dict diff --git a/tests/api/test_services_status.py b/tests/api/test_services_status.py index 3872af83..e8997b19 100644 --- a/tests/api/test_services_status.py +++ b/tests/api/test_services_status.py @@ -5,6 +5,7 @@ from mod_api.services.status import (derive_output_status, derive_run_status, derive_sample_status, get_run_timestamps, is_dummy_row) +from mod_customized.models import CustomizedTest from mod_regression.models import RegressionTestOutput from mod_regression.models import \ RegressionTestOutputFiles as RegressionTestMultipleFiles @@ -24,6 +25,12 @@ def setUp(self): g.db.add(self.test_obj) g.db.commit() + def _limit_run_to_regression(self, regression_id): + """Scope this run to one sample so partial-suite fixtures stay valid.""" + g.db.add(CustomizedTest(self.test_obj.id, regression_id)) + g.db.commit() + g.db.refresh(self.test_obj) + def test_derive_run_status_queued(self): self.assertEqual(derive_run_status(self.test_obj), 'queued') @@ -34,6 +41,7 @@ def test_derive_run_status_running(self): self.assertEqual(derive_run_status(self.test_obj), 'running') def test_derive_run_status_pass(self): + self._limit_run_to_regression(1) tp = TestProgress(self.test_obj.id, TestStatus.completed, 'done') # A passing result: exit code matches and the expected output for # regression test 1 was produced and matched (got=None). @@ -51,7 +59,18 @@ def test_derive_run_status_completed_without_results_is_error(self): g.db.commit() self.assertEqual(derive_run_status(self.test_obj), 'error') + def test_derive_run_status_completed_partial_results_is_error(self): + # Base fixtures seed two active regression tests. Completing with a + # result for only one of them must not report pass (#1177). + tp = TestProgress(self.test_obj.id, TestStatus.completed, 'done') + tr = TestResult(self.test_obj.id, 1, 100, 0, 0) + rf = TestResultFile(self.test_obj.id, 1, 1, 'sample_out1') + g.db.add_all([tp, tr, rf]) + g.db.commit() + self.assertEqual(derive_run_status(self.test_obj), 'error') + def test_derive_run_status_fail(self): + self._limit_run_to_regression(1) tp = TestProgress(self.test_obj.id, TestStatus.completed, 'done') # runtime 100, exit_code 1, expected 0 tr = TestResult(self.test_obj.id, 1, 100, 1, 0)