From 76c015092f32d323ad09a8a85da9b33ba960ff29 Mon Sep 17 00:00:00 2001 From: Omar Abou Selo Date: Fri, 10 May 2024 15:44:27 +0300 Subject: [PATCH] Rerun filtered environments button (#179) * Remove unnecessary artefactId dependency * Add dummy RerunFilteredEnvironmentsButton * Add artefact test executions rerun filter by environment name * Revert "Add dummy RerunFilteredEnvironmentsButton" This reverts commit 4df766535497f748356be8c1083c4aa8efd20d73. * Revert "Add artefact test executions rerun filter by environment name" This reverts commit 48047717d2a81aa8bc9c69fa519abed69971f705. * Revert "Revert "Add dummy RerunFilteredEnvironmentsButton"" This reverts commit b25cf3646d8a79b1680253d78ab89fbebc651e8c. * Revert "Revert "Add artefact test executions rerun filter by environment name"" This reverts commit b72ed72a6debdcb172f3d11dd0f29a997855686b. * extend test executions rerun endpoint to rerun multiple * remove artefact rerun endpoint as it's not necessary * fix seed script * update test executions rerun endpoint * remove unused imports * fix tests * Show filtered test executions confirmation dialog * Remove filteredTestExecutionIds provider * Fix bug * Submit rerun request for filtered test executions * Some code cleanup * Add required trailing comma * Remove unused model * fix test case after rebase * Deal properly with multiple rerun requests * Ignore useless ruff rule * fix black formatting * Use consistent wording as Environment * Only mark successful rerun requests --- backend/pyproject.toml | 2 +- backend/scripts/seed_data.py | 10 +- .../controllers/artefacts/artefacts.py | 27 +----- .../controllers/artefacts/models.py | 5 - .../controllers/test_executions/models.py | 2 +- .../controllers/test_executions/reruns.py | 41 ++++++-- .../controllers/artefacts/test_artefacts.py | 93 ------------------- .../test_executions/test_reruns.py | 62 +++++++++++-- frontend/lib/models/rerun_request.dart | 15 +++ frontend/lib/providers/artefact_builds.dart | 16 ++-- ...ids.dart => filtered_test_executions.dart} | 22 +++-- frontend/lib/repositories/api_repository.dart | 13 ++- .../artefact_build_expandable.dart | 24 ++--- .../ui/artefact_page/artefact_page_body.dart | 16 +++- .../rerun_filtered_environments_button.dart | 85 +++++++++++++++++ .../test_execution_expandable.dart | 58 ++++++++---- ...art => filtered_test_executions_test.dart} | 51 +++++----- 17 files changed, 311 insertions(+), 231 deletions(-) create mode 100644 frontend/lib/models/rerun_request.dart rename frontend/lib/providers/{filtered_test_execution_ids.dart => filtered_test_executions.dart} (81%) create mode 100644 frontend/lib/ui/artefact_page/rerun_filtered_environments_button.dart rename frontend/test/providers/{filtered_test_execution_ids_test.dart => filtered_test_executions_test.dart} (74%) diff --git a/backend/pyproject.toml b/backend/pyproject.toml index f607078f..662dd6f3 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -54,7 +54,7 @@ select = [ "PLE", "TID252", ] -ignore = ["ANN201", "ANN003", "N999", "ANN101", "ANN204"] +ignore = ["ANN201", "ANN003", "N999", "ANN101", "ANN204", "N818"] [tool.ruff.flake8-bugbear] extend-immutable-calls = [ diff --git a/backend/scripts/seed_data.py b/backend/scripts/seed_data.py index a1c97e92..a3657a7d 100644 --- a/backend/scripts/seed_data.py +++ b/backend/scripts/seed_data.py @@ -315,11 +315,11 @@ def seed_data(client: TestClient | requests.Session, session: Session | None = N def _rerun_some_test_executions( client: TestClient | requests.Session, test_executions: list[dict] ) -> None: - for te in test_executions[::2]: - client.post( - RERUN_TEST_EXECUTION_URL, - json={"test_execution_id": te["id"]}, - ).raise_for_status() + te_ids = [te["id"] for te in test_executions[::2]] + client.post( + RERUN_TEST_EXECUTION_URL, + json={"test_execution_ids": te_ids}, + ).raise_for_status() def _add_bugurl_and_duedate(session: Session) -> None: diff --git a/backend/test_observer/controllers/artefacts/artefacts.py b/backend/test_observer/controllers/artefacts/artefacts.py index fbefd221..eb8f0348 100644 --- a/backend/test_observer/controllers/artefacts/artefacts.py +++ b/backend/test_observer/controllers/artefacts/artefacts.py @@ -25,10 +25,9 @@ Artefact, ArtefactBuild, TestExecution, - TestExecutionRerunRequest, ) from test_observer.data_access.models_enums import ArtefactStatus, FamilyName -from test_observer.data_access.repository import get_artefacts_by_family, get_or_create +from test_observer.data_access.repository import get_artefacts_by_family from test_observer.data_access.setup import get_db from .logic import ( @@ -39,7 +38,6 @@ ArtefactBuildDTO, ArtefactDTO, ArtefactPatch, - RerunArtefactTestExecutionsRequest, ) router = APIRouter(tags=["artefacts"]) @@ -146,26 +144,3 @@ def get_artefact_builds(artefact_id: int, db: Session = Depends(get_db)): ) return latest_builds - - -@router.post("/{artefact_id}/reruns") -def rerun_artefact_test_executions( - request: RerunArtefactTestExecutionsRequest | None = None, - artefact: Artefact = Depends(_get_artefact_from_db), - db: Session = Depends(get_db), -): - latest_builds = db.scalars( - queries.latest_artefact_builds.where(ArtefactBuild.artefact_id == artefact.id) - ) - test_executions = (te for ab in latest_builds for te in ab.test_executions) - - if request: - if status := request.test_execution_status: - test_executions = (te for te in test_executions if te.status == status) - if (decision := request.test_execution_review_decision) is not None: - test_executions = ( - te for te in test_executions if set(te.review_decision) == decision - ) - - for te in test_executions: - get_or_create(db, TestExecutionRerunRequest, {"test_execution_id": te.id}) diff --git a/backend/test_observer/controllers/artefacts/models.py b/backend/test_observer/controllers/artefacts/models.py index 78469a27..8f22450e 100644 --- a/backend/test_observer/controllers/artefacts/models.py +++ b/backend/test_observer/controllers/artefacts/models.py @@ -93,8 +93,3 @@ class ArtefactBuildDTO(BaseModel): class ArtefactPatch(BaseModel): status: ArtefactStatus - - -class RerunArtefactTestExecutionsRequest(BaseModel): - test_execution_status: TestExecutionStatus | None = None - test_execution_review_decision: set[TestExecutionReviewDecision] | None = None diff --git a/backend/test_observer/controllers/test_executions/models.py b/backend/test_observer/controllers/test_executions/models.py index 35610cca..36df1821 100644 --- a/backend/test_observer/controllers/test_executions/models.py +++ b/backend/test_observer/controllers/test_executions/models.py @@ -157,7 +157,7 @@ class TestResultDTO(BaseModel): class RerunRequest(BaseModel): - test_execution_id: int + test_execution_ids: set[int] class PendingRerun(BaseModel): diff --git a/backend/test_observer/controllers/test_executions/reruns.py b/backend/test_observer/controllers/test_executions/reruns.py index 08a78db9..2eb8acd8 100644 --- a/backend/test_observer/controllers/test_executions/reruns.py +++ b/backend/test_observer/controllers/test_executions/reruns.py @@ -1,4 +1,6 @@ -from fastapi import APIRouter, Depends, HTTPException +import contextlib + +from fastapi import APIRouter, Depends, HTTPException, Response, status from sqlalchemy import delete, select from sqlalchemy.orm import Session, joinedload @@ -17,14 +19,35 @@ router = APIRouter() -@router.post("/reruns") -def create_a_rerun_request(request: RerunRequest, db: Session = Depends(get_db)): - te = db.get(TestExecution, request.test_execution_id) +@router.post("/reruns", response_model=list[PendingRerun]) +def create_rerun_requests( + request: RerunRequest, response: Response, db: Session = Depends(get_db) +): + rerun_requests = [] + for test_execution_id in request.test_execution_ids: + with contextlib.suppress(_TestExecutionNotFound): + rerun_requests.append(_create_rerun_request(test_execution_id, db)) + + if not rerun_requests: + raise HTTPException( + status.HTTP_404_NOT_FOUND, + "Didn't find test executions with provided ids", + ) + + if len(rerun_requests) != len(request.test_execution_ids): + response.status_code = status.HTTP_207_MULTI_STATUS + + return rerun_requests + + +def _create_rerun_request( + test_execution_id: int, db: Session +) -> TestExecutionRerunRequest: + te = db.get(TestExecution, test_execution_id) if not te: - msg = f"No test execution with id {request.test_execution_id} found" - raise HTTPException(status_code=404, detail=msg) + raise _TestExecutionNotFound - get_or_create(db, TestExecutionRerunRequest, {"test_execution_id": te.id}) + return get_or_create(db, TestExecutionRerunRequest, {"test_execution_id": te.id}) @router.get("/reruns", response_model=list[PendingRerun]) @@ -47,3 +70,7 @@ def delete_rerun_requests(request: DeleteReruns, db: Session = Depends(get_db)): TestExecutionRerunRequest.test_execution_id.in_(request.test_execution_ids) ) ) + + +class _TestExecutionNotFound(ValueError): + ... diff --git a/backend/tests/controllers/artefacts/test_artefacts.py b/backend/tests/controllers/artefacts/test_artefacts.py index aaf50fb4..308f9ea9 100644 --- a/backend/tests/controllers/artefacts/test_artefacts.py +++ b/backend/tests/controllers/artefacts/test_artefacts.py @@ -25,7 +25,6 @@ from test_observer.data_access.models_enums import ( ArtefactStatus, TestExecutionReviewDecision, - TestExecutionStatus, ) from tests.data_generator import DataGenerator @@ -343,95 +342,3 @@ def test_artefact_signoff_ignore_old_build_on_reject( ) assert response.status_code == 400 - - -def test_rerun_all_artefact_test_executions( - test_client: TestClient, test_execution: TestExecution -): - artefact_id = test_execution.artefact_build.artefact_id - - response = test_client.post(f"/v1/artefacts/{artefact_id}/reruns") - - assert response.status_code == 200 - assert test_execution.rerun_request - - -def test_rerun_skips_test_executions_of_old_builds( - test_client: TestClient, generator: DataGenerator -): - a = generator.gen_artefact("candidate") - ab1 = generator.gen_artefact_build(a, revision=1) - ab2 = generator.gen_artefact_build(a, revision=2) - e = generator.gen_environment() - te1 = generator.gen_test_execution(ab1, e) - te2 = generator.gen_test_execution(ab2, e) - - response = test_client.post(f"/v1/artefacts/{a.id}/reruns") - - assert response.status_code == 200 - assert te1.rerun_request is None - assert te2.rerun_request - - -def test_rerun_failed_artefact_test_executions( - test_client: TestClient, generator: DataGenerator -): - a = generator.gen_artefact("candidate") - ab = generator.gen_artefact_build(a) - e1 = generator.gen_environment(name="laptop") - e2 = generator.gen_environment(name="server") - te1 = generator.gen_test_execution(ab, e1) - te2 = generator.gen_test_execution(ab, e2, status=TestExecutionStatus.FAILED) - - response = test_client.post( - f"/v1/artefacts/{a.id}/reruns", - json={"test_execution_status": TestExecutionStatus.FAILED}, - ) - - assert response.status_code == 200 - assert te1.rerun_request is None - assert te2.rerun_request - - -def test_rerun_undecided_artefact_test_executions( - test_client: TestClient, generator: DataGenerator -): - a = generator.gen_artefact("candidate") - ab = generator.gen_artefact_build(a) - e1 = generator.gen_environment(name="laptop") - e2 = generator.gen_environment(name="server") - te1 = generator.gen_test_execution( - ab, e1, review_decision=[TestExecutionReviewDecision.APPROVED_ALL_TESTS_PASS] - ) - te2 = generator.gen_test_execution(ab, e2, review_decision=[]) - - response = test_client.post( - f"/v1/artefacts/{a.id}/reruns", - json={"test_execution_review_decision": []}, - ) - - assert response.status_code == 200 - assert te1.rerun_request is None - assert te2.rerun_request - - -def test_rerun_filters_ignore_review_decisions_order( - test_client: TestClient, test_execution: TestExecution -): - test_execution.review_decision = [ - TestExecutionReviewDecision.APPROVED_INCONSISTENT_TEST, - TestExecutionReviewDecision.APPROVED_FAULTY_HARDWARE, - ] - - response = test_client.post( - f"/v1/artefacts/{test_execution.artefact_build.artefact_id}/reruns", - json={ - "test_execution_review_decision": [ - TestExecutionReviewDecision.APPROVED_FAULTY_HARDWARE, - TestExecutionReviewDecision.APPROVED_INCONSISTENT_TEST, - ] - }, - ) - - assert response.status_code == 200 - assert test_execution.rerun_request diff --git a/backend/tests/controllers/test_executions/test_reruns.py b/backend/tests/controllers/test_executions/test_reruns.py index 0f985ef8..85a68d70 100644 --- a/backend/tests/controllers/test_executions/test_reruns.py +++ b/backend/tests/controllers/test_executions/test_reruns.py @@ -45,13 +45,29 @@ def test_post_no_data_returns_422(post: Post): def test_post_invalid_id_returns_404_with_message(post: Post): - response = post({"test_execution_id": 1}) + response = post({"test_execution_ids": [1]}) assert response.status_code == 404 - assert response.json()["detail"] == "No test execution with id 1 found" + assert response.json()["detail"] == "Didn't find test executions with provided ids" -def test_valid_post_returns_200(post: Post, test_execution: TestExecution): - assert post({"test_execution_id": test_execution.id}).status_code == 200 +def test_valid_post(post: Post, test_execution: TestExecution): + test_execution.ci_link = "ci.link" + response = post({"test_execution_ids": [test_execution.id]}) + + assert response.status_code == 200 + assert response.json() == [ + { + "test_execution_id": test_execution.id, + "ci_link": test_execution.ci_link, + "family": test_execution.artefact_build.artefact.stage.family.name, + } + ] + + +def test_post_with_valid_and_invalid_ids(post: Post, test_execution: TestExecution): + test_execution.ci_link = "ci.link" + response = post({"test_execution_ids": [test_execution.id, test_execution.id + 1]}) + assert response.status_code == 207 def test_get_returns_200_with_empty_list(get: Get): @@ -63,7 +79,7 @@ def test_get_returns_200_with_empty_list(get: Get): def test_get_after_one_post(get: Get, post: Post, test_execution: TestExecution): test_execution.ci_link = "ci.link" - post({"test_execution_id": test_execution.id}) + post({"test_execution_ids": [test_execution.id]}) assert get().json() == [ { @@ -79,8 +95,8 @@ def test_get_after_two_identical_posts( ): test_execution.ci_link = "ci.link" - post({"test_execution_id": test_execution.id}) - post({"test_execution_id": test_execution.id}) + post({"test_execution_ids": [test_execution.id]}) + post({"test_execution_ids": [test_execution.id]}) assert get().json() == [ { @@ -100,8 +116,34 @@ def test_get_after_two_different_posts( e2 = generator.gen_environment("desktop") te2 = generator.gen_test_execution(te1.artefact_build, e2, ci_link="ci2.link") - post({"test_execution_id": te1.id}) - post({"test_execution_id": te2.id}) + post({"test_execution_ids": [te1.id]}) + post({"test_execution_ids": [te2.id]}) + + assert get().json() == [ + { + "test_execution_id": te1.id, + "ci_link": te1.ci_link, + "family": te1.artefact_build.artefact.stage.family.name, + }, + { + "test_execution_id": te2.id, + "ci_link": te2.ci_link, + "family": te2.artefact_build.artefact.stage.family.name, + }, + ] + + +def test_get_after_post_with_two_test_execution_ids( + get: Get, post: Post, generator: DataGenerator +): + a = generator.gen_artefact("beta") + ab = generator.gen_artefact_build(a) + e1 = generator.gen_environment("e1") + e2 = generator.gen_environment("e2") + te1 = generator.gen_test_execution(ab, e1, ci_link="ci1.link") + te2 = generator.gen_test_execution(ab, e2, ci_link="ci2.link") + + post({"test_execution_ids": [te1.id, te2.id]}) assert get().json() == [ { @@ -121,7 +163,7 @@ def test_post_delete_get( get: Get, post: Post, delete: Delete, test_execution: TestExecution ): test_execution.ci_link = "ci.link" - post({"test_execution_id": test_execution.id}) + post({"test_execution_ids": [test_execution.id]}) response = delete({"test_execution_ids": [test_execution.id]}) assert response.status_code == 200 diff --git a/frontend/lib/models/rerun_request.dart b/frontend/lib/models/rerun_request.dart new file mode 100644 index 00000000..5421e1aa --- /dev/null +++ b/frontend/lib/models/rerun_request.dart @@ -0,0 +1,15 @@ +import 'package:freezed_annotation/freezed_annotation.dart'; + +part 'rerun_request.freezed.dart'; +part 'rerun_request.g.dart'; + +@freezed +class RerunRequest with _$RerunRequest { + const factory RerunRequest({ + @JsonKey(name: 'test_execution_id') required int testExecutionId, + @JsonKey(name: 'ci_link') required String ciLink, + }) = _RerunRequest; + + factory RerunRequest.fromJson(Map json) => + _$RerunRequestFromJson(json); +} diff --git a/frontend/lib/providers/artefact_builds.dart b/frontend/lib/providers/artefact_builds.dart index 399a7f6f..780cf165 100644 --- a/frontend/lib/providers/artefact_builds.dart +++ b/frontend/lib/providers/artefact_builds.dart @@ -26,21 +26,21 @@ class ArtefactBuilds extends _$ArtefactBuilds { reviewComment, ); - await _updateTestExecution(testExecutionId, (_) => testExecution); + await _updateTestExecutions({testExecutionId}, (_) => testExecution); } - Future rerunTestExecution(int testExecutionId) async { + Future rerunTestExecutions(Set testExecutionIds) async { final api = ref.read(apiProvider); - await api.rerunTestExecution(testExecutionId); + final rerunRequests = await api.rerunTestExecutions(testExecutionIds); - await _updateTestExecution( - testExecutionId, + await _updateTestExecutions( + rerunRequests.map((rr) => rr.testExecutionId).toSet(), (te) => te.copyWith(isRerunRequested: true), ); } - Future _updateTestExecution( - int testExecutionId, + Future _updateTestExecutions( + Set testExecutionIds, TestExecution Function(TestExecution) update, ) async { final artefactBuilds = await future; @@ -48,7 +48,7 @@ class ArtefactBuilds extends _$ArtefactBuilds { for (final ab in artefactBuilds) { final newTestExecutions = [ for (final te in ab.testExecutions) - if (te.id == testExecutionId) update(te) else te, + if (testExecutionIds.contains(te.id)) update(te) else te, ]; newArtefactBuilds.add(ab.copyWith(testExecutions: newTestExecutions)); diff --git a/frontend/lib/providers/filtered_test_execution_ids.dart b/frontend/lib/providers/filtered_test_executions.dart similarity index 81% rename from frontend/lib/providers/filtered_test_execution_ids.dart rename to frontend/lib/providers/filtered_test_executions.dart index 65ad5063..7ae9159b 100644 --- a/frontend/lib/providers/filtered_test_execution_ids.dart +++ b/frontend/lib/providers/filtered_test_executions.dart @@ -2,25 +2,28 @@ import 'package:dartx/dartx.dart'; import 'package:riverpod_annotation/riverpod_annotation.dart'; import '../models/filters.dart'; -import '../models/test_execution.dart'; import 'artefact_builds.dart'; -part 'filtered_test_execution_ids.g.dart'; +import '../models/test_execution.dart'; +import '../routing.dart'; + +part 'filtered_test_executions.g.dart'; @riverpod -Set filteredTestExecutionIds( - FilteredTestExecutionIdsRef ref, - int artefactId, +List filteredTestExecutions( + FilteredTestExecutionsRef ref, Uri pageUri, ) { + final artefactId = AppRoutes.artefactIdFromUri(pageUri); + final filters = + emptyTestExecutionFilters.copyWithQueryParams(pageUri.queryParametersAll); + final searchValue = pageUri.queryParameters['q'] ?? ''; + final builds = ref.watch(artefactBuildsProvider(artefactId)).requireValue; final testExecutions = [ for (final build in builds) for (final testExecution in build.testExecutions) testExecution, ]; - final filters = - emptyTestExecutionFilters.copyWithQueryParams(pageUri.queryParametersAll); - final searchValue = pageUri.queryParameters['q'] ?? ''; return testExecutions .filter( @@ -28,8 +31,7 @@ Set filteredTestExecutionIds( _testExecutionPassesSearch(testExecution, searchValue) && filters.doesObjectPassFilters(testExecution), ) - .map((te) => te.id) - .toSet(); + .toList(); } bool _testExecutionPassesSearch( diff --git a/frontend/lib/repositories/api_repository.dart b/frontend/lib/repositories/api_repository.dart index a08d0840..54946ae7 100644 --- a/frontend/lib/repositories/api_repository.dart +++ b/frontend/lib/repositories/api_repository.dart @@ -4,6 +4,7 @@ import '../models/artefact.dart'; import '../models/artefact_build.dart'; import '../models/family_name.dart'; +import '../models/rerun_request.dart'; import '../models/test_execution.dart'; import '../models/test_result.dart'; @@ -65,10 +66,16 @@ class ApiRepository { return testResults; } - Future rerunTestExecution(int testExecutionId) async { - await dio.post( + Future> rerunTestExecutions( + Set testExecutionIds, + ) async { + final response = await dio.post( '/v1/test-executions/reruns', - data: {'test_execution_id': testExecutionId}, + data: {'test_execution_ids': testExecutionIds.toList()}, ); + final List rerunsJson = response.data; + final reruns = + rerunsJson.map((json) => RerunRequest.fromJson(json)).toList(); + return reruns; } } diff --git a/frontend/lib/ui/artefact_page/artefact_build_expandable.dart b/frontend/lib/ui/artefact_page/artefact_build_expandable.dart index 45f5fdf6..758af2e4 100644 --- a/frontend/lib/ui/artefact_page/artefact_build_expandable.dart +++ b/frontend/lib/ui/artefact_page/artefact_build_expandable.dart @@ -4,31 +4,27 @@ import 'package:go_router/go_router.dart'; import 'package:intersperse/intersperse.dart'; import '../../models/artefact_build.dart'; -import '../../providers/filtered_test_execution_ids.dart'; +import '../../providers/filtered_test_executions.dart'; import '../spacing.dart'; import 'test_execution_expandable.dart'; class ArtefactBuildExpandable extends ConsumerWidget { - const ArtefactBuildExpandable({ - super.key, - required this.artefactBuild, - required this.artefactId, - }); + const ArtefactBuildExpandable({super.key, required this.artefactBuild}); final ArtefactBuild artefactBuild; - final int artefactId; @override Widget build(BuildContext context, WidgetRef ref) { final pageUri = GoRouterState.of(context).uri; + final revisionText = artefactBuild.revision == null ? '' : ' (${artefactBuild.revision})'; - final filteredTestExecutionIds = - ref.watch(filteredTestExecutionIdsProvider(artefactId, pageUri)); - final filteredTestExecutions = [ - for (final te in artefactBuild.testExecutions) - if (filteredTestExecutionIds.contains(te.id)) te, - ]; + + final filteredTestExecutions = + ref.watch(filteredTestExecutionsProvider(pageUri)).toSet(); + final artefactBuildTestExecutions = artefactBuild.testExecutions.toSet(); + final relevantTestExecutions = + filteredTestExecutions.intersection(artefactBuildTestExecutions); return ExpansionTile( initiallyExpanded: true, @@ -58,7 +54,7 @@ class ArtefactBuildExpandable extends ConsumerWidget { .intersperse(const SizedBox(width: Spacing.level4)), ], ), - children: filteredTestExecutions + children: relevantTestExecutions .map((te) => TestExecutionExpandable(testExecution: te)) .toList(), ); diff --git a/frontend/lib/ui/artefact_page/artefact_page_body.dart b/frontend/lib/ui/artefact_page/artefact_page_body.dart index 15e53ffe..f7541588 100644 --- a/frontend/lib/ui/artefact_page/artefact_page_body.dart +++ b/frontend/lib/ui/artefact_page/artefact_page_body.dart @@ -7,6 +7,7 @@ import '../../providers/artefact_builds.dart'; import '../page_filters/page_filters.dart'; import '../spacing.dart'; import 'artefact_build_expandable.dart'; +import 'rerun_filtered_environments_button.dart'; class ArtefactPageBody extends ConsumerWidget { const ArtefactPageBody({super.key, required this.artefact}); @@ -28,9 +29,17 @@ class ArtefactPageBody extends ConsumerWidget { crossAxisAlignment: CrossAxisAlignment.start, children: [ const SizedBox(height: Spacing.level3), - Text( - 'Environments', - style: Theme.of(context).textTheme.headlineSmall, + Row( + crossAxisAlignment: CrossAxisAlignment.baseline, + textBaseline: TextBaseline.alphabetic, + children: [ + Text( + 'Environments', + style: Theme.of(context).textTheme.headlineSmall, + ), + const Spacer(), + const RerunFilteredEnvironmentsButton(), + ], ), Expanded( child: ListView.builder( @@ -40,7 +49,6 @@ class ArtefactPageBody extends ConsumerWidget { padding: const EdgeInsets.only(right: Spacing.level3), child: ArtefactBuildExpandable( artefactBuild: artefactBuilds[i], - artefactId: artefact.id, ), ), ), diff --git a/frontend/lib/ui/artefact_page/rerun_filtered_environments_button.dart b/frontend/lib/ui/artefact_page/rerun_filtered_environments_button.dart new file mode 100644 index 00000000..9f906151 --- /dev/null +++ b/frontend/lib/ui/artefact_page/rerun_filtered_environments_button.dart @@ -0,0 +1,85 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:go_router/go_router.dart'; +import 'package:intersperse/intersperse.dart'; + +import '../../models/test_execution.dart'; +import '../../providers/artefact_builds.dart'; +import '../../providers/filtered_test_executions.dart'; +import '../../routing.dart'; +import '../spacing.dart'; + +class RerunFilteredEnvironmentsButton extends ConsumerWidget { + const RerunFilteredEnvironmentsButton({super.key}); + + @override + Widget build(BuildContext context, WidgetRef ref) { + final pageUri = GoRouterState.of(context).uri; + final artefactId = AppRoutes.artefactIdFromUri(pageUri); + + final filteredTestExecutions = + ref.watch(filteredTestExecutionsProvider(pageUri)).toList(); + + handlePress() => showDialog( + context: context, + builder: (_) => _ConfirmationDialog( + filteredTestExecutions: filteredTestExecutions, + artefactId: artefactId, + ), + ); + + return TextButton( + onPressed: handlePress, + child: Text( + 'Rerun ${filteredTestExecutions.length} Filtered Environments', + textScaler: const TextScaler.linear(1.2), + ), + ); + } +} + +class _ConfirmationDialog extends ConsumerWidget { + const _ConfirmationDialog({ + required this.filteredTestExecutions, + required this.artefactId, + }); + + final List filteredTestExecutions; + final int artefactId; + + @override + Widget build(BuildContext context, WidgetRef ref) { + handleYes() { + final testExecutionIds = {for (final te in filteredTestExecutions) te.id}; + ref + .read(artefactBuildsProvider(artefactId).notifier) + .rerunTestExecutions(testExecutionIds); + context.pop(); + } + + return AlertDialog( + scrollable: true, + title: Text( + 'Are you sure you want to rerun the following' + ' ${filteredTestExecutions.length} environments?', + ), + content: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: filteredTestExecutions + .map((te) => Text(te.environment.name)) + .intersperse(const SizedBox(height: Spacing.level2)) + .toList(), + ), + actions: [ + TextButton( + onPressed: handleYes, + child: const Text('yes'), + ), + TextButton( + onPressed: () => context.pop(), + child: const Text('no'), + ), + ], + ); + } +} diff --git a/frontend/lib/ui/artefact_page/test_execution_expandable.dart b/frontend/lib/ui/artefact_page/test_execution_expandable.dart index 684feebb..89082610 100644 --- a/frontend/lib/ui/artefact_page/test_execution_expandable.dart +++ b/frontend/lib/ui/artefact_page/test_execution_expandable.dart @@ -97,26 +97,9 @@ class _RerunButton extends ConsumerWidget { ? null : () => showDialog( context: context, - builder: (_) => AlertDialog( - title: const Text( - 'Are you sure you want to rerun this test execution?', - ), - actions: [ - TextButton( - autofocus: true, - onPressed: () { - ref - .read(artefactBuildsProvider(artefactId).notifier) - .rerunTestExecution(testExecution.id); - context.pop(); - }, - child: const Text('yes'), - ), - TextButton( - onPressed: () => context.pop(), - child: const Text('no'), - ), - ], + builder: (_) => _RerunConfirmationDialog( + artefactId: artefactId, + testExecutionId: testExecution.id, ), ); @@ -129,3 +112,38 @@ class _RerunButton extends ConsumerWidget { ); } } + +class _RerunConfirmationDialog extends ConsumerWidget { + const _RerunConfirmationDialog({ + required this.artefactId, + required this.testExecutionId, + }); + + final int artefactId; + final int testExecutionId; + + @override + Widget build(BuildContext context, WidgetRef ref) { + return AlertDialog( + title: const Text( + 'Are you sure you want to rerun this environment?', + ), + actions: [ + TextButton( + autofocus: true, + onPressed: () { + ref + .read(artefactBuildsProvider(artefactId).notifier) + .rerunTestExecutions({testExecutionId}); + context.pop(); + }, + child: const Text('yes'), + ), + TextButton( + onPressed: () => context.pop(), + child: const Text('no'), + ), + ], + ); + } +} diff --git a/frontend/test/providers/filtered_test_execution_ids_test.dart b/frontend/test/providers/filtered_test_executions_test.dart similarity index 74% rename from frontend/test/providers/filtered_test_execution_ids_test.dart rename to frontend/test/providers/filtered_test_executions_test.dart index c5d3764d..1feda2c8 100644 --- a/frontend/test/providers/filtered_test_execution_ids_test.dart +++ b/frontend/test/providers/filtered_test_executions_test.dart @@ -4,7 +4,7 @@ import 'package:testcase_dashboard/models/artefact_build.dart'; import 'package:testcase_dashboard/models/test_execution.dart'; import 'package:testcase_dashboard/providers/api.dart'; import 'package:testcase_dashboard/providers/artefact_builds.dart'; -import 'package:testcase_dashboard/providers/filtered_test_execution_ids.dart'; +import 'package:testcase_dashboard/providers/filtered_test_executions.dart'; import 'package:testcase_dashboard/repositories/api_repository.dart'; import '../dummy_data.dart'; @@ -21,15 +21,16 @@ void main() { // Wait on artefact builds to load cause test execution filters uses requireValue await container.read(artefactBuildsProvider(artefactId).future); - final filteredTestExecutionIds = - container.read(filteredTestExecutionIdsProvider(artefactId, Uri())); + final filteredTestExecutions = container.read( + filteredTestExecutionsProvider(Uri.parse('/snaps/$artefactId')), + ); final builds = await apiMock.getArtefactBuilds(artefactId); - final allTestExecutionIds = { + final allTestExecutions = { for (final build in builds) - for (final testExecution in build.testExecutions) testExecution.id, + for (final testExecution in build.testExecutions) testExecution, }; - expect(filteredTestExecutionIds, allTestExecutionIds); + expect(filteredTestExecutions, allTestExecutions); }); test('it filters test executions by review status', () async { @@ -42,18 +43,19 @@ void main() { // Wait on artefact builds to load cause test execution filters uses requireValue await container.read(artefactBuildsProvider(artefactId).future); - final filteredTestExecutionIds = container.read( - filteredTestExecutionIdsProvider( - artefactId, - Uri( - queryParameters: { - 'Review status': 'Undecided', - }, - ), + final filteredTestExecutions = container.read( + filteredTestExecutionsProvider( + Uri.parse('/snaps/$artefactId?Review+status=Undecided'), ), ); - expect(filteredTestExecutionIds, {1}); + expect(filteredTestExecutions, { + dummyTestExecution.copyWith( + id: 1, + reviewDecision: [], + status: TestExecutionStatus.failed, + ), + }); }); test('it filters test executions by test execution status', () async { @@ -66,18 +68,19 @@ void main() { // Wait on artefact builds to load cause test execution filters uses requireValue await container.read(artefactBuildsProvider(artefactId).future); - final filteredTestExecutionIds = container.read( - filteredTestExecutionIdsProvider( - artefactId, - Uri( - queryParameters: { - 'Execution status': 'Failed', - }, - ), + final filteredTestExecutions = container.read( + filteredTestExecutionsProvider( + Uri.parse('/snaps/$artefactId?Execution+status=Failed'), ), ); - expect(filteredTestExecutionIds, {1}); + expect(filteredTestExecutions, { + dummyTestExecution.copyWith( + id: 1, + reviewDecision: [], + status: TestExecutionStatus.failed, + ), + }); }); }