diff --git a/mod_ci/controllers.py b/mod_ci/controllers.py index 9b6475ea1..618d31516 100755 --- a/mod_ci/controllers.py +++ b/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) diff --git a/tests/test_ci/test_negative_runtime_fix.py b/tests/test_ci/test_negative_runtime_fix.py new file mode 100644 index 000000000..ea1ee20d7 --- /dev/null +++ b/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()