Skip to content

Commit

Permalink
Fix end-to-end tests post Next.js migration
Browse files Browse the repository at this point in the history
- In order to run either the mock tests or the end-to-end tests, we need
  a dev server running in CI. Previously, we would spawn that manually
  from inside of a script. Now, we start that from inside of the action
  that does the testing. This means that if you want to run the tests
  locally, you'll also need your local dev server running (which was
  already true).
- Add necessary parts of the `test` folder to public. This means that we
  will be able to run against preview branches as well.
- Add a github action for setting up Next.js with a cache, so we only
  have to copy one line everywhere, not an entire step. It would be nice
  if we could further refactor out the test stripes, because a lot of
  things are repeated 9 times.
- Remove the `dev-server.js` file. If we're going to use next.js, then
  we should try and keep our config as out-of-the-box as possible, in
  order to allow us to stay up-to-date. The fact that the `test` folder
  has to be available on the development server for the tests to run at
  all seems like the real problem here. A good follow-on task would be
  to sort out the best way to handle those files in the test/dev
  environment.
- Go back to node 14 for lock file. I didn't realize when I changed this
  that we were stuck on such an outdated node version. We should fix
  this ASAP, but until then, let's stick with what was working, just to
  minimize the factors that have gone into resolving all of the failing
  tests.
- Add a `waitingFor` field to the `waitUntil` method used in tests. It's
  really frustrating when a test fails and all it says is `timed out`.
  Like... timed out while waiting for *what*? Test failures should guide
  you to the place of failure as quickly as possible.
- By default, wait for 10 seconds. If something doesn't appear in the
  DOM in 10 seconds, that should be considered a failure.
- Log more from playwright tests. This could be annoying, maybe we
  should tune it, but it's nice to know what's going on in these tests,
  and we're only going to look at the output when something is failing
  anyways, so it may as well be verbose.
- Log the time that tests take. We started with a lot of timeouts. I
  fixed them, but sometimes it was hard to see what tests were timing
  out. Now, when a test completes, we emit some test stats. Eventually,
  it might be nice to just like... use a real test runner... but this is
  fine for now.
- Mock test URLs like
  http://localhost:8080/recording/24c0bc00-fdad-4e5e-b2ac-23193ba3ac6b?mock=1
  show a gray screen after a few seconds. If you remove the `mock` query
  param then the screen shows up totally normally. I'm not sure of the
  exact interactions here. Let's figure this out in a follow-on task.
- Add `test/run` and `test/mock/run` scripts for running the rest
  runners locally, and taking care of making sure that the files get
  shuffled around correctly before and after, and that the dev server is
  running, etc.
- Add README instructions about `.env` files and new test run scripts.

That's true! We still have flaky, intermittent tests. The goal of this
PR was to get things to *run*, getting them to pass is a future us
problem! I'm disabling the tests that seem to *always* fail for now.

I'm going to put these into a notion doc as well, but documenting here
couldn't hurt. Three things that tripped me up on this PR:

- *any* merge conflicts mean actions *will not run at all*. If you have
  a branch and it's unclear why actions aren't kicking off, make sure
  you check for and resolve merge conflicts.
- When using actions that are type `javascript` your `inputs` get
  automatically turned into environment variables, which is convenient.
  However **when using `composite` actions this won't happen**. You have
  to manually pass the environment variables in with an
  [expression](https://docs.github.com/en/actions/learn-github-actions/expressions).
- The `checkout` action that we use all the time performs a strict
  `clean` of the directory that it's about to clone into! So, if you are
  trying to do some sort of caching or otherwise unloading assets into a
  dir that you also need to do `git checkout XXXX` in (with the checkout
  action), you either need to checkout *first*, or you can use `[clean:
  false](https://github.com/actions/checkout#usage)` I think, though I
  haven't actually tried this.
- Actions are somewhat limited. More details here:
  actions/runner#646

Fixes replayio#4313
  • Loading branch information
ryanjduffy authored and jcmorrow committed Nov 9, 2021
1 parent ffcaa1d commit 489e12f
Show file tree
Hide file tree
Showing 151 changed files with 9,113 additions and 6,832 deletions.
4 changes: 4 additions & 0 deletions .env.sample
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# Configures the graphql endpoint
NEXT_PUBLIC_API_URL=https://api.replay.io/v1/graphql
# Configures the dispatcher websocket endpoint
NEXT_PUBLIC_DISPATCH_URL=wss://dispatch.replay.io
17 changes: 13 additions & 4 deletions .github/actions/mockTest/action.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,15 @@
name: "Mock Test"
name: "Mock Tests"
description: "Run mock tests"

runs:
using: "node12"
main: "./main.js"
using: "composite"
steps:
- name: Copy .env file
run: cp .env.sample .env
shell: bash
- name: Start Next.js development server
run: ./node_modules/.bin/next dev -p 8080 &
shell: bash
- run: node ${{ github.action_path }}/main.js
shell: bash
env:
DEBUG: "pw:api"
30 changes: 1 addition & 29 deletions .github/actions/mockTest/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,42 +5,14 @@ const devtools = `${__dirname}/../../..`;

console.log(new Date(), "Start");

spawnCheckedRetry("npm", ["install"], { cwd: devtools, stdio: "inherit" });
const devServerProcess = spawn("node_modules/.bin/webpack", ["serve"], {
cwd: devtools,
stdio: ["inherit", "pipe", "inherit"],
});

console.log(new Date(), "Installed devtools and started webpack build");

(async function () {
console.log("Waiting for Webpack server start");
await Promise.race([
new Promise(r => {
devServerProcess.stdout.on("data", chunk => {
process.stdout.write(chunk);
// Once the dev server starts outputting stuff, we assume it has started
// its server and it is safe to curl.
r();
});
}),
new Promise(r => setTimeout(r, 10 * 1000)).then(() => {
throw new Error("Failed to start dev server");
}),
]);
console.log("Waiting for Webpack build");

// Wait for the initial Webpack build to complete before
// trying to run the tests so the tests don't run
// the risk of timing out if the build itself is slow.
spawnChecked(
"curl",
["--max-time", "600", "--range", "0-50", "http://localhost:8080/dist/main.js"],
["--max-time", "180", "--range", "0-50", "http://localhost:8080/test/harness.js"],
{
stdio: "inherit",
}
);
console.log("Done Initial Webpack build");

spawnChecked("node", [`${devtools}/test/mock/run`], { stdio: "inherit" });
process.exit(0);
Expand Down
13 changes: 13 additions & 0 deletions .github/actions/npm-install/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
name: "npm install"
description: "Installs dependencies from npm, with caching"
runs:
using: "composite"
steps:
- name: Setup node
uses: actions/setup-node@v2
with:
node-version: "14"
cache: "npm"
- name: Install dependencies
run: npm ci
shell: bash
14 changes: 14 additions & 0 deletions .github/actions/setup-next/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
name: "Setup Next.js"
description: "Restores the .next/cache folder"
runs:
using: "composite"
steps:
- name: "Setup NextJS Cache"
uses: actions/cache@v2
with:
path: ${{ github.workspace }}/.next/cache
# Generate a new cache whenever packages or source files change.
key: ${{ runner.os }}-nextjs-${{ hashFiles('**/package-lock.json') }}-${{ hashFiles('**.[jt]s', '**.[jt]sx') }}
# If source files changed but packages didn't, rebuild from a prior cache.
restore-keys: |
${{ runner.os }}-nextjs-${{ hashFiles('**/package-lock.json') }}-
24 changes: 17 additions & 7 deletions .github/actions/test/action.yml
Original file line number Diff line number Diff line change
@@ -1,13 +1,23 @@
name: "Test"
description: "Run correctness tests"
description: "Run end-to-end tests. Note that this action assumes that Node and Next.js have already been properly setup. See the setup-next task for details."
inputs:
stripe:
description: "Stripe to run, in the form INDEX/TOTAL"
required: true
github_ci:
description: "A flag to tell whether we are in CI"
required: false

runs:
using: "node12"
main: "./main.js"
using: "composite"
steps:
- run: mv node_modules/node_modules.tgz ./
shell: bash
- run: tar -xzf node_modules.tgz
shell: bash
- name: Copy .env file
run: cp .env.sample .env
shell: bash
- name: Start Next.js development server
run: ./node_modules/.bin/next dev -p 8080 &
shell: bash
- run: node ${{ github.action_path }}/main.js
shell: bash
env:
INPUT_STRIPE: ${{ inputs.stripe }}
51 changes: 9 additions & 42 deletions .github/actions/test/main.js
Original file line number Diff line number Diff line change
@@ -1,32 +1,27 @@
const fs = require("fs");
const { spawnSync, spawn } = require("child_process");
const { spawnSync } = require("child_process");

function spawnChecked(...args) {
console.log(`spawn`, args);
console.log(`Spawning`, args);
const rv = spawnSync.apply(this, args);
if (rv.status != 0 || rv.error) {
throw new Error("Spawned process failed");
}
console.log(`Spawned`, args);
}

function checkForFile(path) {
console.log(`Checking for file: ${path}`);
if (!fs.existsSync(path)) {
throw new Error(`Required file/directory does not exist: ${path}`);
}
console.log(`Found ${path}`);
}

checkForFile("dist/dist.tgz");
checkForFile("replay/replay.dmg");
checkForFile("replay-node/macOS-replay-node");
checkForFile("replay-driver/macOS-recordreplay.so");

console.log(new Date(), "Start");

spawnChecked("mv", ["dist/dist.tgz", "dist.tgz"]);
spawnChecked("tar", ["-xzf", "dist.tgz"]);

console.log(new Date(), "Unpackaged distribution");

spawnChecked("hdiutil", ["attach", "replay/replay.dmg"]);
spawnChecked("cp", ["-R", "/Volumes/Replay/Replay.app", "/Applications"]);
try {
Expand All @@ -41,39 +36,11 @@ spawnChecked("chmod", ["+x", "replay-node/macOS-replay-node"]);
process.env.RECORD_REPLAY_NODE = "replay-node/macOS-replay-node";
process.env.RECORD_REPLAY_DRIVER = "replay-driver/macOS-recordreplay.so";

const devServerProcess = spawn("node_modules/.bin/webpack", ["serve"], {
detached: true,
stdio: ["inherit", "pipe", "inherit"],
});

(async function () {
console.log("Waiting for Webpack server start");
await Promise.race([
new Promise(r => {
devServerProcess.stdout.on("data", chunk => {
process.stdout.write(chunk);
// Once the dev server starts outputting stuff, we assume it has started
// its server and it is safe to curl.
r();
});
}),
new Promise(r => setTimeout(r, 10 * 1000)).then(() => {
throw new Error("Failed to start dev server");
}),
]);
console.log("Waiting for Webpack build");

// Wait for the initial Webpack build to complete before
// trying to run the tests so the tests don't run
// the risk of timing out if the build itself is slow.
spawnChecked(
"curl",
["--max-time", "600", "--range", "0-50", "http://localhost:8080/dist/main.js"],
{
stdio: "inherit",
}
);
console.log("Done Initial Webpack build");
spawnChecked("curl", ["--max-time", "180", "--range", "0-50", "http://localhost:8080/"], {
stdio: "inherit",
});
console.log("Made contact with server on localhost:8080");

require("../../../test/run");
})().catch(err => {
Expand Down
Loading

0 comments on commit 489e12f

Please sign in to comment.