-
Notifications
You must be signed in to change notification settings - Fork 9
tests: standardize MSW error simulation and reduce handler boilerplate #552
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,44 @@ | ||
| import { HttpResponse } from "msw"; | ||
|
|
||
| export const ENDPOINT_STATUS_API_ERROR_MESSAGE = `The endpoint status is set to "error".`; | ||
|
|
||
| const DEFAULT_ERROR_STATUS = 500; | ||
|
|
||
| /** | ||
| * @deprecated Use {@link createEndpointStatusError} instead. | ||
| * Keeping for backward compatibility during migration. | ||
| */ | ||
| export const ENDPOINT_STATUS_API_ERROR = HttpResponse.json( | ||
| { | ||
| error: "EndpointStatusError", | ||
| message: ENDPOINT_STATUS_API_ERROR_MESSAGE, | ||
| }, | ||
| { status: 500 }, | ||
| { status: DEFAULT_ERROR_STATUS }, | ||
| ); | ||
|
|
||
| /** | ||
| * Creates a fresh JSON-body error response for endpoint-status-driven error | ||
| * simulation. Use this where tests assert on the parsed error body (e.g. | ||
| * package-profiles, ubuntu-pro, activities list). | ||
| * | ||
| * Prefer this over the static `ENDPOINT_STATUS_API_ERROR` constant because | ||
| * response objects should not be shared across handler invocations. | ||
| */ | ||
| export const createEndpointStatusError = (status = DEFAULT_ERROR_STATUS) => | ||
| HttpResponse.json( | ||
| { | ||
| error: "EndpointStatusError", | ||
| message: ENDPOINT_STATUS_API_ERROR_MESSAGE, | ||
| }, | ||
| { status }, | ||
| ); | ||
|
|
||
| /** | ||
| * Creates a fresh null-body error response for endpoint-status-driven error | ||
| * simulation. Use this where the handler originally used | ||
| * `throw new HttpResponse(null, { status: 500 })`. | ||
| */ | ||
| export const createEndpointStatusNetworkError = ( | ||
| status = DEFAULT_ERROR_STATUS, | ||
| ) => new HttpResponse(null, { status }); | ||
|
|
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,15 +1,14 @@ | ||||||||||||
| import { API_URL, API_URL_OLD } from "@/constants"; | ||||||||||||
| import type { Activity, GetActivitiesParams } from "@/features/activities"; | ||||||||||||
| import type { Activity } from "@/features/activities"; | ||||||||||||
| import { getEndpointStatus } from "@/tests/controllers/controller"; | ||||||||||||
| import { | ||||||||||||
| activities, | ||||||||||||
| activityTypes, | ||||||||||||
| INVALID_ACTIVITY_SEARCH_QUERY, | ||||||||||||
| } from "@/tests/mocks/activity"; | ||||||||||||
| import type { ApiPaginatedResponse } from "@/types/api/ApiPaginatedResponse"; | ||||||||||||
| import { http, HttpResponse } from "msw"; | ||||||||||||
| import { ENDPOINT_STATUS_API_ERROR } from "./_constants"; | ||||||||||||
| import { generatePaginatedResponse, isAction } from "./_helpers"; | ||||||||||||
| import { createEndpointStatusError, createEndpointStatusNetworkError } from "./_constants"; | ||||||||||||
| import { generatePaginatedResponse, isAction, shouldApplyEndpointStatus } from "./_helpers"; | ||||||||||||
|
|
||||||||||||
| const STATUS_QUERY_REGEX = /(?:^|\s)status:([^\s]+)/; | ||||||||||||
| const TYPE_QUERY_REGEX = /(?:^|\s)type:([^\s]+)/; | ||||||||||||
|
|
@@ -48,13 +47,14 @@ const parseActivitiesQuery = ( | |||||||||||
| }; | ||||||||||||
|
|
||||||||||||
| export default [ | ||||||||||||
| http.get<never, GetActivitiesParams, ApiPaginatedResponse<Activity>>( | ||||||||||||
| http.get( | ||||||||||||
| `${API_URL}activities`, | ||||||||||||
| async ({ request }) => { | ||||||||||||
| const endpointStatus = getEndpointStatus(); | ||||||||||||
|
|
||||||||||||
| if (endpointStatus.status === "error") { | ||||||||||||
| throw ENDPOINT_STATUS_API_ERROR; | ||||||||||||
| if (shouldApplyEndpointStatus("activities")) { | ||||||||||||
| const { status } = getEndpointStatus(); | ||||||||||||
| if (status === "error") { | ||||||||||||
| throw createEndpointStatusError(); | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| const url = new URL(request.url); | ||||||||||||
|
|
@@ -97,17 +97,15 @@ export default [ | |||||||||||
| }, | ||||||||||||
| ), | ||||||||||||
|
|
||||||||||||
| http.get<{ id: string }, GetActivitiesParams, Activity>( | ||||||||||||
| http.get( | ||||||||||||
| `${API_URL}activities/:id`, | ||||||||||||
| async ({ params: { id } }) => { | ||||||||||||
| const endpointStatus = getEndpointStatus(); | ||||||||||||
|
|
||||||||||||
| if (endpointStatus.status === "error") { | ||||||||||||
| throw new HttpResponse(null, { status: 500 }); | ||||||||||||
| if (shouldApplyEndpointStatus("activities/:id")) { | ||||||||||||
| throw createEndpointStatusNetworkError(); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| return HttpResponse.json<Activity>( | ||||||||||||
| activities.find((activity) => activity.id === parseInt(id)) ?? { | ||||||||||||
| activities.find((activity) => activity.id === parseInt(id as string)) ?? { | ||||||||||||
| activity_status: "succeeded", | ||||||||||||
| approval_time: null, | ||||||||||||
| children: [], | ||||||||||||
|
|
@@ -162,26 +160,22 @@ export default [ | |||||||||||
| ]); | ||||||||||||
| }), | ||||||||||||
|
|
||||||||||||
| http.post<never, { activity_ids: number[] }, number[]>( | ||||||||||||
| http.post( | ||||||||||||
| `${API_URL}activities/revert`, | ||||||||||||
| async () => { | ||||||||||||
| const endpointStatus = getEndpointStatus(); | ||||||||||||
|
|
||||||||||||
| if (endpointStatus.status === "error") { | ||||||||||||
| throw ENDPOINT_STATUS_API_ERROR; | ||||||||||||
| if (shouldApplyEndpointStatus("activities/revert")) { | ||||||||||||
|
||||||||||||
| if (shouldApplyEndpointStatus("activities/revert")) { | |
| if ( | |
| shouldApplyEndpointStatus("activities/revert") && | |
| getEndpointStatus().status === "error" | |
| ) { |
Copilot
AI
Apr 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the activities/reapply handler, shouldApplyEndpointStatus("activities/reapply") currently triggers a 500 for any non-default status (including empty). This changes behavior from only erroring on status === "error". Please fetch the endpoint status and only throw createEndpointStatusError() when status === "error".
| if (shouldApplyEndpointStatus("activities/reapply")) { | |
| const status = getEndpointStatus("activities/reapply"); | |
| if (status === "error") { |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,9 +2,9 @@ import { API_URL, API_URL_OLD } from "@/constants"; | |
| import type { APTSource, GetAPTSourcesParams } from "@/features/apt-sources"; | ||
| import { getEndpointStatus } from "@/tests/controllers/controller"; | ||
| import { aptSources } from "@/tests/mocks/apt-sources"; | ||
| import { isAction } from "@/tests/server/handlers/_helpers"; | ||
| import { isAction, shouldApplyEndpointStatus } from "@/tests/server/handlers/_helpers"; | ||
| import { http, HttpResponse } from "msw"; | ||
| import { ENDPOINT_STATUS_API_ERROR } from "./_constants"; | ||
| import { createEndpointStatusNetworkError } from "./_constants"; | ||
|
|
||
| export default [ | ||
| http.get<never, GetAPTSourcesParams, APTSource[]>( | ||
|
|
@@ -19,15 +19,10 @@ export default [ | |
| ), | ||
|
|
||
| http.delete(`${API_URL}repository/apt-source/:sourceId`, () => { | ||
| const endpointStatus = getEndpointStatus(); | ||
|
|
||
| if ( | ||
| !endpointStatus.path || | ||
| (endpointStatus.path && | ||
| endpointStatus.path === "repository/apt-source/:sourceId") | ||
| ) { | ||
| if (endpointStatus.status === "error") { | ||
| throw HttpResponse.json(ENDPOINT_STATUS_API_ERROR, { status: 500 }); | ||
| if (shouldApplyEndpointStatus("repository/apt-source/:sourceId")) { | ||
| const { status } = getEndpointStatus(); | ||
| if (status === "error") { | ||
| throw createEndpointStatusNetworkError(); | ||
| } | ||
|
Comment on lines
21
to
26
|
||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldApplyEndpointStatus("activities/:id")becomes true for any non-default status (includingempty), but this handler now throws a 500 unconditionally when it matches. Previously onlystatus === "error"triggered the 500. Please readstatusfromgetEndpointStatus()and only throwcreateEndpointStatusNetworkError()whenstatus === "error"(and optionally handleemptyexplicitly if desired).