From c2e22f68239e14ba625c9487a8e9fb84324f9139 Mon Sep 17 00:00:00 2001 From: Omar Selo Date: Tue, 7 May 2024 08:07:47 +0000 Subject: [PATCH 1/3] Don't dismiss review after rerunning a test execution --- .../controllers/test_executions/logic.py | 2 -- .../test_executions/test_start_test.py | 35 +++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/backend/test_observer/controllers/test_executions/logic.py b/backend/test_observer/controllers/test_executions/logic.py index a50fa37f..83dcc817 100644 --- a/backend/test_observer/controllers/test_executions/logic.py +++ b/backend/test_observer/controllers/test_executions/logic.py @@ -38,8 +38,6 @@ def reset_test_execution( test_execution.status = TestExecutionStatus.IN_PROGRESS test_execution.ci_link = request.ci_link test_execution.c3_link = None - test_execution.review_decision = [] - test_execution.review_comment = "" if test_execution.rerun_request: db.delete(test_execution.rerun_request) db.commit() diff --git a/backend/tests/controllers/test_executions/test_start_test.py b/backend/tests/controllers/test_executions/test_start_test.py index cc8b414b..ab781d67 100644 --- a/backend/tests/controllers/test_executions/test_start_test.py +++ b/backend/tests/controllers/test_executions/test_start_test.py @@ -29,6 +29,7 @@ ) from test_observer.data_access.models_enums import ( FamilyName, + TestExecutionReviewDecision, TestExecutionStatus, ) from tests.data_generator import DataGenerator @@ -328,3 +329,37 @@ def test_keep_rerun_request_if_same_ci_link( ) assert te.rerun_request + + +def test_rerun_keeps_review_as_is(test_client: TestClient, generator: DataGenerator): + review_comment = "review comment" + review_decision = [TestExecutionReviewDecision.REJECTED] + a = generator.gen_artefact("beta") + ab = generator.gen_artefact_build(a) + e = generator.gen_environment() + te = generator.gen_test_execution( + ab, + e, + ci_link="ci.link", + review_comment=review_comment, + review_decision=review_decision, + ) + + test_client.put( + "/v1/test-executions/start-test", + json={ + "family": a.stage.family.name, + "name": a.name, + "version": a.version, + "revision": ab.revision, + "track": a.track, + "store": a.store, + "arch": ab.architecture, + "execution_stage": a.stage.name, + "environment": e.name, + "ci_link": "different-ci.link", + }, + ) + + assert te.review_comment == review_comment + assert te.review_decision == review_decision From 2c426f6761b5b4910ccafd0d1db8db279c7e7d18 Mon Sep 17 00:00:00 2001 From: Omar Selo Date: Tue, 7 May 2024 08:14:29 +0000 Subject: [PATCH 2/3] Merge duplicate test_execution fixture --- backend/tests/conftest.py | 10 ++++++++++ .../tests/controllers/test_executions/test_patch.py | 12 ------------ .../tests/controllers/test_executions/test_reruns.py | 9 --------- 3 files changed, 10 insertions(+), 21 deletions(-) diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 7ae2986c..a2653585 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -33,6 +33,7 @@ drop_database, ) +from test_observer.data_access.models import TestExecution from test_observer.data_access.setup import get_db from test_observer.main import app from tests.data_generator import DataGenerator @@ -94,3 +95,12 @@ def test_client(db_session: Session) -> TestClient: @pytest.fixture def generator(db_session: Session) -> DataGenerator: return DataGenerator(db_session) + + +@pytest.fixture +def test_execution(generator: DataGenerator) -> TestExecution: + a = generator.gen_artefact("beta") + ab = generator.gen_artefact_build(a) + e = generator.gen_environment() + te = generator.gen_test_execution(ab, e) + return te diff --git a/backend/tests/controllers/test_executions/test_patch.py b/backend/tests/controllers/test_executions/test_patch.py index 5e92aca8..cb9bc867 100644 --- a/backend/tests/controllers/test_executions/test_patch.py +++ b/backend/tests/controllers/test_executions/test_patch.py @@ -14,7 +14,6 @@ # along with this program. If not, see . # -import pytest from fastapi.testclient import TestClient from test_observer.data_access.models import ( @@ -24,17 +23,6 @@ TestExecutionReviewDecision, TestExecutionStatus, ) -from tests.data_generator import DataGenerator - - -@pytest.fixture -def test_execution(generator: DataGenerator) -> TestExecution: - a = generator.gen_artefact("beta") - ab = generator.gen_artefact_build(a) - e = generator.gen_environment() - te = generator.gen_test_execution(ab, e, ci_link="http://cilink") - - return te def test_updates_test_execution(test_client: TestClient, test_execution: TestExecution): diff --git a/backend/tests/controllers/test_executions/test_reruns.py b/backend/tests/controllers/test_executions/test_reruns.py index 56fa9aff..306f2f9e 100644 --- a/backend/tests/controllers/test_executions/test_reruns.py +++ b/backend/tests/controllers/test_executions/test_reruns.py @@ -11,15 +11,6 @@ reruns_url = "/v1/test-executions/reruns" -@pytest.fixture -def test_execution(generator: DataGenerator) -> TestExecution: - a = generator.gen_artefact("beta") - ab = generator.gen_artefact_build(a) - e = generator.gen_environment() - te = generator.gen_test_execution(ab, e) - return te - - @pytest.fixture def post(test_client: TestClient): def post_helper(data: Any) -> Response: # noqa: ANN401 From 9ab0b5ecf1dc46bb730d54cf8a5bf14deed0285a Mon Sep 17 00:00:00 2001 From: Omar Selo Date: Tue, 7 May 2024 08:23:22 +0000 Subject: [PATCH 3/3] Pullout some common code into a fixture --- .../test_executions/test_start_test.py | 81 ++++++++++--------- 1 file changed, 42 insertions(+), 39 deletions(-) diff --git a/backend/tests/controllers/test_executions/test_start_test.py b/backend/tests/controllers/test_executions/test_start_test.py index ab781d67..8aee4940 100644 --- a/backend/tests/controllers/test_executions/test_start_test.py +++ b/backend/tests/controllers/test_executions/test_start_test.py @@ -14,9 +14,13 @@ # along with this program. If not, see . # +from collections.abc import Callable from datetime import date, timedelta +from typing import Any, TypeAlias +import pytest from fastapi.testclient import TestClient +from httpx import Response from sqlalchemy.orm import Session from test_observer.controllers.test_executions.models import StartTestExecutionRequest @@ -34,11 +38,20 @@ ) from tests.data_generator import DataGenerator +Execute: TypeAlias = Callable[[dict[str, Any]], Response] -def test_creates_all_data_models(db_session: Session, test_client: TestClient): - response = test_client.put( - "/v1/test-executions/start-test", - json={ + +@pytest.fixture +def execute(test_client: TestClient) -> Execute: + def execute_helper(data: dict[str, Any]) -> Response: + return test_client.put("/v1/test-executions/start-test", json=data) + + return execute_helper + + +def test_creates_all_data_models(db_session: Session, execute: Execute): + response = execute( + { "family": "snap", "name": "core22", "version": "abec123", @@ -49,7 +62,7 @@ def test_creates_all_data_models(db_session: Session, test_client: TestClient): "execution_stage": "beta", "environment": "cm3", "ci_link": "http://localhost", - }, + } ) artefact = ( @@ -99,11 +112,10 @@ def test_creates_all_data_models(db_session: Session, test_client: TestClient): assert response.json() == {"id": test_execution.id} -def test_invalid_artefact_format(test_client: TestClient): +def test_invalid_artefact_format(execute: Execute): """Artefact with invalid format no store should not be created""" - response = test_client.put( - "/v1/test-executions/start-test", - json={ + response = execute( + { "family": "snap", "name": "core22", "version": "abec123", @@ -120,7 +132,7 @@ def test_invalid_artefact_format(test_client: TestClient): def test_uses_existing_models( db_session: Session, - test_client: TestClient, + execute: Execute, generator: DataGenerator, ): artefact = generator.gen_artefact("beta") @@ -148,9 +160,8 @@ def test_uses_existing_models( ci_link="http://localhost/", ) - test_execution_id = test_client.put( - "/v1/test-executions/start-test", - json=request.model_dump(mode="json"), + test_execution_id = execute( + request.model_dump(mode="json"), ).json()["id"] test_execution = ( @@ -174,13 +185,12 @@ def test_uses_existing_models( def test_new_artefacts_get_assigned_a_reviewer( - db_session: Session, test_client: TestClient, generator: DataGenerator + db_session: Session, execute: Execute, generator: DataGenerator ): user = generator.gen_user() - test_client.put( - "/v1/test-executions/start-test", - json={ + execute( + { "family": "snap", "name": "core22", "version": "abec123", @@ -199,13 +209,12 @@ def test_new_artefacts_get_assigned_a_reviewer( assert artefact.assignee.launchpad_handle == user.launchpad_handle -def test_non_kernel_artefact_due_date(db_session: Session, test_client: TestClient): +def test_non_kernel_artefact_due_date(db_session: Session, execute: Execute): """ For non-kernel snaps, the default due date should be set to now + 10 days """ - test_client.put( - "/v1/test-executions/start-test", - json={ + execute( + { "family": FamilyName.SNAP, "name": "core22", "version": "abec123", @@ -235,13 +244,12 @@ def test_non_kernel_artefact_due_date(db_session: Session, test_client: TestClie assert artefact.due_date == date.today() + timedelta(10) -def test_kernel_artefact_due_date(db_session: Session, test_client: TestClient): +def test_kernel_artefact_due_date(db_session: Session, execute: Execute): """ For kernel artefacts, due date shouldn't be set to default """ - test_client.put( - "/v1/test-executions/start-test", - json={ + execute( + { "family": FamilyName.SNAP, "name": "pi-kernel", "version": "abec123", @@ -272,7 +280,7 @@ def test_kernel_artefact_due_date(db_session: Session, test_client: TestClient): def test_deletes_rerun_request_if_different_ci_link( - test_client: TestClient, generator: DataGenerator + execute: Execute, generator: DataGenerator ): a = generator.gen_artefact("beta") ab = generator.gen_artefact_build(a) @@ -282,9 +290,8 @@ def test_deletes_rerun_request_if_different_ci_link( assert te.rerun_request - test_client.put( - "/v1/test-executions/start-test", - json={ + execute( + { "family": a.stage.family.name, "name": a.name, "version": a.version, @@ -301,9 +308,7 @@ def test_deletes_rerun_request_if_different_ci_link( assert not te.rerun_request -def test_keep_rerun_request_if_same_ci_link( - test_client: TestClient, generator: DataGenerator -): +def test_keep_rerun_request_if_same_ci_link(execute: Execute, generator: DataGenerator): a = generator.gen_artefact("beta") ab = generator.gen_artefact_build(a) e = generator.gen_environment() @@ -312,9 +317,8 @@ def test_keep_rerun_request_if_same_ci_link( assert te.rerun_request - test_client.put( - "/v1/test-executions/start-test", - json={ + execute( + { "family": a.stage.family.name, "name": a.name, "version": a.version, @@ -331,7 +335,7 @@ def test_keep_rerun_request_if_same_ci_link( assert te.rerun_request -def test_rerun_keeps_review_as_is(test_client: TestClient, generator: DataGenerator): +def test_rerun_keeps_review_as_is(execute: Execute, generator: DataGenerator): review_comment = "review comment" review_decision = [TestExecutionReviewDecision.REJECTED] a = generator.gen_artefact("beta") @@ -345,9 +349,8 @@ def test_rerun_keeps_review_as_is(test_client: TestClient, generator: DataGenera review_decision=review_decision, ) - test_client.put( - "/v1/test-executions/start-test", - json={ + execute( + { "family": a.stage.family.name, "name": a.name, "version": a.version,