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.

Sign up for GitHub

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

Jump to bottom

feat: add E2E tests for chromium and firefox #1121

Merged
merged 11 commits into from
Jan 17, 2023
Merged

feat: add E2E tests for chromium and firefox #1121

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
bf8646e
feat: add E2E tests for chromium and firefox
galargh Dec 19, 2022
00d14ea
fix: do not use arrow functions in mocha
galargh Dec 20, 2022
5a933cd
feat: create a workflow for running e2e tests
galargh Dec 20, 2022
6503c92
Merge branch 'main' into feat/test-e2e
galargh Dec 20, 2022
d8c6603
chore: apply review comments
galargh Dec 21, 2022
963e571
Merge branch 'main' into feat/test-e2e
SgtPooki Jan 13, 2023
416cb1a
chore: support override of firefox image
SgtPooki Jan 14, 2023
2807670
fix(test): await async destroy operation
SgtPooki Jan 14, 2023
ba30b87
chore: address review comments
galargh Jan 16, 2023
4bf55b6
chore: allow overriding access control settings through env
galargh Jan 17, 2023
25a4760
Update docs/DEVELOPER-NOTES.md
galargh Jan 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions .github/workflows/e2e.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
name: e2e
on:
workflow_dispatch:
inputs:
firefox-version:
description: The version of selenium/standalone-firefox image to use
default: latest
required: true
galargh marked this conversation as resolved.
Show resolved Hide resolved
chromium-version:
description: The version of selenium/standalone-chrome image to use
default: latest
required: true
SgtPooki marked this conversation as resolved.
Show resolved Hide resolved
kubo-version:
description: The version of ipfs/kubo image to use
default: latest
required: true
galargh marked this conversation as resolved.
Show resolved Hide resolved
ipfs-companion-version:
description: The version of ipfs-companion extension to use (defaults to building the extension from source)
default: ''
required: false

jobs:
test:
runs-on: ubuntu-latest
steps:
- name: Check out repo
uses: actions/checkout@v3
- name: Set up node
uses: actions/setup-node@v3
with:
node-version: 18
- name: Download ipfs-companion
if: inputs.ipfs-companion-version != ''
run: ./ci/download-release-artifacts.sh
env:
IPFS_COMPANION_VERSION: ${{ inputs.ipfs-companion-version }}
galargh marked this conversation as resolved.
Show resolved Hide resolved
- name: Build ipfs-companion
if: inputs.ipfs-companion-version == ''
run: npm run release-build
Copy link
Member

Choose a reason for hiding this comment

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

we should eventually upload the built docker image (ipfs-companion-release-build) and reuse that if possible.. (hash of source or non-docker build artifacts to determine if we want to use the same image?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did a write-up recently on docker publishing/caching in CI for build speed-up. See https://filecoinproject.slack.com/archives/C03KLC57LKB/p1673517809976679?thread_ts=1673468796.401379&cid=C03KLC57LKB

I'd say it's a little bit beyond the scope of this PR though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I created #1132 for this

- name: Prepare E2E env
run: npm run compose:e2e:prepare
env:
FIREFOX_VERSION: ${{ inputs.firefox-version }}
CHROMIUM_VERSION: ${{ inputs.chromium-version }}
KUBO_VERSION: ${{ inputs.kubo-version }}
- name: Start E2E env
run: npm run compose:e2e:up
env:
FIREFOX_VERSION: ${{ inputs.firefox-version }}
CHROMIUM_VERSION: ${{ inputs.chromium-version }}
KUBO_VERSION: ${{ inputs.kubo-version }}
- name: Wait for E2E env set up to complete
run: sleep 60
- name: Run E2E tests
run: npm run compose:e2e:test
env:
IPFS_COMPANION_VERSION: ${{ inputs.ipfs-companion-version }}
- name: Stop E2E env
run: npm run compose:e2e:down
9 changes: 6 additions & 3 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,22 @@ ARG GROUP_ID
RUN curl -s https://ipfs.io/ipfs/QmbukYcmtyU6ZEKt6fepnvrTNa9F6VqsUPMUgNxQjEmphH > /usr/local/bin/jq && chmod +x /usr/local/bin/jq

RUN mkdir -p /home/node/app
WORKDIR /home/node/app

RUN if [ ${USER_ID:-0} -ne 0 ] && [ ${GROUP_ID:-0} -ne 0 ]; then \
userdel -f node && \
if getent group node ; then groupdel node; fi && \
groupadd -g ${GROUP_ID} node && \
useradd -l -u ${USER_ID} -g node node && \
chown -fhR ${USER_ID}:${GROUP_ID} /home/node; fi
RUN chown node:node /home/node/app

COPY --chown=${USER_ID}:${GROUP_ID} . /home/node/app
WORKDIR /home/node/app

COPY --chown=node:node ./package.json ./package-lock.json /home/node/app/

USER node

RUN npm run ci:install

COPY --chown=node:node . /home/node/app

ENV PATH="/home/node/app/node_modules/.bin:${PATH}"
5 changes: 5 additions & 0 deletions ci/access-control-allow-all.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#!/bin/sh
set -ex

ipfs config --json API.HTTPHeaders.Access-Control-Allow-Origin '["*"]'
ipfs config --json API.HTTPHeaders.Access-Control-Allow-Methods '["*"]'
Copy link
Contributor

Choose a reason for hiding this comment

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

[opt] Question: won't this be detrimental to the e2e spirit of testing companion? If we're configuring the API to work for all origins, we might not catch a few issues.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Anything in particular we're concerned with? The reason why this is needed is because in the "everything in docker" setup, the browsers run in different containers than kubo. There might be another way to do it. This was just the one that required the least thought.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, kubo has support for specific user agents, which includes ipfs-companion since ipfs/kubo#8690

This is not normal user behaviour, which means we can have cases where tests pass but the users don't see this behaviour. That'll defeat the purpose of e2e testing.

13 changes: 13 additions & 0 deletions ci/download-release-artifacts.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#!/bin/sh
set -ex

IPFS_COMPANION_VERSION=${IPFS_COMPANION_VERSION:-$(jq -r '.version' ./add-on/manifest.common.json)}

id="$(curl --retry 5 --no-progress-meter "https://api.github.com/repos/ipfs/ipfs-companion/releases/tags/v$IPFS_COMPANION_VERSION" | jq '.id')"
assets="$(curl --retry 5 --no-progress-meter --location "https://api.github.com/repos/ipfs/ipfs-companion/releases/$id/assets" | jq -r '.[].name')"

mkdir build

for asset in $assets; do
curl --retry 5 --no-progress-meter --location --output "build/$asset" "https://github.com/ipfs/ipfs-companion/releases/download/v$IPFS_COMPANION_VERSION/$asset"
done
galargh marked this conversation as resolved.
Show resolved Hide resolved
40 changes: 40 additions & 0 deletions docker-compose.e2e.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
version: "3.9"
Copy link
Member

Choose a reason for hiding this comment

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

Note that we should be able to set hostname and domain name for these containers, which would make addressing https://github.com/ipfs/ipfs-companion/pull/1121/files#r1053853150 much easier. We would know the hostname, protocol, and port

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure how exactly setting hostname and domainname works. Could you elaborate on how they could affect the setup?

Copy link
Member

Choose a reason for hiding this comment

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

Are you asking about how hostname and domainname are set inside the container? I'm not fully aware of the implementation details but I believe the etc/hostname file (and docker configs) are updated appropriately.

Ultimately, setting domainname and hostname would allow you to address the CORS concerns brought up by @whizzzkid, because you would be able to specify the origins set via hostname/domainname and port.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I got confused a bit and though these were global settings for the compose file 🤦

Back to the matter, we're already "using" hostname. If the value is not specified, then the hostname defaults to the service name. We already use this fact to communicate between services. E.g. that's how browsers know they can find Kubo at http://kubo.

Domainname, I'm not so familiar with, but apparently it's not that useful - https://stackoverflow.com/questions/70741780/what-is-the-domainname-option-used-for-in-docker

Anyway, I tried to be more explicit about access control and limit the origins to http://chromium and http://firefox but, apparently, that's not enough.

You can now set access control values through env - https://github.com/ipfs/ipfs-companion/blob/25a4760138ac21fdfdf198a6476a88b92bd09c4c/ci/access-control-allow-all.sh. This should allow for easier experimentation in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I created an issue for this at #1130

services:
firefox:
image: selenium/standalone-firefox:${FIREFOX_VERSION:-latest}
galargh marked this conversation as resolved.
Show resolved Hide resolved
shm_size: 2g
ports:
- 4444
- 7900
chromium:
# WARN: `standalone-chrome` does NOT work on ARM-based machines;
# see https://github.com/SeleniumHQ/docker-selenium#experimental-mult-arch-aarch64armhfamd64-images;
# try using `seleniarm/standalone-chromium` instead
Comment on lines +10 to +12
Copy link
Member

Choose a reason for hiding this comment

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

+1. thanks for this

# export CHROMIUM_IMAGE=seleniarm/standalone-chromium
image: ${CHROMIUM_IMAGE:-selenium/standalone-chrome}:${CHROMIUM_VERSION:-latest}
shm_size: 2g
ports:
- 4444
- 7900
kubo:
image: ipfs/kubo:${KUBO_VERSION:-latest}
ports:
- 4001
- 5001
- 8080
volumes:
- ./ci/access-control-allow-all.sh:/container-init.d/001-access-control-allow-all.sh
e2e:
build:
dockerfile: ./Dockerfile
environment:
- SELENIUM_REMOTE_CHROMIUM_URL=http://chromium:4444
- SELENIUM_REMOTE_FIREFOX_URL=http://firefox:4444
- IPFS_API_URL=http://kubo:5001
- CUSTOM_GATEWAY_URL=http://kubo:8080
- TEST_E2E=true
- TEST_HEADLESS=${TEST_HEADLESS}
- TEST_DEBUG=${TEST_DEBUG}
- IPFS_COMPANION_VERSION=${IPFS_COMPANION_VERSION}
volumes:
- ./build:/home/node/app/build
56 changes: 56 additions & 0 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 9 additions & 3 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
"watch:js": "run-p watch:js:*",
"watch:js:webpack": "webpack --watch --mode development --devtool inline-source-map --config ./webpack.config.js",
"test": "run-s test:*",
"test:e2e": "mocha --timeout 300000 --exit --require ignore-styles \"test/e2e/**/*.test.js\"",
"test:functional": "c8 mocha --timeout 5000 --exit --require ignore-styles \"test/functional/**/*.test.js\"",
"lint": "run-s lint:*",
"lint:standard": "standard -v \"*.js\" \"add-on/src/**/*.js\" \"test/**/*.js\" \"scripts/**/*.js\"",
Expand All @@ -57,10 +58,14 @@
"ci:build": "npm run build",
"ci:test": "npm test",
"ci:lint": "npm run lint",
"beta-build": "docker rmi -f ipfs-companion-beta-build && docker build -t ipfs-companion-beta-build --build-arg USER_ID=$(id -u ${USER}) --build-arg GROUP_ID=$(id -g ${USER}) . && mkdir -p build && docker run --rm -it --net=host -e RELEASE_CHANNEL=beta -v $(pwd)/build:/home/node/app/build ipfs-companion-beta-build npm run ci:build",
"release-build": "docker rmi -f ipfs-companion-release-build && docker build -t ipfs-companion-release-build --build-arg USER_ID=$(id -u ${USER}) --build-arg GROUP_ID=$(id -g ${USER}) . && mkdir -p build && docker run --rm -it --net=host -e RELEASE_CHANNEL=stable -v $(pwd)/build:/home/node/app/build ipfs-companion-release-build npm run ci:build",
"beta-build": "docker rmi -f ipfs-companion-beta-build && docker build -t ipfs-companion-beta-build --build-arg USER_ID=$(id -u ${USER}) --build-arg GROUP_ID=$(id -g ${USER}) . && mkdir -p build && docker run --rm --net=host -e RELEASE_CHANNEL=beta -v $(pwd)/build:/home/node/app/build ipfs-companion-beta-build npm run ci:build",
"release-build": "docker rmi -f ipfs-companion-release-build && docker build -t ipfs-companion-release-build --build-arg USER_ID=$(id -u ${USER}) --build-arg GROUP_ID=$(id -g ${USER}) . && mkdir -p build && docker run --rm --net=host -e RELEASE_CHANNEL=stable -v $(pwd)/build:/home/node/app/build ipfs-companion-release-build npm run ci:build",
galargh marked this conversation as resolved.
Show resolved Hide resolved
"dev-build": "npm ci && npm run build",
"yarn-build": "npm run dev-build"
"yarn-build": "npm run dev-build",
galargh marked this conversation as resolved.
Show resolved Hide resolved
"compose:e2e:prepare": "docker compose --file docker-compose.e2e.yml pull && docker compose --file docker-compose.e2e.yml build",
galargh marked this conversation as resolved.
Show resolved Hide resolved
"compose:e2e:up": "docker compose --file docker-compose.e2e.yml up --remove-orphans --detach kubo chromium firefox",
"compose:e2e:test": "docker compose --file docker-compose.e2e.yml run e2e npm run test:e2e",
"compose:e2e:down": "docker compose --file docker-compose.e2e.yml down"
},
"private": true,
"preferGlobal": false,
Expand Down Expand Up @@ -103,6 +108,7 @@
"path": "0.12.7",
"raw-loader": "4.0.2",
"request-progress": "3.0.0",
"selenium-webdriver": "^4.7.1",
"shx": "0.3.4",
"sinon": "13.0.1",
"sinon-chrome": "3.0.1",
Expand Down
Loading