Run e2e tests inside a docker container #1057

sarayourfriend · 2022-03-04T17:38:12Z

Fixes

Related to and blocks #1017 by @sarayourfriend

I originally started working on this directly in the same PR to address #1017 but decided to split this work out and apply it to the existing e2e tests to minimize the scope of the visual regression testing PR.

Description

Note about the line count for this PR!

The vast majority of those changes are in the updated tapes due to the sudden introduction of Brotli compression halfway through developing this. The main places to focus reviews on are the following:

playwright.config.ts
docker-compose.playwright.ts
proxy.js

Rationale

In order for visual regression tests to run in docker (to alleviate cross-platform rendering differences causing false-positives in the snapshot tests) we need to be able to have the full playwright stack working in docker due to networking complications.

Because our app is SSR and client side rendered, there are two contexts in which API requests are made:

In the SSR application
In the docker contained headless browser that is being controlled by playwright

If we just run playwright inside of docker, then we run into an issue where our API url has to be different for the client and the server, because the server will be on the host network and can use localhost, whereas the client will be in the docker network and will need to use something like host.docker.internal. This complicates the code and also makes debugging difficult.

Solution

To fix this, we run everything inside a single Playwright docker container so that they all share the same network and (crucially) can communicate over plain old localhost. Without being able to use localhost (i.e., relying on docker-compose networking) we can lots of weird SSL errors with the browser automatically redirecting requests to places like http://web or http://api to https. These fail because we don't have SSL set up locally.

Instead, I discovered that Playwright actually has built into it the ability to run and wait for a server to start! This means we can run the talkback proxy and the Nuxt server all inside the same container as Playwright, still get the benefits of the Playwright container's browser, all without the hassle of setting up local SSL for everyone trying to run the e2e tests locally.

The only snag here is that writing new tapes from talkback needs to have some permissions juggling fixed for Linux. Apparently this is a non-issue on macOS and Windows because the volumes are shared in a different way, but on Linux files written by docker containers into the host will all have root permissions. To fix this, we can just run chmod on them after updating the tapes. To prevent having to run a sudo script inside our package.json, I chose to just use the node:alpine image to run chmod as the image will already be present on your machine if you run the e2e tests because it's used as the base image for the running Nuxt app. I think we could also use the Playwright image but we need to interpolate the Playwright version to avoid downloading extraneous versions and node:alpine doens't have that problem.

I had to use npm-run-all because pre/post scripts in npm do not run if the main script fails, so if you try to update tapes and the tests don't pass, then the permissions fix won't run. npm-run-all with the --continue-on-error flag set will run all the scripts in sequence even if one of them fails while also still producing a non-zero exit code if any of the scripts produce non-zero exit codes.

Meta commentary about the PR

There are a few long caveat laden comments that needed to be added to explain everything that was going on, specifically with a few edge cases here and there. If when reviewing you find any other places that need clarification, don't hesitate to ask me to add more comments. Given the complexity and depth of this change I think more explanatory comments are best, for our own sanity's sake down the line when we're trying to understand how this all works again.

I actually think that the solution I landed on is ultimately simpler than the solutions I explored along the way. For example, this still uses docker-compose to run the Playwright container, but that's mostly just so that we don't have to stuff all the options into a long docker run line.

Related fixes

Because I was debugging all sorts of broken https requests, I was carefully inspecting the tapes and requests being made by the frontend. I discovered that the thumbnail routes were not being proxied. This PR also fixes that (hence the enormous number of new tapes being added).

Halfway through working on this I ran into an issue where some responses that were being recorded by talkback when updating/creating new tapes were compressed. Talkback, for some reason, doesn't decompress them for you. That's fine because zlib is built into Node anyway and we can easily decompress and recompress responses. I don't know why we didn't run into this previously but as I mentioned in the comment in proxy.js I think it's actually good that we're dealing with it because it more closely mirrors the situation in production.

It does, however, raise the question of why some responses aren't Brotli compressed and others are.

In light of @obulat's existing PR to fix up some of the tape generation logic in #1045 we can hold off on merging this PR until her changes are in. I think it'll be easier to merge in her changes to this PR than the other way around. @obulat feel free to correct me if you think the other way will be easier! I'm not sure if your PR is running into the same Brotli compression issue I ran into with this one, but if so then the other way around (merge this first then rebase the other branch) might be easier. I will leave that up to you though, Olga.

Testing Instructions

Checkout this PR and run pnpm test:e2e and pnpm test:e2e:update-tapes and ensure that they work (i.e., that talkback responses are being used/created, file permissions are set correctly, etc). Also try pnpm test:e2e:debug with and without a local Nuxt server running. Both should work. pnpm generate-playwright-tests is unaffected, but I renamed it in anticipation of visual-regression being added and the generate script no longer being specific to just "e2e" tests.

Checklist

My pull request has a descriptive title (not a vague title like Update index.md).
My pull request targets the default branch of the repository (main) or a parent feature branch.
My commit messages follow best practices.
My code follows the established code style of the repository.
I added or updated tests for the changes I made (if applicable).
I added or updated documentation (if applicable).
I tried running the project locally and verified that there are no visible errors.

Developer Certificate of Origin

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

@todo

@todo(sarayourfriend): Find a way to de-duplicate the docker-compose invocation. Perhaps a separate bash script in `bin`?

sarayourfriend · 2022-03-04T19:03:52Z

I've gotten very far with this. I think the final issue that remains is to fix the various SSL errors that happen. You can see these by modifying the playwright.config.js to include use: { trace: 'on' }, extracting the trace.zip created in test-results and opening the trace.trace file. Search for https and you'll find several requests to both https://api:3000 and https://web:8443. I'm not sure what is causing this to be honest but I ran into it with the visual regression PR I was working on as well.

As I see it there are really two solutions:

Either somehow force http to be used across the board (I'm assuming this is the easiest solution)
Allow SSL to be used between containers, potentially by using a reverse proxy like caddy between them (probably a little harder BUT would be more in line with production so could be more desirable if the effort is low)

I plan to tackle these in reverse order. If it's possible to get SSL working between the containers without too much effort, then I think that is preferable because as I mentioned it is closer to production anyway. I suspect the first solution will also require some hacking/brute force solution that would be best to avoid.

sarayourfriend · 2022-03-05T04:20:55Z

test/e2e/translation-banner.spec.js

-  const banner = await page.locator(
-    '.span:has-text("Help us get to 100 percent")'
-  )
+  const banner = page.locator('.span:has-text("Help us get to 100 percent")')


page.locator doesn't return a Promise. Completely unrelated to this PR but it was bugging me while I was debugging this particular test 😅

zackkrida · 2022-03-14T14:29:23Z

src/utils/env.ts

 /**
 * Default environment variables are set on this key. Defaults are fallbacks to existing env vars.
 * All boolean values should be designed to be false by default.
 */
 export const env = {
-  apiUrl: process.env.API_URL ?? 'https://api.openverse.engineering/',
+  apiUrl: apiUrl.endsWith('/') ? apiUrl : `${apiUrl}/`,


zackkrida · 2022-03-14T14:40:16Z

Getting Brotli related errors in my attempts to test this:

Full terminal output

front on  try/e2e-docker +/- [$] via  v16.13.0
❯ pnpm test:e2e

> [email protected] pretest:e2e /Users/zackkrida/Code/openverse/front
> pnpm install

Lockfile is up-to-date, resolution step is skipped
Already up-to-date

> [email protected] prepare /Users/zackkrida/Code/openverse/front
> husky install

husky - Git hooks installed

> [email protected] test:e2e /Users/zackkrida/Code/openverse/front
> cross-env PLAYWRIGHT_VERSION=1.17.1 TEST_TYPE=e2e docker-compose -f docker-compose.playwright.yml up --abort-on-container-exit --exit-code-from playwright

WARN[0000] Found orphan containers ([front_web_1]) for this project. If you removed or renamed this service in your compose file, you can run this command with the --remove-orphans flag to clean it up.
[+] Running 1/0
 ⠿ Container front-playwright-1  Created                                                                                                                                                                                                                                                                                 0.0s
Attaching to front-playwright-1
front-playwright-1  |
front-playwright-1  | > [email protected] test:e2e:local /app
front-playwright-1  | > playwright test -c test/e2e
front-playwright-1  |
front-playwright-1  | [WebServer] 2022-03-14T14:37:43.272Z [Openverse e2e proxy] [ERROR] Error reading tape test/tapes/response-106.json5 Unsupported content-encoding br
front-playwright-1  | [WebServer] 2022-03-14T14:37:43.280Z [Openverse e2e proxy] [ERROR] Error reading tape test/tapes/response-107.json5 Unsupported content-encoding br
front-playwright-1  | [WebServer] 2022-03-14T14:37:43.397Z [Openverse e2e proxy] [ERROR] Error reading tape test/tapes/response-114.json5 Unsupported content-encoding br
front-playwright-1  | [WebServer] 2022-03-14T14:37:43.471Z [Openverse e2e proxy] [ERROR] Error reading tape test/tapes/response-117.json5 Unsupported content-encoding br
front-playwright-1  | [WebServer] 2022-03-14T14:37:43.595Z [Openverse e2e proxy] [ERROR] Error reading tape test/tapes/response-121.json5 Unsupported content-encoding br
front-playwright-1  | [WebServer] 2022-03-14T14:37:43.708Z [Openverse e2e proxy] [ERROR] Error reading tape test/tapes/response-125.json5 Unsupported content-encoding br
front-playwright-1  | [WebServer] 2022-03-14T14:37:43.962Z [Openverse e2e proxy] [ERROR] Error reading tape test/tapes/response-13.json5 Unsupported content-encoding br
front-playwright-1  | [WebServer] 2022-03-14T14:37:44.673Z [Openverse e2e proxy] [ERROR] Error reading tape test/tapes/response-15.json5 Unsupported content-encoding br
front-playwright-1  | [WebServer] 2022-03-14T14:37:45.234Z [Openverse e2e proxy] [ERROR] Error reading tape test/tapes/response-175.json5 Unsupported content-encoding br
front-playwright-1  | [WebServer] 2022-03-14T14:37:45.253Z [Openverse e2e proxy] [ERROR] Error reading tape test/tapes/response-179.json5 Unsupported content-encoding br
front-playwright-1  | [WebServer] 2022-03-14T14:37:45.297Z [Openverse e2e proxy] [ERROR] Error reading tape test/tapes/response-180.json5 Unsupported content-encoding br
front-playwright-1  | [WebServer] 2022-03-14T14:37:45.311Z [Openverse e2e proxy] [ERROR] Error reading tape test/tapes/response-181.json5 Unsupported content-encoding br
front-playwright-1  | [WebServer] 2022-03-14T14:37:45.413Z [Openverse e2e proxy] [ERROR] Error reading tape test/tapes/response-185.json5 Unsupported content-encoding br
front-playwright-1  | [WebServer] 2022-03-14T14:37:45.428Z [Openverse e2e proxy] [ERROR] Error reading tape test/tapes/response-186.json5 Unsupported content-encoding br
front-playwright-1  | [WebServer] 2022-03-14T14:37:45.439Z [Openverse e2e proxy] [ERROR] Error reading tape test/tapes/response-187.json5 Unsupported content-encoding br
front-playwright-1  | [WebServer] 2022-03-14T14:37:45.623Z [Openverse e2e proxy] [ERROR] Error reading tape test/tapes/response-190.json5 Unsupported content-encoding br
front-playwright-1  | [WebServer] 2022-03-14T14:37:45.723Z [Openverse e2e proxy] [ERROR] Error reading tape test/tapes/response-21.json5 Unsupported content-encoding br
front-playwright-1  | [WebServer] 2022-03-14T14:37:45.759Z [Openverse e2e proxy] [ERROR] Error reading tape test/tapes/response-24.json5 Unsupported content-encoding br
front-playwright-1  | [WebServer] 2022-03-14T14:37:46.021Z [Openverse e2e proxy] [ERROR] Error reading tape test/tapes/response-30.json5 Unsupported content-encoding br
front-playwright-1  | [WebServer] 2022-03-14T14:37:46.470Z [Openverse e2e proxy] [ERROR] Error reading tape test/tapes/response-42.json5 Unsupported content-encoding br
front-playwright-1  | [WebServer] 2022-03-14T14:37:47.053Z [Openverse e2e proxy] [ERROR] Error reading tape test/tapes/response-58.json5 Unsupported content-encoding br
front-playwright-1  | [WebServer] 2022-03-14T14:37:47.056Z [Openverse e2e proxy] [ERROR] Error reading tape test/tapes/response-59.json5 Unsupported content-encoding br
front-playwright-1  | [WebServer] 2022-03-14T14:37:47.074Z [Openverse e2e proxy] [ERROR] Error reading tape test/tapes/response-60.json5 Unsupported content-encoding br
front-playwright-1  | [WebServer] 2022-03-14T14:37:47.089Z [Openverse e2e proxy] [ERROR] Error reading tape test/tapes/response-61.json5 Unsupported content-encoding br
front-playwright-1  | [WebServer] 2022-03-14T14:37:47.340Z [Openverse e2e proxy] [ERROR] Error reading tape test/tapes/response-68.json5 Unsupported content-encoding br
front-playwright-1  | [WebServer] 2022-03-14T14:37:47.370Z [Openverse e2e proxy] [ERROR] Error reading tape test/tapes/response-70.json5 Unsupported content-encoding br

sarayourfriend · 2022-03-14T15:06:09Z

@zackkrida that's a talkback error: https://github.com/sarayourfriend/talkback/blob/7226f9c24ad78957575750cc39a3b17787270602/src/utils/content-encoding.ts#L39

Just need to update to the most recent version of talkback since my PR to add Brotli support was merged upstream: https://github.com/ijpiantanida/talkback/releases/tag/v3.0.1

sarayourfriend · 2022-03-14T16:07:56Z

@zackkrida Ready for re-review.

sarayourfriend · 2022-03-14T17:06:37Z

@zackkrida I'm going to push another commit to solve an issue with duplicate tapes that @obulat pointed out to me. Should be pushing it within the next hour or so.

obulat · 2022-03-16T17:34:11Z

Running the tests with pnpm test:e2e works well, but only if I increase the timeout in playwright.config.ts. Updating tapes works well, too. But running in debug mode causes the container to fail. This is what I see in the Docker log:

undefined
 ERR_PNPM_RECURSIVE_EXEC_FIRST_FAIL  not found: test::local

sarayourfriend · 2022-03-16T19:07:29Z

@obulat What command did you use to run the debug? pnpm test:e2e:debug? It looks like somehow the tests were run without a TEST_TYPE for one of the docker-based approaches... 🤔

sarayourfriend · 2022-03-16T19:08:02Z

but only if I increase the timeout in playwright.config.ts

I'll increase the timeout to five minutes I think.

sarayourfriend · 2022-03-17T13:49:59Z

@obulat I tried running pnpm test:e2e:debug locally and it worked fine for me 😕 Unfortunately we can't reuse the docker approach for debugging tests because docker would need to connect to a running Xorg server to be able to display the headed browsers.... and configuring that just for one OS is hard enough, making it cross platform to macOS, Linux, and Windows (even with WSL) would require a lot of work. Maybe one day we can tackle that but for now test:e2e:debug's approach is the only way 😞

obulat

Well, debugging works locally, and e2e tests run well in docker. This is such a great improvement, and it's great that you split this part of the change into a separate smaller PR:)

sarayourfriend · 2022-03-17T16:31:26Z

@zackkrida can you re-review today if you have time? I'd like to merge this to avoid any more merge conflicts. Thanks!

sarayourfriend · 2022-03-18T01:17:45Z

@krysal Would you be able to review this when you have a chance?

zackkrida

LGTM. This is working flawlessly for me now. My only nit is that after running pnpm test:e2e, this is the only output for 2-3 minutes:

Starting front_playwright_1 ... done
Attaching to front_playwright_1
playwright_1  |
playwright_1  | > [email protected] test:e2e:local /app
playwright_1  | > playwright test -c test/e2e
playwright_1  |
playwright_1  |

If we could log a message or something right away that indicates that a bunch of background work is happening, that might reassure users while this is loading. I worry about folks with slower machines than my m1 macbook pro. I left an inline comment with a suggestion on how to do this.

zackkrida · 2022-03-18T02:08:51Z

docker-compose.playwright.yml

+      - .:/app
+    user: ${PW_USER:-pwuser}
+    working_dir: /app
+    command: pnpm test:${TEST_TYPE}:local


Could do something here to log a message like so:

command: > sh -c "echo Some type of message here pnpm test:${TEST_TYPE}:local"

sarayourfriend added 4 commits March 4, 2022 08:32

Add talkback dockerfile

5550f4d

Get playwright tests running inside docker-compose

70f8d4f

Correctly wire-up the frontend to use the talkback proxy

ffb2515

Allow updating tapes with fixed permissions

1aa75c3

sarayourfriend requested a review from a team as a code owner March 4, 2022 17:38

sarayourfriend requested review from krysal and obulat March 4, 2022 17:38

sarayourfriend added 3 commits March 4, 2022 12:43

Fix bad package.json changes

f07a35d

Remove unnecessary node-env setup for dockerized e2e tests

fb68359

@todo(sarayourfriend): Find a way to de-duplicate the docker-compose invocation. Perhaps a separate bash script in `bin`?

Remove one place that playwright version is duplicated

76f09bf

sarayourfriend force-pushed the try/e2e-docker branch from 0c18ba8 to 76f09bf Compare March 4, 2022 18:17

sarayourfriend added 3 commits March 4, 2022 13:18

Deduplicate pnpm version specification

3df4726

Fix bad interpreter

f93c2db

Improve comment clarity

89c1a76

sarayourfriend added 8 commits March 4, 2022 14:10

Replace upstream API references with proxy in recorded tapes

d7bfb0f

Remove unnecessary await

b16e674

Update scripts that run inside playwright to use plain npm run

4da0758

Handle the occasional brotli compressed response

cdb9354

Run app and proxy inside the playwright container

ac8b217

Remove unused script

4e18ad2

Update documentation to reflect dockerization

11d0631

Remove errant dc dependency and install deps

7daf4b0

sarayourfriend commented Mar 5, 2022

View reviewed changes

zackkrida requested review from zackkrida and removed request for zackkrida March 11, 2022 14:12

Update tapes

cb6eddb

sarayourfriend changed the title ~~Run e2e tests inside a docker-compose stack~~ Run e2e tests inside a docker container Mar 14, 2022

zackkrida reviewed Mar 14, 2022

View reviewed changes

Fix bad main merge

7ffc52d

sarayourfriend added 3 commits March 14, 2022 13:13

Fix duplicate tape issue and type check proxy.js

66cffee

Run prettier on md files as well

139f4c7

Fix formatting

3f0d35e

sarayourfriend added 2 commits March 16, 2022 15:42

Increase timeout to five minutes

b0ab0f1

Merge main branch into try/e2e-docker

99d5e35

obulat approved these changes Mar 17, 2022

View reviewed changes

zackkrida approved these changes Mar 18, 2022

View reviewed changes

zackkrida reviewed Mar 18, 2022

View reviewed changes

Add message to notify about build wait

7a9a526

sarayourfriend merged commit ba9a8d9 into main Mar 18, 2022

sarayourfriend deleted the try/e2e-docker branch March 18, 2022 11:34

sarayourfriend mentioned this pull request Mar 18, 2022

Configure local visual regression testing #1017

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run e2e tests inside a docker container #1057

Run e2e tests inside a docker container #1057

sarayourfriend commented Mar 4, 2022 •

edited

Loading

sarayourfriend commented Mar 4, 2022 •

edited

Loading

sarayourfriend Mar 5, 2022

zackkrida Mar 14, 2022

zackkrida commented Mar 14, 2022

sarayourfriend commented Mar 14, 2022

sarayourfriend commented Mar 14, 2022

sarayourfriend commented Mar 14, 2022

obulat commented Mar 16, 2022

sarayourfriend commented Mar 16, 2022

sarayourfriend commented Mar 16, 2022

sarayourfriend commented Mar 17, 2022

obulat left a comment

sarayourfriend commented Mar 17, 2022

sarayourfriend commented Mar 18, 2022

zackkrida left a comment •

edited

Loading

zackkrida Mar 18, 2022

Run e2e tests inside a docker container #1057

Run e2e tests inside a docker container #1057

Conversation

sarayourfriend commented Mar 4, 2022 • edited Loading

Fixes

Description

Note about the line count for this PR!

Rationale

Solution

Meta commentary about the PR

Related fixes

Testing Instructions

Checklist

Developer Certificate of Origin

sarayourfriend commented Mar 4, 2022 • edited Loading

sarayourfriend Mar 5, 2022

Choose a reason for hiding this comment

zackkrida Mar 14, 2022

Choose a reason for hiding this comment

zackkrida commented Mar 14, 2022

sarayourfriend commented Mar 14, 2022

sarayourfriend commented Mar 14, 2022

sarayourfriend commented Mar 14, 2022

obulat commented Mar 16, 2022

sarayourfriend commented Mar 16, 2022

sarayourfriend commented Mar 16, 2022

sarayourfriend commented Mar 17, 2022

obulat left a comment

Choose a reason for hiding this comment

sarayourfriend commented Mar 17, 2022

sarayourfriend commented Mar 18, 2022

zackkrida left a comment • edited Loading

Choose a reason for hiding this comment

zackkrida Mar 18, 2022

Choose a reason for hiding this comment

sarayourfriend commented Mar 4, 2022 •

edited

Loading

sarayourfriend commented Mar 4, 2022 •

edited

Loading

zackkrida left a comment •

edited

Loading