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

[Issue #2537] Create e2e tests for subscribe page #3787

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open
3 changes: 3 additions & 0 deletions frontend/next.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,9 @@ const nextConfig = {
"types",
],
},
experimental: {
testProxy: true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

🆒

},
};

module.exports = withNextIntl(nextConfig);
2 changes: 1 addition & 1 deletion frontend/src/constants/environments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export const environment: { [key: string]: string } = {
ENVIRONMENT === "prod" ? "https://grants.gov" : "https://test.grants.gov",
NEXT_PUBLIC_BASE_PATH: NEXT_PUBLIC_BASE_PATH ?? "",
USE_SEARCH_MOCK_DATA: USE_SEARCH_MOCK_DATA || "",
SENDY_API_URL: SENDY_API_URL || "",
SENDY_API_URL: SENDY_API_URL || "http://localhost:3000",
SENDY_API_KEY: SENDY_API_KEY || "",
SENDY_LIST_ID: SENDY_LIST_ID || "",
API_URL: API_URL || "",
Expand Down
69 changes: 69 additions & 0 deletions frontend/tests/e2e/subscribe.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/* eslint-disable testing-library/prefer-screen-queries */

import {
expect,
NextFixture,
test,
} from "next/experimental/testmode/playwright";

function mockAPIEndpoints(next: NextFixture, responseText = "1") {
next.onFetch((request: Request) => {
if (request.url.endsWith("/subscribe") && request.method === "POST") {
return new Response(responseText, {
status: 200,
headers: {
"Content-Type": "text/plain",
},
});
}

return "abort";
});
}
test.beforeEach(async ({ page }) => {
await page.goto("/subscribe");
});

test.afterEach(async ({ context }) => {
await context.close();
});

test("has title", async ({ page }) => {
await expect(page).toHaveTitle(/Subscribe | Simpler.Grants.gov/);
});

test("client side errors", async ({ page }) => {
await page.getByRole("button", { name: /subscribe/i }).click();

// Verify client-side errors for required fields
await expect(page.getByTestId("errorMessage")).toHaveCount(2);
await expect(page.getByText("Please enter a name.")).toBeVisible();
await expect(page.getByText("Please enter an email address.")).toBeVisible();
});

test("successful signup", async ({ next, page }) => {
mockAPIEndpoints(next);

await page.getByLabel("First Name (required)").fill("Apple");
await page.getByLabel("Email (required)").fill("name@example.com");

await page.getByRole("button", { name: /subscribe/i }).click();

await expect(
page.getByRole("heading", { name: /you['’]re subscribed/i }),
).toBeVisible();
});

test("error during signup", async ({ next, page }) => {
mockAPIEndpoints(next, "Error with subscribing");

await page.getByLabel("First Name (required)").fill("Apple");
await page.getByLabel("Email (required)").fill("name@example.com");

await page.getByRole("button", { name: /subscribe/i }).click();

await expect(page.getByTestId("errorMessage")).toHaveCount(1);
await expect(
Copy link
Collaborator

Choose a reason for hiding this comment

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

this test seems to be failing, and I think it's because the API request isn't being proxied for some reason. Logging out the requests that come through the proxy locally it seems to be only catching a google analytics request at the moment. My guess is that's because the request for subscription is in a server action, so it isn't being made from the client directly?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's interesting that this test is failing now in CI, but on my local, it seems to be passing and showing that it's mocking the server-side request. I will take a deeper look and will definitely check out MSW. Thanks @doug-s-nava @acouch !
subscribe-spec-e2e

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@doug-s-nava @acouch I think I've figured out what might be happening. I noticed this error in the github actions logs [WebServer] Error subscribing user: Failed to parse URL from /subscribe. I was able to reproduce this error locally by unsetting the SENDY_API_URL variable, as opposed to setting it to http://localhost:3000.

Can you confirm that SENDY_API_URL variable is set to something in the action?

Copy link
Collaborator

Choose a reason for hiding this comment

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

the e2e test action does not have the sendy variables set, so that could definitely be what's going on here. Since we're mocking the request, hopefully the sendy url won't matter, and we can set it to be something random and harmless rather than using the real sendy url.

if you want to test that out, this is where you'd want to look in the code

cat .env.development >> .env.local

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks so much @doug-s-nava I actually was able to get it to work by defaulting to localhost if that SENDY_API_URL variable is not set 4a7113c

Copy link
Collaborator

Choose a reason for hiding this comment

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

Glad you cracked the issue! This change is probably fine but I'd rather avoid any change to the application itself if we can make a change just to the testing configuration. Can you try adding the environment variable to the action definition instead of defaulting within the app itself?

page.getByText(/an error occurred when trying to save your subscription./i),
).toBeVisible();
});