Skip to content

Commit 513f19e

Browse files
Hell1213canihavesomecoffee
Hell1213
authored and
canihavesomecoffee
committed
Fix: Guard against negative runtime values in test results
Add input validation in finish_type_request() to clamp negative runtime values to 0 and log warnings for diagnostics.
1 parent 1c13293 commit 513f19e

2 files changed

Lines changed: 196 additions & 1 deletion

File tree

‎mod_ci/controllers.py‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2708,8 +2708,20 @@ def finish_type_request(log, test_id, test, request):
27082708
"""
27092709
log.debug(f"Finish for {test_id}/{request.form['test_id']}")
27102710
regression_test = RegressionTest.query.filter(RegressionTest.id == request.form['test_id']).first()
2711+
2712+
raw_runtime = request.form.get('runTime', 0)
2713+
try:
2714+
runtime = int(raw_runtime)
2715+
except (TypeError, ValueError):
2716+
log.warning(f"Invalid runtime '{raw_runtime}' for test {test_id}; storing 0")
2717+
runtime = 0
2718+
2719+
if runtime < 0:
2720+
log.warning(f"Negative runtime {runtime} for test {test_id}; clamping to 0")
2721+
runtime = 0
2722+
27112723
result = TestResult(
2712-
test.id, regression_test.id, request.form['runTime'],
2724+
test.id, regression_test.id, runtime,
27132725
request.form['exitCode'], regression_test.expected_rc
27142726
)
27152727
g.db.add(result)

‎tests/test_ci/test_negative_runtime_fix.py‎

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
import unittest
2+
from unittest.mock import Mock, patch
3+
4+
from mod_ci.controllers import finish_type_request
5+
from tests.base import BaseTestCase
6+
7+
8+
class TestNegativeRuntimeFix(BaseTestCase):
9+
10+
@patch('mod_ci.controllers.safe_db_commit')
11+
@patch('mod_ci.controllers.g')
12+
@patch('mod_ci.controllers.RegressionTest')
13+
@patch('mod_ci.controllers.TestResult')
14+
def test_negative_runtime_clamped_to_zero(self, mock_test_result,
15+
mock_regression_test, mock_g,
16+
mock_safe_db_commit):
17+
mock_log = Mock()
18+
mock_test = Mock()
19+
mock_test.id = 123
20+
21+
mock_regression_test_instance = Mock()
22+
mock_regression_test_instance.id = 1
23+
mock_regression_test_instance.expected_rc = 0
24+
mock_regression_test.query.filter.return_value.first.return_value = \
25+
mock_regression_test_instance
26+
27+
mock_request = Mock()
28+
mock_request.form = {
29+
'test_id': '1',
30+
'runTime': '-5000',
31+
'exitCode': '0'
32+
}
33+
34+
mock_g.db = Mock()
35+
mock_safe_db_commit.return_value = True
36+
37+
finish_type_request(mock_log, 123, mock_test, mock_request)
38+
39+
mock_log.warning.assert_called_once_with(
40+
"Negative runtime -5000 for test 123; clamping to 0")
41+
mock_test_result.assert_called_once_with(123, 1, 0, '0', 0)
42+
mock_g.db.add.assert_called_once()
43+
mock_safe_db_commit.assert_called_once()
44+
45+
@patch('mod_ci.controllers.safe_db_commit')
46+
@patch('mod_ci.controllers.g')
47+
@patch('mod_ci.controllers.RegressionTest')
48+
@patch('mod_ci.controllers.TestResult')
49+
def test_positive_runtime_unchanged(self, mock_test_result,
50+
mock_regression_test, mock_g,
51+
mock_safe_db_commit):
52+
mock_log = Mock()
53+
mock_test = Mock()
54+
mock_test.id = 123
55+
56+
mock_regression_test_instance = Mock()
57+
mock_regression_test_instance.id = 1
58+
mock_regression_test_instance.expected_rc = 0
59+
mock_regression_test.query.filter.return_value.first.return_value = \
60+
mock_regression_test_instance
61+
62+
mock_request = Mock()
63+
mock_request.form = {
64+
'test_id': '1',
65+
'runTime': '12345',
66+
'exitCode': '0'
67+
}
68+
69+
mock_g.db = Mock()
70+
mock_safe_db_commit.return_value = True
71+
72+
finish_type_request(mock_log, 123, mock_test, mock_request)
73+
74+
mock_log.warning.assert_not_called()
75+
mock_test_result.assert_called_once_with(123, 1, 12345, '0', 0)
76+
mock_g.db.add.assert_called_once()
77+
mock_safe_db_commit.assert_called_once()
78+
79+
@patch('mod_ci.controllers.safe_db_commit')
80+
@patch('mod_ci.controllers.g')
81+
@patch('mod_ci.controllers.RegressionTest')
82+
@patch('mod_ci.controllers.TestResult')
83+
def test_invalid_runtime_defaults_to_zero(self, mock_test_result,
84+
mock_regression_test, mock_g,
85+
mock_safe_db_commit):
86+
mock_log = Mock()
87+
mock_test = Mock()
88+
mock_test.id = 123
89+
90+
mock_regression_test_instance = Mock()
91+
mock_regression_test_instance.id = 1
92+
mock_regression_test_instance.expected_rc = 0
93+
mock_regression_test.query.filter.return_value.first.return_value = \
94+
mock_regression_test_instance
95+
96+
mock_request = Mock()
97+
mock_request.form = {
98+
'test_id': '1',
99+
'runTime': 'invalid_string',
100+
'exitCode': '0'
101+
}
102+
103+
mock_g.db = Mock()
104+
mock_safe_db_commit.return_value = True
105+
106+
finish_type_request(mock_log, 123, mock_test, mock_request)
107+
108+
mock_log.warning.assert_called_once_with(
109+
"Invalid runtime 'invalid_string' for test 123; storing 0")
110+
mock_test_result.assert_called_once_with(123, 1, 0, '0', 0)
111+
mock_g.db.add.assert_called_once()
112+
mock_safe_db_commit.assert_called_once()
113+
114+
@patch('mod_ci.controllers.safe_db_commit')
115+
@patch('mod_ci.controllers.g')
116+
@patch('mod_ci.controllers.RegressionTest')
117+
@patch('mod_ci.controllers.TestResult')
118+
def test_zero_runtime_unchanged(self, mock_test_result,
119+
mock_regression_test, mock_g,
120+
mock_safe_db_commit):
121+
mock_log = Mock()
122+
mock_test = Mock()
123+
mock_test.id = 123
124+
125+
mock_regression_test_instance = Mock()
126+
mock_regression_test_instance.id = 1
127+
mock_regression_test_instance.expected_rc = 0
128+
mock_regression_test.query.filter.return_value.first.return_value = \
129+
mock_regression_test_instance
130+
131+
mock_request = Mock()
132+
mock_request.form = {
133+
'test_id': '1',
134+
'runTime': '0',
135+
'exitCode': '0'
136+
}
137+
138+
mock_g.db = Mock()
139+
mock_safe_db_commit.return_value = True
140+
141+
finish_type_request(mock_log, 123, mock_test, mock_request)
142+
143+
mock_log.warning.assert_not_called()
144+
mock_test_result.assert_called_once_with(123, 1, 0, '0', 0)
145+
mock_g.db.add.assert_called_once()
146+
mock_safe_db_commit.assert_called_once()
147+
148+
@patch('mod_ci.controllers.safe_db_commit')
149+
@patch('mod_ci.controllers.g')
150+
@patch('mod_ci.controllers.RegressionTest')
151+
@patch('mod_ci.controllers.TestResult')
152+
def test_missing_runtime_defaults_to_zero(self, mock_test_result,
153+
mock_regression_test, mock_g,
154+
mock_safe_db_commit):
155+
mock_log = Mock()
156+
mock_test = Mock()
157+
mock_test.id = 123
158+
159+
mock_regression_test_instance = Mock()
160+
mock_regression_test_instance.id = 1
161+
mock_regression_test_instance.expected_rc = 0
162+
mock_regression_test.query.filter.return_value.first.return_value = \
163+
mock_regression_test_instance
164+
165+
mock_request = Mock()
166+
mock_request.form = {
167+
'test_id': '1',
168+
'exitCode': '0'
169+
}
170+
171+
mock_g.db = Mock()
172+
mock_safe_db_commit.return_value = True
173+
174+
finish_type_request(mock_log, 123, mock_test, mock_request)
175+
176+
mock_log.warning.assert_not_called()
177+
mock_test_result.assert_called_once_with(123, 1, 0, '0', 0)
178+
mock_g.db.add.assert_called_once()
179+
mock_safe_db_commit.assert_called_once()
180+
181+
182+
if __name__ == '__main__':
183+
unittest.main()

0 commit comments

Comments
 (0)