[Issue #2537] Create e2e tests for subscribe page #3787
Conversation
KeithNava
requested review from
acouch,
andycochran,
btabaska and
doug-s-nava
as code owners
| @@ -101,6 +101,9 @@ const nextConfig = { | |||
| "types", | |||
| ], | |||
| }, | |||
| experimental: { | |||
| testProxy: true, | |||
| await page.getByRole("button", { name: /subscribe/i }).click(); | ||
|
|
||
| await expect(page.getByTestId("errorMessage")).toHaveCount(1); | ||
| await expect( |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
There was a problem hiding this comment.
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 !
There was a problem hiding this comment.
@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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Ok I think I got it stable now without impacting the application logic 👉 https://github.com/HHS/simpler-grants-gov/pull/3787/files#diff-def35e8365b9471bd377c2100868dd944a82954372a3a4efc7e29063be9aa844R63
…thub.com:KeithNava/simpler-grants-gov into KeithNava/2537-add-e2e-tests-for-subscribe-route
Summary
Fixes #2537
Time to review: 15 mins
Changes proposed
Creates the E2E tests for the /subscribe page.
Context for reviewers
Because of some challenges with mocking API calls within NextJS' server components in the current version (v15 as of this PR) we had to utilize some experimental features - https://github.com/vercel/next.js/tree/canary/packages/next/src/experimental/testmode/playwright
Warning
There is some risk in depending on experimental features but this does appear to be the recommended path today
Additional information