Fix e2e tests #559
Fix e2e tests #559
Conversation
Kabir-Ivan
force-pushed
the
fix-mac-docker
branch
2 times, most recently
from
05496d0
to
f13d32f
Compare
There was a problem hiding this comment.
A few changes are needed. What I expect from working with Docker images in this repo:
- I could pull
yinfra/html-reporter-browsersfrom Docker hub and everything will work fine - I can go to CONTRIBUTING.md and instead of pulling an image from Docker hub follow instructions step-by-step to build an image identical to
yinfra/html-reporter-browsersand then launch this image locally and run tests using this locally-built image - I can go to CONTRIBUTING.md and follow step-by-step instruction to publish a new version of
yinfra/...image.
| @@ -20,9 +20,9 @@ you need to launch browsers inside a Docker container. | |||
| - On Windows, you may use Windows Subsystem for Linux to run the Docker CLI without the Desktop application. | |||
There was a problem hiding this comment.
Since you're already editing this file, can you please fix this "How to?" section? In the markdown preview (three dots -> view file) we can see that it's broken. I suppose we need line breaks before/after <summary> tags
| - `colima start --profile amd --arch amd` | ||
| - `colima start --profile arm --arch arm` | ||
| 2. Create a buildx context to use the created instances as nodes | ||
| - `docker buildx create --use --name custom colima-amd` |
There was a problem hiding this comment.
We need to include buildx installation instructions here (at least a link to where one can find it)
CONTRIBUTING.md
Outdated
| @@ -38,3 +38,25 @@ If you want a finer-grained control over the process, the following commands may | |||
| - `npm run e2e:generate-fixtures` — generate fixture reports to run tests on | |||
| - `npm run --workspace=test/func/tests gui:plugins` — launch hermione GUI for the `plugins` tests set | |||
| - `npm run e2e:test` — run e2e tests only, without building packages or generating fixtures | |||
|
|
|||
| If you want to build an image with browsers or launch a local image, you can use these commands: | |||
There was a problem hiding this comment.
I'd make the following structure:
### Working with browser docker images
...
#### Building an image on Mac (Apple Silicon)
...
#### Publishing
...
package.json
Outdated
| @@ -20,7 +20,8 @@ | |||
| "e2e:build-browsers": "docker build -f test/func/docker/Dockerfile -t html-reporter-browsers:0.0.1 --network host test/func/docker", | |||
There was a problem hiding this comment.
As far as I understand, this command is now obsolete. I think we can update it to use buildx and use it in contributing.md file
CONTRIBUTING.md
Outdated
| - `docker buildx create --use --name custom colima-amd` | ||
| - `docker buildx create --append --name custom colima-arm` | ||
| 3. Build the image | ||
| - `docker buildx build --platform linux/amd64,linux/arm64 test/func/docker` |
There was a problem hiding this comment.
This command does nothing. It builds images, but I can't see it in docker images and can't use it.
See this discussion: docker/buildx#166 (comment)
Ideally, the instructions should clearly explain how to build the image for both arm and amd architectures and then how to launch each of them locally for testing
package.json
Outdated
| @@ -20,7 +20,8 @@ | |||
| "e2e:build-browsers": "docker build -f test/func/docker/Dockerfile -t html-reporter-browsers:0.0.1 --network host test/func/docker", | |||
| "e2e:build-packages": "npm run --workspace=test/func/packages --if-present build", | |||
| "e2e:generate-fixtures": "npm run --workspace=test/func/fixtures generate", | |||
| "e2e:launch-browsers": "docker run -it --rm --network=host --add-host=host.docker.internal:0.0.0.0 html-reporter-browsers:0.0.1", | |||
| "e2e:launch-browsers-local": "if which colima; then docker run -it --rm --network=host html-reporter-browsers:0.0.1; else docker run -it --rm --network=host --add-host=host.docker.internal:0.0.0.0 html-reporter-browsers:0.0.1; fi", | |||
| "e2e:launch-browsers": "if which colima; then docker run -it --rm --network=host yinfra/html-reporter-browsers; else docker run -it --rm --network=host --add-host=host.docker.internal:0.0.0.0 yinfra/html-reporter-browsers; fi", | |||
There was a problem hiding this comment.
What's the difference between these two scripts? Only in the yinfra prefix? I'd keep a single script and always use yinfra/... image as I don't see any downsides of this approach.
Also, this may be simplified to something along the lines of:
docker run -it --rm --network=host $(which colima >/dev/null || echo --add-host=host.docker.internal:0.0.0.0) yinfra/html-reporter-browsers:0.0.1
test/func/docker/Dockerfile
Outdated
| @@ -1,15 +1,26 @@ | |||
| FROM cimg/node:16.20-browsers | |||
| FROM node:18 | |||
There was a problem hiding this comment.
I see that the resulting images size is around 1GB. Have you tried making it smaller? One obvious thing that comes to mind is that -slim version of node:18 image weights only 60MB vs 400MB for the default one: https://hub.docker.com/_/node/tags?page=&page_size=&ordering=&name=18
test/func/docker/Dockerfile
Outdated
| if [ "$RELATIVE_PATH" != "" ]; then mv "$RELATIVE_PATH/chromedriver" "/chromedriver"; fi && \ | ||
| apt update -y && \ | ||
| apt install libnss3 -y && \ | ||
| npx playwright install chromium && \ |
There was a problem hiding this comment.
I'm not sure keeping playwright deps/browsers inside the image is a good idea. First, it makes the image much bigger. Secondly, when we'll update playwright in html-reporter, the image won't be updated and this will lead to pretty weird situation. Would it be better to install pwt deps at runtime in workflow, what do you think?
|
|
||
| ENTRYPOINT selenium-standalone start --drivers.chrome.version=$CHROME_VERSION | ||
| ENTRYPOINT ./chromedriver --port=4444 --whitelisted-ips="" |
There was a problem hiding this comment.
I remember we tried --whitelisted-ips option when testing. Is it actually required?
fix: change package.json
Kabir-Ivan
force-pushed
the
fix-mac-docker
branch
from
3ed72d1
to
c5a4a1c
Compare
No description provided.