Switch from Cypress to Playwright #107
Conversation
|
Nice, can you write comments for every part of the diff? (incl. each config setting, separately) |
playwright.config.ts
Outdated
| webServer: { | ||
| command: 'pnpm start', | ||
| port: 3000, | ||
| reuseExistingServer: !process.env.CI, | ||
| }, |
There was a problem hiding this comment.
To launch a development web server during the tests. It specifies the command to start the server, the port number, and whether to reuse an existing server (reuseExistingServer).
https://playwright.dev/docs/api/class-testconfig#test-config-web-server
There was a problem hiding this comment.
I think we should remove reuseExistingServer - it creates inconsistent behavior where locally, Playwright will not fail if there's a server already running
two potential problems I thought of:
- student runs something else on port 3000 (test will fail from other application not fulfilling the test conditions)
- student runs dev server on port 3000 (test may fail from inconsistencies between development and production environments)
There was a problem hiding this comment.
looking at the latest diff, it appears that reuseExistingServer was removed in 10f606a, although not commented here
There was a problem hiding this comment.
command: 'pnpm start',
should this be changed at all based on the research below?
There was a problem hiding this comment.
Changing the command from pnpm start to /node_modules/.bin/next start will not make any difference if we only test on the build server. The local build server will already have all of the dependencies installed, and there is no need to save memory.
There was a problem hiding this comment.
- it doesn't have to do with dependencies being installed - it's about running with lower RSS (Resident Set Size)
- we should minimize this in every environment, always
There was a problem hiding this comment.
Updated the webServer start command
98d4078
There was a problem hiding this comment.
ok, to be clear, the commit 98d4078 changes the start command to ./node_modules/.bin/next start
the research hasn't been yet completed in upleveled/next-js-example-spring-2023-atvie#10, but to get this merged quicker, we can put this in with the ./node_modules/.bin/next start suggestion for now, so approving this for now 👍 but leaving this conversation unresolved until we finish the research in the future
if we decide to change to output: 'standalone' in the future, then we need to change all of the ./node_modules/.bin/next start occurrences back - I started adding some TODOs for you here: upleveled/next-js-example-spring-2023-atvie#10 (comment)
.github/workflows/playwright.yml
Outdated
| - name: Install Playwright | ||
| run: pnpm playwright install --with-deps chromium |
There was a problem hiding this comment.
install Chromium browser
playwright/navigation.spec.ts
Outdated
|
|
||
| test('Navigate and check page content', async ({ page }) => { | ||
| await page.goto('/'); | ||
| await expect(page.getByTestId('homePageTitle')).toBeVisible(); |
There was a problem hiding this comment.
this check is not consistent with the others that we have:
- it uses
homePageTitleinstead ofpageTitle - it does not check the content of the element
- you may also want to consider moving it to the bottom of the block, to be consistent with the position in the other blocks
There was a problem hiding this comment.
The data-test-id attribute was changed to pageTitle and added an assertion that the h1 is not empty.
commit 8de6c11
| webServer: { | ||
| command: './node_modules/.bin/next start', | ||
| port: 3000, | ||
| }, |
There was a problem hiding this comment.
let's also add stdout: 'pipe' (default is 'ignore', which discards stdout from the process), added in:
- PR: chore: migrate builtin reporters to ReporterV2 microsoft/playwright#23985
- original issue: [Feature] add option to webServer to show logs microsoft/playwright#22454
this can be useful for debugging
There was a problem hiding this comment.
Issue https://github.com/upleveled/courses/issues/1895
Description
This PR replaces the existing Cypress end-to-end tests with Playwright.