WIP: Update backend to support test execution review

backend/migrations/versions/2023_12_06_1316-889ba2349ffa_add_review_status_and_comment.py

@@ -0,0 +1,37 @@
+"""Add review status and comment
+
+Revision ID: 889ba2349ffa
+Revises: 871eec26dc90
+Create Date: 2023-12-06 13:16:07.171288+00:00
+
+"""
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision = '889ba2349ffa'
+down_revision = '871eec26dc90'
+branch_labels = None
+depends_on = None
+
+
+def upgrade() -> None:
+    op.execute(
+        "CREATE TYPE test_execution_review_status_enum AS "
+        "ENUM('APPROVED_FLAKY_TESTS', 'APPROVED_PROVISION_ERRORS', "
+        "'APPROVED', 'MARKED_AS_FAILED', 'UNDECIDED')"
+    )
+    op.execute(
+        "ALTER TABLE test_execution ADD COLUMN "
+        "review_status test_execution_review_status_enum"
+    )
+    op.add_column(
+        "test_execution", sa.Column("review_comment", sa.String(), nullable=True)
+    )
+
+
+def downgrade() -> None:
+    op.drop_column("test_execution", "review_comment")
+    op.drop_column("test_execution", "review_status")
+    op.execute("DROP TYPE test_execution_review_status_enum")

backend/test_observer/controllers/artefacts/models.py

@@ -19,7 +19,11 @@
 #        Nadzeya Hutsko <[email protected]>
 from pydantic import AliasPath, BaseModel, ConfigDict, Field
 
-from test_observer.data_access.models_enums import ArtefactStatus, TestExecutionStatus
+from test_observer.data_access.models_enums import (
+    ArtefactStatus,
+    TestExecutionReviewStatus,
+    TestExecutionStatus,
+)
 
 
 class ArtefactDTO(BaseModel):
@@ -52,6 +56,8 @@ class TestExecutionDTO(BaseModel):
     c3_link: str | None
     environment: EnvironmentDTO
     status: TestExecutionStatus
+    review_status: TestExecutionReviewStatus
+    review_comment: str | None
 
 
 class ArtefactBuildDTO(BaseModel):

backend/test_observer/controllers/test_executions/models.py

@@ -23,7 +23,11 @@
 
 from pydantic import BaseModel, HttpUrl, field_serializer, model_validator
 
-from test_observer.data_access.models_enums import FamilyName, TestExecutionStatus
+from test_observer.data_access.models_enums import (
+    FamilyName,
+    TestExecutionReviewStatus,
+    TestExecutionStatus,
+)
 
 
 class StartTestExecutionRequest(BaseModel):
@@ -84,3 +88,8 @@ class TestExecutionsPatchRequest(BaseModel):
     c3_link: HttpUrl | None
     ci_link: HttpUrl | None
     status: TestExecutionStatus | None
+
+
+class TestExecutionsReviewPatchRequest(BaseModel):
+    review_status: TestExecutionReviewStatus | None
+    review_comment: str | None

backend/test_observer/controllers/test_executions/test_executions.py

@@ -38,6 +38,7 @@
     EndTestExecutionRequest,
     StartTestExecutionRequest,
     TestExecutionsPatchRequest,
+    TestExecutionsReviewPatchRequest,
 )
 
 router = APIRouter()
@@ -145,3 +146,23 @@ def patch_test_execution(
         test_execution.status = request.status
 
     db.commit()
+
+
+@router.patch("/{id}/review")
+def review_test_execution(
+    id: int,
+    request: TestExecutionsReviewPatchRequest,
+    db: Session = Depends(get_db),
+):
+    test_execution = db.get(TestExecution, id)
+
+    if test_execution is None:
+        raise HTTPException(status_code=404, detail="TestExecution not found")
+
+    if request.review_status is not None:
+        test_execution.review_status = request.review_status
+
+    if request.review_comment is not None:
+        test_execution.review_comment = request.review_comment
+
+    db.commit()

backend/test_observer/data_access/models.py

@@ -31,6 +31,7 @@
 
 from test_observer.data_access.models_enums import (
     ArtefactStatus,
+    TestExecutionReviewStatus,
     TestExecutionStatus,
 )
 
@@ -225,6 +226,10 @@ class TestExecution(Base):
     status: Mapped[TestExecutionStatus] = mapped_column(
         default=TestExecutionStatus.NOT_STARTED
     )
+    review_status: Mapped[TestExecutionReviewStatus] = mapped_column(
+        default=TestExecutionReviewStatus.UNDECIDED
+    )
+    review_comment: Mapped[str | None]
 
     __table_args__ = (UniqueConstraint("artefact_build_id", "environment_id"),)
 

backend/test_observer/data_access/models_enums.py

@@ -35,6 +35,14 @@ class TestExecutionStatus(Enum):
     NOT_TESTED = "NOT_TESTED"
 
 
+class TestExecutionReviewStatus(Enum):
+    APPROVED_FLAKY_TESTS = "APPROVED_FLAKY_TESTS"
+    APPROVED_PROVISION_ERRORS = "APPROVED_PROVISION_ERRORS"
+    APPROVED = "APPROVED"
+    MARKED_AS_FAILED = "MARKED_AS_FAILED"
+    UNDECIDED = "UNDECIDED"
+
+
 class ArtefactStatus(str, Enum):
     APPROVED = "APPROVED"
     MARKED_AS_FAILED = "MARKED_AS_FAILED"

backend/tests/controllers/artefacts/test_artefacts.py

@@ -53,7 +53,7 @@ def test_get_latest_artefacts_by_family(db_session: Session, test_client: TestCl
             "series": relevant_artefact.series,
             "repo": relevant_artefact.repo,
             "stage": relevant_artefact.stage.name,
-            "status": relevant_artefact.status,
+            "status": relevant_artefact.status.value,
         }
     ]
 
@@ -74,7 +74,7 @@ def test_get_artefact(db_session: Session, test_client: TestClient):
         "series": artefact.series,
         "repo": artefact.repo,
         "stage": artefact.stage.name,
-        "status": artefact.status,
+        "status": artefact.status.value,
     }
 
 
@@ -104,6 +104,8 @@ def test_get_artefact_builds(db_session: Session, test_client: TestClient):
                     "ci_link": test_execution.ci_link,
                     "c3_link": test_execution.c3_link,
                     "status": test_execution.status.value,
+                    "review_status": test_execution.review_status.value,
+                    "review_comment": test_execution.review_comment,
                     "environment": {
                         "id": environment.id,
                         "name": environment.name,

backend/tests/controllers/test_executions/test_test_executions.py

@@ -32,9 +32,9 @@
 )
 from test_observer.data_access.models_enums import (
     FamilyName,
+    TestExecutionReviewStatus,
     TestExecutionStatus,
 )
-from tests.helpers import create_artefact
 
 
 def test_creates_all_data_models(db_session: Session, test_client: TestClient):
@@ -232,10 +232,14 @@ def test_updates_test_execution(db_session: Session, test_client: TestClient):
             "ci_link": "http://ci_link/",
             "c3_link": "http://c3_link/",
             "status": TestExecutionStatus.PASSED.name,
+            "review_status": TestExecutionReviewStatus.APPROVED.name,
+            "review_comment": "Flaky tests",
         },
     )
 
     db_session.refresh(test_execution)
     assert test_execution.ci_link == "http://ci_link/"
     assert test_execution.c3_link == "http://c3_link/"
     assert test_execution.status == TestExecutionStatus.PASSED
+    assert test_execution.review_status == TestExecutionReviewStatus.APPROVED
+    assert test_execution.review_comment == "Flaky tests"

frontend/lib/models/test_execution.dart

@@ -16,12 +16,57 @@ class TestExecution with _$TestExecution {
     @JsonKey(name: 'c3_link') required String? c3Link,
     required TestExecutionStatus status,
     required Environment environment,
+    @JsonKey(name: 'review_status')
+    required TestExecutionReviewStatus reviewStatus,
+    @JsonKey(name: 'review_comment') required String? reviewComment,
   }) = _TestExecution;
 
   factory TestExecution.fromJson(Map<String, Object?> json) =>
       _$TestExecutionFromJson(json);
 }
 
+enum TestExecutionReviewStatus {
+  @JsonValue('UNDECIDED')
+  undecided,
+  @JsonValue('APPROVED')
+  accepted,
+  @JsonValue('MARKED_AS_FAILED')
+  markedAsFailed;
+
+  String get name {
+    switch (this) {
+      case undecided:
+        return 'Undecided';
+      case accepted:
+        return 'Approved';
+      case markedAsFailed:
+        return 'Marked as Failed';
+    }
+  }
+
+  Chip get chip {
+    switch (this) {
+      case undecided:
+        return const Chip(
+          label: Text('Undecided'),
+          backgroundColor: Color.fromARGB(255, 168, 166, 166),
+          side: BorderSide.none,
+        );
+      case accepted:
+        return const Chip(
+          label: Text('Approved'),
+          backgroundColor: Color.fromARGB(255, 157, 205, 165),
+          side: BorderSide.none,
+        );
+      case markedAsFailed:
+        return const Chip(
+          label: Text('Marked as failed'),
+          backgroundColor: Color.fromARGB(255, 218, 150, 166),
+        );
+    }
+  }
+}
+
 enum TestExecutionStatus {
   @JsonValue('FAILED')
   failed,

frontend/lib/ui/dashboard/artefact_dialog.dart

@@ -17,6 +17,7 @@ import '../../providers/artefact_builds.dart';
 import '../../routing.dart';
 import '../inline_url_text.dart';
 import '../spacing.dart';
+import 'review_pop_overs.dart';
 
 class ArtefactDialog extends ConsumerWidget {
   const ArtefactDialog({super.key, required this.artefactId});
@@ -41,7 +42,7 @@ class ArtefactDialog extends ConsumerWidget {
               data: (artefact) => Column(
                 crossAxisAlignment: CrossAxisAlignment.start,
                 children: [
-                  _ArtefactHeader(title: artefact.name),
+                  _ArtefactHeader(artefact: artefact),
                   const SizedBox(height: Spacing.level4),
                   _ArtefactInfoSection(artefact: artefact),
                   const SizedBox(height: Spacing.level4),
@@ -61,9 +62,9 @@ class ArtefactDialog extends ConsumerWidget {
 }
 
 class _ArtefactHeader extends StatelessWidget {
-  const _ArtefactHeader({required this.title});
+  const _ArtefactHeader({required this.artefact});
 
-  final String title;
+  final Artefact artefact;
 
   @override
   Widget build(BuildContext context) {
@@ -75,9 +76,16 @@ class _ArtefactHeader extends StatelessWidget {
         ),
       ),
       child: Row(
-        mainAxisAlignment: MainAxisAlignment.spaceBetween,
         children: [
-          Text(title, style: Theme.of(context).textTheme.headlineLarge),
+          Row(
+            children: [
+              Text(artefact.name,
+                  style: Theme.of(context).textTheme.headlineLarge),
+              const SizedBox(height: Spacing.level6),
+              ApproveArtefactButton(artefact: artefact),
+            ],
+          ),
+          const Spacer(),
           InkWell(
             child: const Icon(
               YaruIcons.window_close,
@@ -286,6 +294,10 @@ class _TestExecutionView extends StatelessWidget {
           const Spacer(),
           Row(
             children: [
+              testExecution.reviewStatus.chip,
+              const SizedBox(width: Spacing.level4),
+              TestExecutionReviewButton(testExecution: testExecution),
+              const SizedBox(width: Spacing.level4),
               if (ciLink != null)
                 InlineUrlText(
                   url: ciLink,