A run can report `pass` after executing almost none of its samples

[gaurav02081]

Summary

_check_completed_run_status iterates only the TestResult rows that exist. Samples that never ran produce no row, so they are invisible to it — a run reports pass whenever the few tests that did run happened to pass, regardless of how many never ran at all.

Reproduction

No database or server needed; this calls the function on master directly:

from mod_api.services import status

class FakeResult:
    def __init__(self, rt_id):
        self.regression_test_id, self.exit_code, self.expected_rc = rt_id, 0, 0

# One test produced a result. The other 236 never ran, so they have no rows.
verdict = status._check_completed_run_status(9360, {9360: [FakeResult(1)]}, {}, {})
print(verdict)   # -> 'pass'

In production

run	status	samples that ran
9322 (PR 2279, linux)	pass	1 of 474
9323 (PR 2279, windows)	pass	1 of 474
9360 (PR 2292, linux)	pass	1 of 237

PR #2279 is open right now and shows a passing run on both platforms.

The API also contradicts itself on the same run:

GET /runs/9360           ->  status: pass
GET /runs/9360/summary   ->  pass_count: 1, skipped_count: 236

Of 116 completed runs sampled, 8 had skipped_count > 0. The three above reported pass; the other five (9338, 9342, 9346, 9350, 9358 — between 70% and 87% executed) reported fail, because something that did run failed. So an incomplete run is reported as passing exactly when the surviving fragment happens to be green.

Cause

results = results_by_test.get(t_id, [])
if not results:
    return 'error'          # zero results is handled
for r in results:           # ... but this only sees rows that exist
    ...
return 'pass'

The zero-result case was considered explicitly; the partial-result case was not.

For context on how a run ends up like this: 9360's log shows Starting with entry 1 of 1, then two 504s, then log-upload retries — the runner stopped after the first category file. Its run config lists all 237 test ids, so the platform did intend to run them.

Suggested fix

mod_api/routes/runs.py:519 already derives the skipped count as total_samples - len(results). The status derivation can use the same comparison and return error — or a new incomplete — when a run produced fewer results than it was configured to run.

Impact

A pull request can show green having tested essentially nothing, which is how a bad change gets merged. Found while reviewing every open CCExtractor pull request through sp_cli, using read-only API access.

[aryanorastar]

Working on a fix for this.

Approach: in _check_completed_run_status, compare reported TestResult.regression_test_ids against the run’s expected sample set (customized tests if present, else all active regression tests — same rule as summary total_samples). If any expected samples are missing, return error instead of pass. Zero-result runs already return error; this extends that to the partial-result case.

PR incoming.