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

Setup more robust @replayio/playwright tests #542

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Andarist
Copy link
Member

@Andarist Andarist commented Jun 20, 2024

This PR introduces jest tests for the @replayio/playwright package Those run separate Playwright instances for all tests and use network mocking to do that.

This way we'll be able to create new tests more easily and I hope we'll end up with good control over their executions and assertions. We should also be able to run tests in a watch mode which should make the development much easier

Copy link

changeset-bot bot commented Jun 20, 2024

⚠️ No Changeset found

Latest commit: 27ef01b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@bundled-es-modules/[email protected] None 0 7 kB passle
npm/@bundled-es-modules/[email protected] None 0 6.82 kB passle
npm/@inquirer/[email protected] None 0 9.28 kB sboudrias
npm/@inquirer/[email protected] None 0 86.1 kB sboudrias
npm/@inquirer/[email protected] None 0 59.4 kB sboudrias
npm/@inquirer/[email protected] None 0 5.81 kB sboudrias
npm/@mswjs/[email protected] None 0 66.9 kB kettanaito
npm/@mswjs/[email protected] None 0 1.11 MB kettanaito
npm/@next/[email protected] environment, filesystem 0 11.7 kB vercel-release-bot
npm/@next/[email protected] None 0 115 MB vercel-release-bot
npm/@next/[email protected] None 0 117 MB vercel-release-bot
npm/@next/[email protected] None 0 116 MB vercel-release-bot
npm/@next/[email protected] None 0 140 MB vercel-release-bot
npm/@next/[email protected] None 0 131 MB vercel-release-bot
npm/@next/[email protected] None 0 157 MB vercel-release-bot
npm/@next/[email protected] None 0 102 MB vercel-release-bot
npm/@next/[email protected] None 0 93.9 MB vercel-release-bot
npm/@next/[email protected] None 0 136 MB vercel-release-bot
npm/@open-draft/[email protected] None 0 25.4 kB kettanaito
npm/@open-draft/[email protected] environment 0 24.3 kB kettanaito
npm/@open-draft/[email protected] None 0 10.1 kB kettanaito
npm/@swc/[email protected] None 0 1.18 kB kdy1
npm/@swc/[email protected] None 0 230 kB kdy1
npm/@types/[email protected] None 0 10.1 kB types
npm/@types/[email protected] None 0 4.5 kB types
npm/@types/[email protected] None 0 4.63 kB types
npm/@types/[email protected] None 0 3.07 kB types
npm/[email protected] None 0 124 kB mscdex
npm/[email protected] None 0 611 B sebmarkbage
npm/[email protected] environment 0 1.34 MB benjie
npm/[email protected] None 0 134 kB kettanaito
npm/[email protected] None 0 5.98 kB kettanaito
npm/[email protected] environment, filesystem, shell 0 2.34 MB kettanaito
npm/[email protected] environment, filesystem, network, shell, unsafe 0 86 MB vercel-release-bot
npm/[email protected] None 0 23 kB kettanaito
npm/[email protected] None 0 16.6 kB mscdex
npm/[email protected] None 0 45.7 kB kettanaito
npm/[email protected] environment 0 1.03 MB vercel-release-bot
npm/[email protected] environment, network 0 141 kB lpinca

🚮 Removed packages: npm/[email protected], npm/[email protected]

View full report↗︎

Copy link

🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎

To accept the risk, merge this PR and you will not be notified again.

Alert Package NoteSourceCI
Install scripts npm/[email protected]
  • Install script: postinstall
  • Source: node -e "try{require('./config/scripts/postinstall')}catch(e){}"
🚫
Telemetry npm/[email protected]
  • Note: Can be disabled by setting the environment variable NEXT_TELEMETRY_DISABLED=1 . See https://nextjs.org/telemetry for more information
🚫

View full report↗︎

Next steps

What is an install script?

Install scripts are run when the package is installed. The majority of malware in npm is hidden in install scripts.

Packages should not be running non-essential scripts during install and there are often solutions to problems people solve with install scripts that can be run at publish time instead.

What is telemetry?

This package contains telemetry which tracks how it is used.

Most telemetry comes with settings to disable it. Consider disabling telemetry if you do not want to be tracked.

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/[email protected] or ignore all packages with @SocketSecurity ignore-all

Copy link
Member Author

Choose a reason for hiding this comment

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

I probably should simplify this example page - it's just taken straight from create-next-app

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the core of the mocking logic. We can continue to refine this further. Right now, the test doesn't have control over WS "responses" (nor over regular requests). So It can't, for example, error any of those.

I'm looking for inspiration right now in Cypress, MSW, and Playwright. I think we could establish some conventions for aliases (like @recording[1] and more). We could set up a queue of "route" mocks where each of the mocks would mock the respective request once. I'd really like to find a way to avoid those be sensitive to the order and timing.

} from "@replayio/protocol";
import { WebSocket } from "undici";

async function globalSetup(_config: FullConfig) {
Copy link
Member Author

Choose a reason for hiding this comment

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

I think it will be best to pass a function through this config, any given test could pass a testing function in .use within its own playwright.config.mts and we could call this function from here with the appropriate context object.

This way each test could keep its test and mocking body close to itself (colocation, yay!).

This might require us to stringify the said function but I think it's fine. It's a standard constraint when dealing with page.evaluate and similar APIs as those pass "functions" (in a form of string) across runtime boundaries

Copy link
Member Author

Choose a reason for hiding this comment

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

This reads the list of Next.js routes to create a dynamic list of tests. In each page directory, we can create a playwright config, a playwright test, a custom page, and whatever else we might need to execute that test. Each route defined there should be a self-contained "unit". Each one of them gets executed as a separate playwright run (in an e2e~ fashion).

I'd likely have to introduce support for some pw-test-config.js or similar. We might want to know here some things about any particular test case, such as its expected status.

I'm also thinking about passing some information from a test to this runner here - like maybe some failed assertions? Jest could create better test failure displays based on that than what we can currently get if we'd just throw within failing tests

@@ -1,7 +1,6 @@
import dbg from "./debug";
import WebSocket from "ws";
Copy link
Member Author

Choose a reason for hiding this comment

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

I need to revert this change. It's nice that it's more standard-compliant and it will be used in node itself at some point... but currently it creates warning about WebSocket being experimental and we can't suppress them 😢

This is out of our control:
https://github.com/nodejs/node/blob/9e535b609fbb2d725465b731258d8458d14add44/lib/internal/process/warning.js#L97-L108

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant