Skip to content
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

[api] migrate testing framework (Jest > Vitest) #3555

Merged
merged 5 commits into from
Aug 29, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 3 additions & 16 deletions api.planx.uk/.eslintrc
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
{
"root": true,
"parser": "@typescript-eslint/parser",
"plugins": ["@typescript-eslint", "jest"],
"plugins": ["@typescript-eslint"],
"extends": [
"eslint:recommended",
"plugin:@typescript-eslint/recommended",
"prettier",
"plugin:jest/recommended"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super happy for this to be a follow up PR - it might be worth looking at something like https://www.npmjs.com/package/eslint-plugin-vitest to replace this?

"prettier"
],
"rules": {
"@typescript-eslint/no-explicit-any": "warn",
@@ -18,18 +17,6 @@
}
],
"@typescript-eslint/no-non-null-assertion": "off",
"jest/expect-expect": [
"error",
{
"assertFunctionNames": [
"expect",
// Allow Supertest expect() calls
"get.expect",
"post.expect",
"supertest.**.expect"
]
}
],
"no-nested-ternary": "error"
},
"globals": {
@@ -44,7 +31,7 @@
"URLSearchParams": "readonly",
"exports": "readonly",
"fetch": "readonly",
"jest": "readonly",
"vi": "readonly",
"test": "readonly",
"describe": "readonly",
"it": "readonly",
2 changes: 1 addition & 1 deletion api.planx.uk/client/index.test.ts
Original file line number Diff line number Diff line change
@@ -8,7 +8,7 @@ test("getClient() throws an error if a store is not set", () => {
});

test("getClient() returns a client if store is set", () => {
const getStoreMock = jest.spyOn(userContext, "getStore");
const getStoreMock = vi.spyOn(userContext, "getStore");
getStoreMock.mockReturnValue({
user: {
sub: "123",
49 changes: 0 additions & 49 deletions api.planx.uk/jest.config.ts

This file was deleted.

11 changes: 0 additions & 11 deletions api.planx.uk/jest.setup.js

This file was deleted.

58 changes: 33 additions & 25 deletions api.planx.uk/lib/hasura/metadata/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,32 +1,40 @@
import { createScheduledEvent, RequiredScheduledEventArgs } from "./index.js";
import Axios, { AxiosError } from "axios";
import axios from "axios";
import type { Mocked } from "vitest";

jest.mock("axios", () => ({
...jest.requireActual("axios"),
post: jest.fn(),
}));
const mockAxios = Axios as jest.Mocked<typeof Axios>;
describe("Creation of scheduled event", () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrapped tests which I adapted in a describe if it didn't already exist 🫡

vi.mock("axios", async (importOriginal) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vitest has this importOriginal argument which is passed to the factory constructor in a mock call - we can use this to import the actual module (replacing jest.requireActual), as per the docs.

Could also have used vi.importActual here, but using the arg seemed neater.

const actualAxios = await importOriginal<typeof import("axios")>();
return {
default: {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vitest does not assume, like jest, that a returned object is the default export of given module, so we have to be explicit about it, as per the docs.

...actualAxios,
post: vi.fn(),
},
};
});
const mockAxios = axios as Mocked<typeof axios>;

const mockScheduledEvent: RequiredScheduledEventArgs = {
webhook: "test url",
schedule_at: new Date(),
comment: "test comment",
payload: {},
};
const mockScheduledEvent: RequiredScheduledEventArgs = {
webhook: "test url",
schedule_at: new Date(),
comment: "test comment",
payload: {},
};

test("createScheduledEvent returns an error if request fails", async () => {
mockAxios.post.mockRejectedValue(new Error());
await expect(createScheduledEvent(mockScheduledEvent)).rejects.toThrow();
});
test("returns an error if request fails", async () => {
mockAxios.post.mockRejectedValue(new Error());
await expect(createScheduledEvent(mockScheduledEvent)).rejects.toThrow();
});

test("createScheduledEvent returns an error if Axios errors", async () => {
mockAxios.post.mockRejectedValue(new AxiosError());
await expect(createScheduledEvent(mockScheduledEvent)).rejects.toThrow();
});
test("returns an error if axios errors", async () => {
mockAxios.post.mockRejectedValue(new axios.AxiosError());
await expect(createScheduledEvent(mockScheduledEvent)).rejects.toThrow();
});

test("createScheduledEvent returns response data on success", async () => {
mockAxios.post.mockResolvedValue({ data: "test data" });
await expect(createScheduledEvent(mockScheduledEvent)).resolves.toBe(
"test data",
);
test("returns response data on success", async () => {
mockAxios.post.mockResolvedValue({ data: "test data" });
await expect(createScheduledEvent(mockScheduledEvent)).resolves.toBe(
"test data",
);
});
});
44 changes: 26 additions & 18 deletions api.planx.uk/lib/hasura/schema/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,33 @@
import { runSQL } from "./index.js";
import Axios, { AxiosError } from "axios";
import axios from "axios";
import type { Mocked } from "vitest";

jest.mock("axios", () => ({
...jest.requireActual("axios"),
post: jest.fn(),
}));
const mockAxios = Axios as jest.Mocked<typeof Axios>;
describe("runSQL", () => {
vi.mock("axios", async (importOriginal) => {
const actualAxios = await importOriginal<typeof import("axios")>();
return {
default: {
...actualAxios,
post: vi.fn(),
},
};
});
const mockAxios = axios as Mocked<typeof axios>;

const sql = "SELECT * FROM TEST";
const sql = "SELECT * FROM TEST";

test("runSQL returns an error if request fails", async () => {
mockAxios.post.mockRejectedValue(new Error());
await expect(runSQL(sql)).rejects.toThrow();
});
test("returns an error if request fails", async () => {
mockAxios.post.mockRejectedValue(new Error());
await expect(runSQL(sql)).rejects.toThrow();
});

test("runSQL returns an error if Axios errors", async () => {
mockAxios.post.mockRejectedValue(new AxiosError());
await expect(runSQL(sql)).rejects.toThrow();
});
test("returns an error if Axios errors", async () => {
mockAxios.post.mockRejectedValue(new axios.AxiosError());
await expect(runSQL(sql)).rejects.toThrow();
});

test("runSQL returns response data on success", async () => {
mockAxios.post.mockResolvedValue({ data: "test data" });
await expect(runSQL(sql)).resolves.toBe("test data");
test("returns response data on success", async () => {
mockAxios.post.mockResolvedValue({ data: "test data" });
await expect(runSQL(sql)).resolves.toBe("test data");
});
});
2 changes: 1 addition & 1 deletion api.planx.uk/lib/notify/index.test.ts
Original file line number Diff line number Diff line change
@@ -2,7 +2,7 @@ import { sendEmail } from "./index.js";
import { NotifyClient } from "notifications-node-client";
import { NotifyConfig } from "../../types.js";

jest.mock("notifications-node-client");
vi.mock("notifications-node-client");

const TEST_EMAIL = "[email protected]";
const mockConfig: NotifyConfig = {
10 changes: 6 additions & 4 deletions api.planx.uk/modules/admin/session/csv.test.ts
Original file line number Diff line number Diff line change
@@ -5,16 +5,16 @@ import { authHeader } from "../../../tests/mockJWT.js";
const endpoint = (strings: TemplateStringsArray) =>
`/admin/session/${strings[0]}/csv`;

const mockGenerateCSVData = jest.fn().mockResolvedValue([
const mockGenerateCSVData = vi.fn().mockResolvedValue([
{
question: "Is this a test?",
responses: [{ value: "Yes" }],
metadata: {},
},
]);
jest.mock("@opensystemslab/planx-core", () => {
vi.mock("@opensystemslab/planx-core", () => {
return {
CoreDomainClient: jest.fn().mockImplementation(() => ({
CoreDomainClient: vi.fn().mockImplementation(() => ({
export: {
csvData: () => mockGenerateCSVData(),
},
@@ -23,7 +23,9 @@ jest.mock("@opensystemslab/planx-core", () => {
});

describe("CSV data admin endpoint", () => {
afterEach(() => jest.clearAllMocks());
afterEach(() => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hooks like afterEach have to return null or undefined, so have remove implicit return, as per docs

vi.clearAllMocks();
});
const auth = authHeader({ role: "platformAdmin" });

it("requires a user to be logged in", async () => {
Original file line number Diff line number Diff line change
@@ -6,13 +6,13 @@ import { expectedPlanningPermissionPayload } from "../../../tests/mocks/digitalP
const endpoint = (strings: TemplateStringsArray) =>
`/admin/session/${strings[0]}/digital-planning-application`;

const mockGenerateDigitalPlanningApplicationPayload = jest
const mockGenerateDigitalPlanningApplicationPayload = vi
.fn()
.mockResolvedValue(expectedPlanningPermissionPayload);

jest.mock("@opensystemslab/planx-core", () => {
vi.mock("@opensystemslab/planx-core", () => {
return {
CoreDomainClient: jest.fn().mockImplementation(() => ({
CoreDomainClient: vi.fn().mockImplementation(() => ({
export: {
digitalPlanningDataPayload: () =>
mockGenerateDigitalPlanningApplicationPayload(),
8 changes: 5 additions & 3 deletions api.planx.uk/modules/admin/session/html.test.ts
Original file line number Diff line number Diff line change
@@ -5,7 +5,7 @@ import { authHeader } from "../../../tests/mockJWT.js";
const endpoint = (strings: TemplateStringsArray) =>
`/admin/session/${strings[0]}/html`;

const mockGenerateHTMLData = jest.fn().mockResolvedValue({
const mockGenerateHTMLData = vi.fn().mockResolvedValue({
responses: [
{
question: "Is this a test?",
@@ -15,7 +15,7 @@ const mockGenerateHTMLData = jest.fn().mockResolvedValue({
],
redactedResponses: [],
});
jest.mock("../../../client", () => {
vi.mock("../../../client", () => {
return {
$api: {
export: {
@@ -26,7 +26,9 @@ jest.mock("../../../client", () => {
});

describe("HTML data admin endpoint", () => {
afterEach(() => jest.clearAllMocks());
afterEach(() => {
vi.clearAllMocks();
});

it("requires a user to be logged in", () => {
return supertest(app)
4 changes: 2 additions & 2 deletions api.planx.uk/modules/admin/session/oneAppXML.test.ts
Original file line number Diff line number Diff line change
@@ -5,11 +5,11 @@ import { authHeader } from "../../../tests/mockJWT.js";
const endpoint = (strings: TemplateStringsArray) =>
`/admin/session/${strings[0]}/xml`;

const mockGenerateOneAppXML = jest
const mockGenerateOneAppXML = vi
.fn()
.mockResolvedValue("<dummy:xml></dummy:xml>");

jest.mock("../../../client", () => {
vi.mock("../../../client", () => {
return {
$api: {
export: {
4 changes: 3 additions & 1 deletion api.planx.uk/modules/admin/session/summary.test.ts
Original file line number Diff line number Diff line change
@@ -19,7 +19,9 @@ describe("Session summary admin endpoint", () => {
});
});

afterEach(() => jest.clearAllMocks());
afterEach(() => {
vi.clearAllMocks();
});

it("requires a user to be logged in", async () => {
await supertest(app)
10 changes: 6 additions & 4 deletions api.planx.uk/modules/admin/session/zip.test.ts
Original file line number Diff line number Diff line change
@@ -2,18 +2,20 @@ import supertest from "supertest";
import app from "../../../server.js";
import { authHeader } from "../../../tests/mockJWT.js";

jest.mock("../../send/utils/exportZip", () => ({
buildSubmissionExportZip: jest.fn().mockResolvedValue({
vi.mock("../../send/utils/exportZip", () => ({
buildSubmissionExportZip: vi.fn().mockResolvedValue({
filename: "tests/mocks/test.zip",
remove: jest.fn,
remove: vi.fn,
}),
}));

const endpoint = (strings: TemplateStringsArray) =>
`/admin/session/${strings[0]}/zip`;

describe("zip data admin endpoint", () => {
afterEach(() => jest.clearAllMocks());
afterEach(() => {
vi.clearAllMocks();
});

it("requires a user to be logged in", async () => {
await supertest(app)
4 changes: 2 additions & 2 deletions api.planx.uk/modules/auth/service.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { checkUserCanAccessEnv } from "./service.js";

const mockIsStagingOnly = jest.fn();
const mockIsStagingOnly = vi.fn();

jest.mock("../../client", () => {
vi.mock("../../client", () => {
return {
$api: {
user: {
Loading

Unchanged files with check annotations Beta

if (template === "save")
returnValue.expiryDate = config.personalisation.expiryDate;
return returnValue;
} catch (error: any) {

Check warning on line 73 in api.planx.uk/lib/notify/index.ts

GitHub Actions / Run API Tests

Unexpected any. Specify a different type
const notifyError = error?.response?.data?.errors?.length
? JSON.stringify(error?.response?.data?.errors?.[0])
: error?.message;
return done({
status: 404,
message: `User (${email}) not found. Do you need to log in to a different Google Account?`,
} as any);

Check warning on line 20 in api.planx.uk/modules/auth/strategy/google.ts

GitHub Actions / Run API Tests

Unexpected any. Specify a different type
}
done(null, { jwt });
const uploadAndLabelFileTypes = uploadAndLabelNodes
.map(([_nodeId, node]: [string, Node]) => node.data?.fileTypes)
.flat();
return uploadAndLabelFileTypes?.map((file: any) => file?.fn as string);

Check warning on line 73 in api.planx.uk/modules/flows/validate/service/fileTypes.ts

GitHub Actions / Run API Tests

Unexpected any. Specify a different type
};
export { validateFileTypes };
fn: PASSPORT_FN,
value: true,
text: `is on a Classified Road`,
data: features.map((feature: any) => ({

Check warning on line 129 in api.planx.uk/modules/gis/service/classifiedRoads.ts

GitHub Actions / Run API Tests

Unexpected any. Specify a different type
name: `${feature.properties["RoadName1"]} - ${feature.properties["RoadClassification"]}`,
entity: feature.properties["GmlID"], // match Planning Data "entity" identifier for convenience when reporting inaccurate constraints
properties: feature.properties,
},
} as GISResponse);
}
} catch (error: any) {

Check warning on line 151 in api.planx.uk/modules/gis/service/classifiedRoads.ts

GitHub Actions / Run API Tests

Unexpected any. Specify a different type
return next({
message: "Failed to fetch classified roads: " + error?.message,
});
options,
)}${datasets}`;
const res = await fetch(url)
.then((response: { json: () => any }) => response.json())

Check warning on line 100 in api.planx.uk/modules/gis/service/digitalLand.ts

GitHub Actions / Run API Tests

Unexpected any. Specify a different type
.catch((error: any) => console.log(error));

Check warning on line 101 in api.planx.uk/modules/gis/service/digitalLand.ts

GitHub Actions / Run API Tests

Unexpected any. Specify a different type
// if analytics are "on", store an audit record of the raw response
if (extras?.analytics !== "false") {
// check for & add any 'positive' constraints to the formattedResult
let formattedResult: Record<string, Constraint> = {};
if (res && res.count > 0 && res.entities) {
res.entities.forEach((entity: { dataset: any }) => {

Check warning on line 135 in api.planx.uk/modules/gis/service/digitalLand.ts

GitHub Actions / Run API Tests

Unexpected any. Specify a different type
// get the planx variable that corresponds to this entity's 'dataset', should never be null because our initial request is filtered on 'dataset'
const key = Object.keys(baseSchema).find((key) =>
baseSchema[key]["digital-land-datasets"]?.includes(entity.dataset),
formattedResult["designated.nationalPark"] &&
formattedResult["designated.nationalPark"].value
) {
formattedResult["designated.nationalPark"]?.data?.forEach((entity: any) => {

Check warning on line 183 in api.planx.uk/modules/gis/service/digitalLand.ts

GitHub Actions / Run API Tests

Unexpected any. Specify a different type
if (
baseSchema[broads]["digital-land-entities"]?.includes(entity.entity)
) {
// loop through any intersecting a4 data entities and set granular planx values based on this local authority's schema
if (a4s && formattedResult["article4"].value) {
formattedResult["article4"]?.data?.forEach((entity: any) => {

Check warning on line 240 in api.planx.uk/modules/gis/service/digitalLand.ts

GitHub Actions / Run API Tests

Unexpected any. Specify a different type
Object.keys(a4s)?.forEach((key) => {
if (
// these are various ways we link source data to granular planx values (see local_authorities/metadata for specifics)
await page.getByText("Continue").click();
await page.getByLabel("email").fill(context.user.email);
await page.getByText("Continue").click();
await page.waitForLoadState("networkidle");

Check warning on line 93 in e2e/tests/ui-driven/src/invite-to-pay/agent.spec.ts

GitHub Actions / E2E tests

Unexpected use of networkidle
await expect(toggleInviteToPayButton).toBeDisabled();
});
const invalidPaymentRequestURL = `/${context.team!.slug!}/${context.flow!
.slug!}/pay?analytics=false&paymentRequestId=INVALID-ID`;
await page.goto(invalidPaymentRequestURL);
await page.waitForLoadState("networkidle");

Check warning on line 87 in e2e/tests/ui-driven/src/invite-to-pay/nominee.spec.ts

GitHub Actions / E2E tests

Unexpected use of networkidle
await expect(page.getByText(PAYMENT_NOT_FOUND_TEXT)).toBeVisible();
});
const invalidPaymentRequestURL = `/${context.team!.slug!}/${context.flow!
.slug!}/pay?analytics=false`;
await page.goto(invalidPaymentRequestURL);
await page.waitForLoadState("networkidle");

Check warning on line 96 in e2e/tests/ui-driven/src/invite-to-pay/nominee.spec.ts

GitHub Actions / E2E tests

Unexpected use of networkidle
await expect(page.getByText(PAYMENT_NOT_FOUND_TEXT)).toBeVisible();
});
const paymentRequestURL = `/${context.team!.slug!}/${context.flow!
.slug!}/pay?analytics=false&paymentRequestId=${paymentRequest.id}`;
await page.goto(paymentRequestURL);
await page.waitForLoadState("networkidle");

Check warning on line 131 in e2e/tests/ui-driven/src/invite-to-pay/nominee.spec.ts

GitHub Actions / E2E tests

Unexpected use of networkidle
}
async function setupPaymentRequest(