Fix: Guard against negative runtime values in test results

mod_ci/controllers.py

@@ -2708,8 +2708,20 @@ def finish_type_request(log, test_id, test, request):
     """
     log.debug(f"Finish for {test_id}/{request.form['test_id']}")
     regression_test = RegressionTest.query.filter(RegressionTest.id == request.form['test_id']).first()
+
+    raw_runtime = request.form.get('runTime', 0)
+    try:
+        runtime = int(raw_runtime)
+    except (TypeError, ValueError):
+        log.warning(f"Invalid runtime '{raw_runtime}' for test {test_id}; storing 0")
+        runtime = 0
+
+    if runtime < 0:
+        log.warning(f"Negative runtime {runtime} for test {test_id}; clamping to 0")
+        runtime = 0
+
     result = TestResult(
-        test.id, regression_test.id, request.form['runTime'],
+        test.id, regression_test.id, runtime,
         request.form['exitCode'], regression_test.expected_rc
     )
     g.db.add(result)

tests/test_ci/test_negative_runtime_fix.py

@@ -0,0 +1,183 @@
+import unittest
+from unittest.mock import Mock, patch
+
+from mod_ci.controllers import finish_type_request
+from tests.base import BaseTestCase
+
+
+class TestNegativeRuntimeFix(BaseTestCase):
+
+    @patch('mod_ci.controllers.safe_db_commit')
+    @patch('mod_ci.controllers.g')
+    @patch('mod_ci.controllers.RegressionTest')
+    @patch('mod_ci.controllers.TestResult')
+    def test_negative_runtime_clamped_to_zero(self, mock_test_result,
+                                              mock_regression_test, mock_g,
+                                              mock_safe_db_commit):
+        mock_log = Mock()
+        mock_test = Mock()
+        mock_test.id = 123
+
+        mock_regression_test_instance = Mock()
+        mock_regression_test_instance.id = 1
+        mock_regression_test_instance.expected_rc = 0
+        mock_regression_test.query.filter.return_value.first.return_value = \
+            mock_regression_test_instance
+
+        mock_request = Mock()
+        mock_request.form = {
+            'test_id': '1',
+            'runTime': '-5000',
+            'exitCode': '0'
+        }
+
+        mock_g.db = Mock()
+        mock_safe_db_commit.return_value = True
+
+        finish_type_request(mock_log, 123, mock_test, mock_request)
+
+        mock_log.warning.assert_called_once_with(
+            "Negative runtime -5000 for test 123; clamping to 0")
+        mock_test_result.assert_called_once_with(123, 1, 0, '0', 0)
+        mock_g.db.add.assert_called_once()
+        mock_safe_db_commit.assert_called_once()
+
+    @patch('mod_ci.controllers.safe_db_commit')
+    @patch('mod_ci.controllers.g')
+    @patch('mod_ci.controllers.RegressionTest')
+    @patch('mod_ci.controllers.TestResult')
+    def test_positive_runtime_unchanged(self, mock_test_result,
+                                        mock_regression_test, mock_g,
+                                        mock_safe_db_commit):
+        mock_log = Mock()
+        mock_test = Mock()
+        mock_test.id = 123
+
+        mock_regression_test_instance = Mock()
+        mock_regression_test_instance.id = 1
+        mock_regression_test_instance.expected_rc = 0
+        mock_regression_test.query.filter.return_value.first.return_value = \
+            mock_regression_test_instance
+
+        mock_request = Mock()
+        mock_request.form = {
+            'test_id': '1',
+            'runTime': '12345',
+            'exitCode': '0'
+        }
+
+        mock_g.db = Mock()
+        mock_safe_db_commit.return_value = True
+
+        finish_type_request(mock_log, 123, mock_test, mock_request)
+
+        mock_log.warning.assert_not_called()
+        mock_test_result.assert_called_once_with(123, 1, 12345, '0', 0)
+        mock_g.db.add.assert_called_once()
+        mock_safe_db_commit.assert_called_once()
+
+    @patch('mod_ci.controllers.safe_db_commit')
+    @patch('mod_ci.controllers.g')
+    @patch('mod_ci.controllers.RegressionTest')
+    @patch('mod_ci.controllers.TestResult')
+    def test_invalid_runtime_defaults_to_zero(self, mock_test_result,
+                                              mock_regression_test, mock_g,
+                                              mock_safe_db_commit):
+        mock_log = Mock()
+        mock_test = Mock()
+        mock_test.id = 123
+
+        mock_regression_test_instance = Mock()
+        mock_regression_test_instance.id = 1
+        mock_regression_test_instance.expected_rc = 0
+        mock_regression_test.query.filter.return_value.first.return_value = \
+            mock_regression_test_instance
+
+        mock_request = Mock()
+        mock_request.form = {
+            'test_id': '1',
+            'runTime': 'invalid_string',
+            'exitCode': '0'
+        }
+
+        mock_g.db = Mock()
+        mock_safe_db_commit.return_value = True
+
+        finish_type_request(mock_log, 123, mock_test, mock_request)
+
+        mock_log.warning.assert_called_once_with(
+            "Invalid runtime 'invalid_string' for test 123; storing 0")
+        mock_test_result.assert_called_once_with(123, 1, 0, '0', 0)
+        mock_g.db.add.assert_called_once()
+        mock_safe_db_commit.assert_called_once()
+
+    @patch('mod_ci.controllers.safe_db_commit')
+    @patch('mod_ci.controllers.g')
+    @patch('mod_ci.controllers.RegressionTest')
+    @patch('mod_ci.controllers.TestResult')
+    def test_zero_runtime_unchanged(self, mock_test_result,
+                                    mock_regression_test, mock_g,
+                                    mock_safe_db_commit):
+        mock_log = Mock()
+        mock_test = Mock()
+        mock_test.id = 123
+
+        mock_regression_test_instance = Mock()
+        mock_regression_test_instance.id = 1
+        mock_regression_test_instance.expected_rc = 0
+        mock_regression_test.query.filter.return_value.first.return_value = \
+            mock_regression_test_instance
+
+        mock_request = Mock()
+        mock_request.form = {
+            'test_id': '1',
+            'runTime': '0',
+            'exitCode': '0'
+        }
+
+        mock_g.db = Mock()
+        mock_safe_db_commit.return_value = True
+
+        finish_type_request(mock_log, 123, mock_test, mock_request)
+
+        mock_log.warning.assert_not_called()
+        mock_test_result.assert_called_once_with(123, 1, 0, '0', 0)
+        mock_g.db.add.assert_called_once()
+        mock_safe_db_commit.assert_called_once()
+
+    @patch('mod_ci.controllers.safe_db_commit')
+    @patch('mod_ci.controllers.g')
+    @patch('mod_ci.controllers.RegressionTest')
+    @patch('mod_ci.controllers.TestResult')
+    def test_missing_runtime_defaults_to_zero(self, mock_test_result,
+                                              mock_regression_test, mock_g,
+                                              mock_safe_db_commit):
+        mock_log = Mock()
+        mock_test = Mock()
+        mock_test.id = 123
+
+        mock_regression_test_instance = Mock()
+        mock_regression_test_instance.id = 1
+        mock_regression_test_instance.expected_rc = 0
+        mock_regression_test.query.filter.return_value.first.return_value = \
+            mock_regression_test_instance
+
+        mock_request = Mock()
+        mock_request.form = {
+            'test_id': '1',
+            'exitCode': '0'
+        }
+
+        mock_g.db = Mock()
+        mock_safe_db_commit.return_value = True
+
+        finish_type_request(mock_log, 123, mock_test, mock_request)
+
+        mock_log.warning.assert_not_called()
+        mock_test_result.assert_called_once_with(123, 1, 0, '0', 0)
+        mock_g.db.add.assert_called_once()
+        mock_safe_db_commit.assert_called_once()
+
+
+if __name__ == '__main__':
+    unittest.main()