[FIX]: Stop rendering "Pass" for regression tests with no test_result row#1136
Open
x15sr71 wants to merge 1 commit into
Open
[FIX]: Stop rendering "Pass" for regression tests with no test_result row#1136x15sr71 wants to merge 1 commit into
x15sr71 wants to merge 1 commit into
Conversation
…ls on missing-row regression tests
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



In raising this pull request, I confirm the following (please check boxes):
My familiarity with the project is as follows (check one):
Problem
Regression test rows on the test detail page render as "Pass" with empty Runtime/Exit Code columns when no
test_resultrow was written for that RT. This contradicts both the category-level "Fail" header and the GitHub bot comments.Production reproduction
For both tests, the DB has exactly 1
test_resultrow (RT 241, Hardsubx):But the UI shows all ~250 regression test rows for each, almost all labeled "Pass" with empty Runtime/Exit Code columns. The bot comments on PR #2279 report
0/86 Options,0/27 General,0/13 WTV, etc. — clearly contradicting the UI.End-to-end chain
test_resultrow writtentest_resultrowget_test_results()inmod_test/controllers.pyfalls into theelsebranch for these RTs:This correctly flags the category-level error (header shows "Fail") and injects a dummy
TestResultFilewithgot='error'. Buttest.resultremainsNonefor these RTs (no row exists intest.results).templates/test/by_id.html:148evaluates:In Jinja's default Undefined mode,
None.exit_codebecomes Undefined, andUndefined != 0evaluates to True. ThePasselif fires before thegot == "error"elif (which sits below it) can run.Verified:
Why this is longstanding but only now noticed
The bug fires only when
test.result is None— i.e., when a regression test has the dummy file-row injected by the controller'selsebranch but no realtest_resultrow.Most production tests have
test_resultrows for every RT, so the bug doesn't manifest. It becomes visible in edge cases:test_resultrow (504 storms killed the VM mid-run) → ~249 rows render as false "Pass"The template has not been modified in this section for years, so the bug has likely been silent the whole time.
Fix
Two changes in
templates/test/by_id.html:test.result.exit_codeaccesses with(test.result and ...)so the comparison is only evaluated when a real result row exists.got == "error"is checked before thePasselif. Defense-in-depth: even without the guards, the bug stays fixed because the correct branch runs first.How to verify after deploy
Open https://sampleplatform.ccextractor.org/test/9322 after the deploy lands. Currently ~249 rows show "Pass" with empty Runtime/Exit Code. After the fix, those rows should render as "No output generated but there should be" (the controller already correctly flags them). Only RT 241 (Hardsubx, the one RT that actually ran) should remain as "Pass" with real runtime data.
Dependency on deploy unblock
Until the PEP 668 deploy block (fix-deploy-pep668 PR) lands, no new sample-platform code reaches the running process. This PR's effect won't be visible until that lands first.