feat: add E2E tests for chromium and firefox #1121

galargh · 2022-12-19T16:25:17Z

Description

This PR adds a setup for E2E testing to ipfs-companion.

The environment setup is automated using docker compose. The environment consists of:

firefox container
chrome container
kubo container
e2e container where the tests are executed from

The tests check the following for firefox and chromium:

can the extension be installed?
can api/gateway URL be modified?
is the number of connected peers correctly reported?
is the number of connected peers greater than 0?

The tests follow the proposed testing procedure from https://github.com/ipfs/kubo/blob/master/docs/RELEASE_ISSUE_TEMPLATE.md.

The newly added tests can be executed through GitHub Actions.

Testing

build companion from source: https://github.com/galargh/ipfs-companion/actions/runs/3739328246
download companion from releases: https://github.com/galargh/ipfs-companion/actions/runs/3739280013

TODO

create GHA workflow for running E2E tests; proposed inputs
- firefox/chromium versions
- ipfs-companion version; proposed behaviours
  - build from source
  - download from releases
- kubo version
add description

welcome · 2022-12-19T16:25:19Z

Thank you for submitting this PR!
A maintainer will be here shortly to review it.
We are super grateful, but we are also overloaded! Help us by making sure that:

The context for this PR is clear, with relevant discussion, decisions
and stakeholders linked/mentioned.
Your contribution itself is clear (code comments, self-review for the
rest) and in its best form. Follow the code contribution
guidelines
if they apply.

Getting other community members to do a review would be great help too on complex PRs (you can ask in the chats/forums). If you are unsure about something, just leave us a comment.
Next steps:

A maintainer will triage and assign priority to this PR, commenting on
any missing things and potentially assigning a reviewer for high
priority items.
The PR gets reviews, discussed and approvals as needed.
The PR is merged by maintainers when it has been approved and comments addressed.

We currently aim to provide initial feedback/triaging within seven business days. Please keep an eye on any labelling actions, as these will indicate priorities and status of your contribution.
We are very grateful for your contribution!

whizzzkid

Thanks for creating this PR @galargh, excellent work! I have some concerns regarding the use of selenium vs playwright and some ideas for improvements (marked as [opt]) those can be looked into later if you'd like!

Cheers :)

whizzzkid · 2022-12-20T23:48:02Z

ci/access-control-allow-all.sh

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


[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.

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.

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.

ci/download-release-artifacts.sh

docker-compose.e2e.yml

package.json

test/e2e/ipfs-companion.test.js

SgtPooki · 2022-12-21T18:16:48Z

I would also like to voice my preference of playwright over selenium.

We're using playwright with all of our projects
Selenium is historically unstable during runtime and maintenance. I've got experience with building & maintaining an e2e library backed by selenium, and I am not a fan.

whizzzkid

Thanks! I would like @lidel to weigh in on selenium vs playwright, everything else LGTM! 🚀

whizzzkid · 2022-12-21T22:30:23Z

ci/access-control-allow-all.sh

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


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.

ci/download-release-artifacts.sh

docker-compose.e2e.yml

package.json

test/e2e/ipfs-companion.test.js

galargh · 2022-12-22T13:04:54Z

I would also like to voice my preference of playwright over selenium.

We're using playwright with all of our projects

Selenium is historically unstable during runtime and maintenance. I've got experience with building & maintaining an e2e library backed by selenium, and I am not a fan.

Personally, I don't know enough about either to prefer one over the other BUT it seems we cannot accomplish the task at all in playwright at the moment because of lack of support for Firefox extensions (microsoft/playwright#7297).

SgtPooki · 2023-01-11T01:43:01Z

it seems we cannot accomplish the task at all in playwright

ACK. makes complete sense.

SgtPooki

Approving with comments.. thanks a ton for putting this together @galargh

.github/workflows/e2e.yml

ci/download-release-artifacts.sh

test/e2e/ipfs-companion.test.js

SgtPooki · 2023-01-13T23:50:37Z

test/e2e/ipfs-companion.test.js

+
+    const peers = await getNumberOfConnectedPeers(browser, url)
+
+    expect(peers).not.to.equal(0)


I feel like this will be flaky unless we have a guaranteed peer running in CI as well.

We can add one if that becomes a problem.

I created #1131 for this

test/e2e/ipfs-companion.test.js

SgtPooki · 2023-01-14T00:14:50Z

@galargh , the only thing I would really like to see before merge is an update to README.md or docs/DEVELOPER-NOTES.md informing users how to run this locally, how to debug, etc.

SgtPooki

When attempting to run locally, I ran into some issues:

npm run clean
npm i
npm run release-build # Error when running this

Error output

 => [3/9] RUN mkdir -p /home/node/app                                                                                                         0.2s
 => ERROR [4/9] RUN if [ 501 -ne 0 ] && [ 20 -ne 0 ]; then     userdel -f node &&     if getent group node ; then groupdel node; fi &&     g  0.3s
------
 > [4/9] RUN if [ 501 -ne 0 ] && [ 20 -ne 0 ]; then     userdel -f node &&     if getent group node ; then groupdel node; fi &&     groupadd -g 20 node &&     useradd -l -u 501 -g node node &&     chown -fhR 501:20 /home/node; fi:
#8 0.255 groupadd: GID '20' already exists
------
executor failed running [/bin/sh -c 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]: exit code: 4

commenting out all the user modifications allows it to work. What is that code supposed to be doing? can we make it smarter?

SgtPooki · 2023-01-14T00:27:19Z

.github/workflows/e2e.yml

+          IPFS_COMPANION_VERSION: ${{ inputs.ipfs-companion-version }}
+      - name: Build ipfs-companion
+        if: inputs.ipfs-companion-version == ''
+        run: npm run release-build


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?)

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.

I created #1132 for this

- replace fixed delays with exponential backoff - make download-release-artifacts.sh work even if build dir already exists - document how to run e2e tests locally - delete user/group if it already exists during release build in docker - make it easier to add new e2e test cases in the future

galargh

@galargh , the only thing I would really like to see before merge is an update to README.md or docs/DEVELOPER-NOTES.md informing users how to run this locally, how to debug, etc.

Docs added to docs/DEVELOPER-NOTES.md.

When attempting to run locally, I ran into some issues.

I added deleting user/group if it already exists to the Dockerfile which should fix the issue. I suspect it's there to maintain correct ownership of built files after we get them out of the docker container.

.github/workflows/e2e.yml

galargh · 2023-01-16T13:13:47Z

.github/workflows/e2e.yml

+          IPFS_COMPANION_VERSION: ${{ inputs.ipfs-companion-version }}
+      - name: Build ipfs-companion
+        if: inputs.ipfs-companion-version == ''
+        run: npm run release-build


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.

galargh · 2023-01-16T13:19:17Z

docker-compose.e2e.yml

@@ -0,0 +1,40 @@
+version: "3.9"


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

galargh · 2023-01-16T14:06:25Z

test/e2e/ipfs-companion.test.js

+
+    const peers = await getNumberOfConnectedPeers(browser, url)
+
+    expect(peers).not.to.equal(0)


We can add one if that becomes a problem.

SgtPooki

lgtm.. a few comments unaddressed, but a significant improvement. Thanks @galargh !

docs/DEVELOPER-NOTES.md

test/e2e/ipfs-companion.test.js

Co-authored-by: Russell Dempsey <[email protected]>

galargh force-pushed the feat/test-e2e branch from 08b2bd5 to 8418e31 Compare December 19, 2022 16:25

feat: add E2E tests for chromium and firefox

bf8646e

galargh force-pushed the feat/test-e2e branch from 8418e31 to bf8646e Compare December 19, 2022 16:37

galargh changed the title ~~feat: add E2E tests for chromium and firefox~~ [WIP] feat: add E2E tests for chromium and firefox Dec 19, 2022

galargh added 2 commits December 20, 2022 09:36

fix: do not use arrow functions in mocha

00d14ea

feat: create a workflow for running e2e tests

5a933cd

galargh force-pushed the feat/test-e2e branch from abef7c7 to 5a933cd Compare December 20, 2022 09:59

Merge branch 'main' into feat/test-e2e

6503c92

galargh changed the title ~~[WIP] feat: add E2E tests for chromium and firefox~~ feat: add E2E tests for chromium and firefox Dec 20, 2022

galargh marked this pull request as ready for review December 20, 2022 10:09

galargh requested review from lidel, meandavejustice and a team as code owners December 20, 2022 10:09

galargh requested a review from whizzzkid December 20, 2022 10:09

whizzzkid reviewed Dec 21, 2022

View reviewed changes

chore: apply review comments

d8c6603

whizzzkid approved these changes Dec 21, 2022

View reviewed changes

SgtPooki approved these changes Jan 13, 2023

View reviewed changes

Merge branch 'main' into feat/test-e2e

963e571

SgtPooki self-requested a review January 14, 2023 00:18

SgtPooki requested changes Jan 14, 2023

View reviewed changes

SgtPooki reviewed Jan 14, 2023

View reviewed changes

SgtPooki and others added 3 commits January 13, 2023 16:44

chore: support override of firefox image

416cb1a

fix(test): await async destroy operation

2807670

galargh commented Jan 16, 2023

View reviewed changes

galargh requested a review from SgtPooki January 16, 2023 15:33

SgtPooki approved these changes Jan 16, 2023

View reviewed changes

docs/DEVELOPER-NOTES.md Show resolved Hide resolved

test/e2e/ipfs-companion.test.js Show resolved Hide resolved

test/e2e/ipfs-companion.test.js Show resolved Hide resolved

galargh and others added 2 commits January 17, 2023 10:10

chore: allow overriding access control settings through env

4bf55b6

Update docs/DEVELOPER-NOTES.md

25a4760

Co-authored-by: Russell Dempsey <[email protected]>

This was referenced Jan 17, 2023

e2e: use explicit values for Access-Control-Allow-Origin #1130

Open

e2e: consider adding a second kubo node and run the test offline #1131

Open

e2e: speed up docker operations through publishing/caching #1132

Open

galargh merged commit eeaa083 into main Jan 17, 2023

galargh deleted the feat/test-e2e branch January 17, 2023 09:31

This was referenced Feb 3, 2023

chore(main): release 2.21.0 #1144

Closed

chore(main): release 2.21.0 #1146

Closed

chore(main): release 2.21.0 #1148

Closed

github-actions bot mentioned this pull request Feb 16, 2023

chore(main): release ipfs-companion 2.23.0 #1159

Closed

github-actions bot mentioned this pull request Sep 21, 2023

chore(main): release 3.0.1 #1288

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add E2E tests for chromium and firefox #1121

feat: add E2E tests for chromium and firefox #1121

galargh commented Dec 19, 2022 •

edited

Loading

welcome bot commented Dec 19, 2022

whizzzkid left a comment

whizzzkid Dec 20, 2022

galargh Dec 21, 2022

whizzzkid Dec 21, 2022

SgtPooki commented Dec 21, 2022

whizzzkid left a comment

whizzzkid Dec 21, 2022

galargh commented Dec 22, 2022

SgtPooki commented Jan 11, 2023

SgtPooki left a comment

SgtPooki Jan 13, 2023

galargh Jan 16, 2023

galargh Jan 17, 2023 •

edited

Loading

SgtPooki commented Jan 14, 2023

SgtPooki left a comment •

edited

Loading

SgtPooki Jan 14, 2023

galargh Jan 16, 2023

galargh Jan 17, 2023

galargh left a comment

galargh Jan 16, 2023

galargh Jan 16, 2023

galargh Jan 16, 2023

SgtPooki left a comment

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


		const peers = await getNumberOfConnectedPeers(browser, url)

		expect(peers).not.to.equal(0)

feat: add E2E tests for chromium and firefox #1121

feat: add E2E tests for chromium and firefox #1121

Conversation

galargh commented Dec 19, 2022 • edited Loading

Description

Testing

TODO

welcome bot commented Dec 19, 2022

whizzzkid left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SgtPooki commented Dec 21, 2022

whizzzkid left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

galargh commented Dec 22, 2022

SgtPooki commented Jan 11, 2023

SgtPooki left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

galargh Jan 17, 2023 • edited Loading

Choose a reason for hiding this comment

SgtPooki commented Jan 14, 2023

SgtPooki left a comment • edited Loading

Choose a reason for hiding this comment

Error output

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

galargh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SgtPooki left a comment

Choose a reason for hiding this comment

galargh commented Dec 19, 2022 •

edited

Loading

galargh Jan 17, 2023 •

edited

Loading

SgtPooki left a comment •

edited

Loading