Keep reviews after test execution rerun

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()

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

backend/tests/controllers/test_executions/test_patch.py

@@ -14,7 +14,6 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 
-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):

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

backend/tests/controllers/test_executions/test_start_test.py

@@ -14,9 +14,13 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 
+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
@@ -29,15 +33,25 @@
 )
 from test_observer.data_access.models_enums import (
     FamilyName,
+    TestExecutionReviewDecision,
     TestExecutionStatus,
 )
 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",
@@ -48,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 = (
@@ -98,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",
@@ -119,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")
@@ -147,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 = (
@@ -173,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",
@@ -198,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",
@@ -234,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",
@@ -271,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)
@@ -281,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,
@@ -300,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()
@@ -311,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,
@@ -328,3 +333,36 @@ def test_keep_rerun_request_if_same_ci_link(
     )
 
     assert te.rerun_request
+
+
+def test_rerun_keeps_review_as_is(execute: Execute, 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,
+    )
+
+    execute(
+        {
+            "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