diff --git a/mod_ci/controllers.py b/mod_ci/controllers.py index 647c7814..28acbd79 100755 --- a/mod_ci/controllers.py +++ b/mod_ci/controllers.py @@ -2383,11 +2383,14 @@ def progress_type_request(log, test, test_id, request) -> bool: message = request.form['message'] if len(test.progress) != 0: - last_status = TestStatus.progress_step(test.progress[-1].status) - - if last_status in [TestStatus.completed, TestStatus.canceled]: + last_progress_status = test.progress[-1].status + # Compare enum to enum. progress_step() returns an int index, which + # never matches TestStatus.completed / TestStatus.canceled. + if last_progress_status in [TestStatus.completed, TestStatus.canceled]: return False + last_status = TestStatus.progress_step(last_progress_status) + if last_status > current_status: status = TestStatus.canceled # type: ignore message = "Duplicate Entries" diff --git a/tests/test_ci/test_controllers.py b/tests/test_ci/test_controllers.py index 47a46144..eb58e6aa 100644 --- a/tests/test_ci/test_controllers.py +++ b/tests/test_ci/test_controllers.py @@ -17,7 +17,8 @@ from mod_home.models import CCExtractorVersion, GeneralData from mod_regression.models import (RegressionTest, RegressionTestOutput, RegressionTestOutputFiles) -from mod_test.models import Test, TestPlatform, TestResultFile, TestType +from mod_test.models import (Test, TestPlatform, TestProgress, TestResultFile, + TestStatus, TestType) from tests.base import (BaseTestCase, MockResponse, create_mock_db_query, create_mock_regression_test, empty_github_token, generate_git_api_header, generate_signature, @@ -2115,6 +2116,88 @@ def test_progress_type_request(self, mock_repo, mock_update_build_badge, mock_ge mock_get_compute_service_object.assert_called() self.assertTrue(response) + def test_progress_step_terminal_guard_compares_int_to_enum(self): + """progress_step returns ints; the terminal guard compares them to enums. + + This is the comparison used in progress_type_request: + last_status in [TestStatus.completed, TestStatus.canceled] + """ + completed_step = TestStatus.progress_step(TestStatus.completed) + canceled_step = TestStatus.progress_step(TestStatus.canceled) + terminal_enums = [TestStatus.completed, TestStatus.canceled] + + self.assertEqual(completed_step, 2) + self.assertEqual(canceled_step, -1) + self.assertIsInstance(completed_step, int) + self.assertIsInstance(canceled_step, int) + # Guard as written: an int is never a member of that enum list. + self.assertNotIn(completed_step, terminal_enums) + self.assertNotIn(canceled_step, terminal_enums) + + def test_progress_type_request_rejects_update_after_completed(self): + """A completed run must reject a later progress update. + + Reproduction: if the int-vs-enum guard is live, this returns True + and appends a second completed row instead of False. + """ + from run import log + + self.create_user_with_role( + self.user.name, self.user.email, self.user.password, Role.tester) + self.create_forktest("own-fork-commit", TestPlatform.linux, regression_tests=[2]) + test = Test.query.filter(Test.id == 3).first() + g.db.add(TestProgress(test.id, TestStatus.completed, 'already done')) + g.db.commit() + g.db.refresh(test) + + request = MagicMock() + request.form = {'status': 'completed', 'message': 'Ran all tests again'} + progress_before = len(test.progress) + + with empty_github_token(): + response = progress_type_request(log, test, test.id, request) + + g.db.refresh(test) + rows = TestProgress.query.filter_by(test_id=test.id).order_by(TestProgress.id).all() + statuses = [p.status.value for p in rows] + self.assertFalse( + response, + f'completed should be terminal; got return={response!r} progress={statuses}') + self.assertEqual(len(rows), progress_before) + self.assertEqual(rows[-1].status, TestStatus.completed) + + def test_progress_type_request_rejects_update_after_canceled(self): + """A canceled run must reject a later completed progress update. + + Reproduction: if the int-vs-enum guard is live, this returns True + and appends a completed row after canceled. + """ + from run import log + + self.create_user_with_role( + self.user.name, self.user.email, self.user.password, Role.tester) + self.create_forktest("own-fork-commit", TestPlatform.linux, regression_tests=[2]) + test = Test.query.filter(Test.id == 3).first() + g.db.add(TestProgress(test.id, TestStatus.canceled, 'Canceled by admin')) + g.db.commit() + g.db.refresh(test) + + request = MagicMock() + request.form = {'status': 'completed', 'message': 'Ran all tests'} + progress_before = len(test.progress) + + with empty_github_token(): + response = progress_type_request(log, test, test.id, request) + + g.db.refresh(test) + rows = TestProgress.query.filter_by(test_id=test.id).order_by(TestProgress.id).all() + statuses = [p.status.value for p in rows] + self.assertFalse( + response, + f'canceled should be terminal; got return={response!r} progress={statuses}') + self.assertEqual(len(rows), progress_before) + self.assertEqual(rows[-1].status, TestStatus.canceled) + @mock.patch('mod_ci.controllers.g') @mock.patch('mod_ci.controllers.TestResultFile') @mock.patch('mod_ci.controllers.RegressionTestOutput')