diff --git a/mod_api/routes/runs.py b/mod_api/routes/runs.py index 8dd5c27ef..66d5d5477 100644 --- a/mod_api/routes/runs.py +++ b/mod_api/routes/runs.py @@ -33,6 +33,7 @@ expected_regression_ids) from mod_api.utils import get_sort_column, paginated_response, single_response from mod_auth.models import Role +from mod_ci.models import GcpInstance from mod_customized.models import CustomizedTest from mod_home.models import CCExtractorVersion from mod_regression.models import RegressionTest, RegressionTestOutput @@ -700,9 +701,10 @@ def restart_run(run_id): """ Queue a finished or stuck run to be executed again. - Clearing the results and the progress trail is what makes the run - eligible again, because CI picks up tests that have no progress - recorded. The run keeps its id, so existing links stay valid, and the + Clearing the results, the progress trail, and any GcpInstance row is + what makes the run eligible again. CI skips tests that still have a + GcpInstance, so a stuck or running restart would otherwise never be + re-queued. The run keeps its id, so existing links stay valid, and the old results are replaced rather than kept alongside the new ones. Like cancel, this is open to anyone holding runs:write rather than to @@ -724,6 +726,8 @@ def restart_run(run_id): TestResult.test_id == test.id).delete(synchronize_session=False) TestProgress.query.filter( TestProgress.test_id == test.id).delete(synchronize_session=False) + GcpInstance.query.filter( + GcpInstance.test_id == test.id).delete(synchronize_session=False) g.db.commit() g.log.info(f'run {run_id} restarted via API by {g.api_user.id}') diff --git a/mod_test/controllers.py b/mod_test/controllers.py index 4c2477b85..62844bbe5 100644 --- a/mod_test/controllers.py +++ b/mod_test/controllers.py @@ -12,6 +12,7 @@ from exceptions import TestNotFoundException from mod_auth.controllers import check_access_rights, login_required from mod_auth.models import Role +from mod_ci.models import GcpInstance from mod_customized.models import TestFork from mod_home.models import CCExtractorVersion, GeneralData from mod_regression.models import (Category, RegressionTestOutput, @@ -358,7 +359,7 @@ def generate_diff(test_id: int, regression_test_id: int, output_id: int, to_view path = os.path.join(config.get('SAMPLE_REPOSITORY', ''), 'TestResults') request_xhr_key = request.headers.get('X-Requested-With') - if (request_xhr_key == 'XMLHttpRequest' or request.accept_mimetypes['application/json']) and to_view == 1: + if (request_xhr_key == 'XMLHttpRequest' or request.accept_mimetypes.accept_json) and to_view == 1: return result.generate_html_diff(path) elif to_view == 0: diff_html_text = result.generate_html_diff(path, to_view=False) @@ -422,6 +423,7 @@ def restart_test(test_id): TestResultFile.query.filter(TestResultFile.test_id == test.id).delete() TestResult.query.filter(TestResult.test_id == test.id).delete() TestProgress.query.filter(TestProgress.test_id == test.id).delete() + GcpInstance.query.filter(GcpInstance.test_id == test.id).delete() g.db.commit() g.log.info(f"test with id: {test_id} restarted") return redirect(url_for('.by_id', test_id=test.id)) diff --git a/tests/api/test_routes_runs.py b/tests/api/test_routes_runs.py index 4264becbe..074aff6da 100644 --- a/tests/api/test_routes_runs.py +++ b/tests/api/test_routes_runs.py @@ -3,6 +3,7 @@ from flask import g +from mod_ci.models import GcpInstance from mod_test.models import (Fork, Test, TestPlatform, TestProgress, TestResult, TestResultFile, TestStatus, TestType) from tests.api.base import ApiTestCase @@ -217,6 +218,8 @@ def test_cancel_run(self): def test_restart_run(self): token = self.get_token('runs_admin@local.com', 'adminpass123', 'restart1', scopes=['runs:write']) + g.db.add(GcpInstance(f'linux-{self.test_id}', self.test_id)) + g.db.commit() res = self.client.post( f'/api/v1/runs/{self.test_id}/restart', headers={'Authorization': f'Bearer {token}'}) @@ -228,6 +231,9 @@ def test_restart_run(self): TestProgress.query.filter_by(test_id=self.test_id).count(), 0) self.assertEqual( TestResult.query.filter_by(test_id=self.test_id).count(), 0) + # A leftover GcpInstance would keep cron from re-queuing the run. + self.assertEqual( + GcpInstance.query.filter_by(test_id=self.test_id).count(), 0) def test_restart_run_not_found(self): token = self.get_token('runs_admin@local.com', 'adminpass123', diff --git a/tests/test_test/test_controllers.py b/tests/test_test/test_controllers.py index 15a4cade9..1fa043fb9 100644 --- a/tests/test_test/test_controllers.py +++ b/tests/test_test/test_controllers.py @@ -3,6 +3,7 @@ from werkzeug.exceptions import Forbidden, NotFound from mod_auth.models import Role +from mod_ci.models import GcpInstance from mod_regression.models import RegressionTest from mod_test.models import (Test, TestPlatform, TestProgress, TestResult, TestResultFile, TestStatus) @@ -65,12 +66,16 @@ def test_restart_with_permission(self): self.user.name, self.user.email, self.user.password, Role.tester) self.create_forktest("own-fork-commit", TestPlatform.linux, regression_tests=[2]) self.create_completed_regression_t_entries(3, [2]) + from flask import g + g.db.add(GcpInstance('linux-3', 3)) + g.db.commit() with self.app.test_client() as c: response = c.post( '/account/login', data=self.create_login_form_data(self.user.email, self.user.password)) response = c.get('/test/restart_test/3') test = Test.query.filter(Test.id == 3).first() self.assertEqual(test.finished, False) + self.assertEqual(GcpInstance.query.filter_by(test_id=3).count(), 0) def test_restart_fails_on_no_permission(self): """Test failed test restart because of no permission.""" @@ -186,7 +191,7 @@ def test_generate_diff_abort_404(self, mock_request, mock_test_result_file): """Try to generate diff when test file not present.""" from mod_test.controllers import generate_diff - mock_request.accept_mimetypes.best = 'application/json' + mock_request.accept_mimetypes.accept_json = True mock_test_result_file.query.filter.return_value.first.return_value = None with self.assertRaises(NotFound): @@ -198,7 +203,7 @@ def test_generate_diff(self, mock_request, mock_test_result_file): """Test to generate diff.""" from mod_test.controllers import generate_diff - mock_request.accept_mimetypes.best = 'application/json' + mock_request.accept_mimetypes.accept_json = True response = generate_diff(1, 1, 1) @@ -212,7 +217,7 @@ def test_generate_diff_download(self, mock_response, mock_request, mock_test_res """Test to download generated diff.""" from mod_test.controllers import generate_diff - mock_request.accept_mimetypes.best = 'application/json' + mock_request.accept_mimetypes.accept_json = True response = generate_diff(1, 1, 1, to_view=0)