Skip to content
This repository was archived by the owner on Jan 12, 2024. It is now read-only.

chore: fix CI workflows #527

Merged
merged 19 commits into from
Oct 14, 2022
Merged

Conversation

SgtPooki
Copy link
Collaborator

@SgtPooki SgtPooki commented Oct 13, 2022

testing to see if changes in #517 break CI workflows, or if they're already broken


They were broken. This PR fixes them and is blocking #517

testing to see if changes in ipfs-examples#517 break CI workflows, or if they're already broken
@SgtPooki
Copy link
Collaborator Author

Making significant progress with CI improvements, see https://github.com/SgtPooki/js-ipfs-examples/actions/runs/3245177404/jobs/5322363884 where all tests except monorepo, run-in-electron and browser-service-worker all succeeded

@SgtPooki
Copy link
Collaborator Author

@SgtPooki SgtPooki changed the title chore: tmp chore: fix CI workflows Oct 13, 2022
@SgtPooki
Copy link
Collaborator Author

SgtPooki commented Oct 13, 2022

hopefully fixed browser-service-worker by resolving error in lib/test-util-ipfs-example/playwright/servers.js where server promises(promiseServers), that were awaited, await Promise.all(promiseServers), were being overwritten when stopping the servers (fix at 2c99e77). I believe this was causing the race condition we were seeing in github CI and not locally.

https://github.com/SgtPooki/js-ipfs-examples/actions/runs/3245576932/jobs/5323287852

@SgtPooki
Copy link
Collaborator Author

Previous fix was not successful.. server is quitting prior to second page.goto request.. will be playing around with this. https://github.com/SgtPooki/js-ipfs-examples/actions/runs/3245623910 now

@SgtPooki
Copy link
Collaborator Author

attempting to increase timeout for the failing test: https://github.com/SgtPooki/js-ipfs-examples/actions/runs/3246023082

@SgtPooki
Copy link
Collaborator Author

Noticed that we were calling server.close instead of server.stop, and stoppable doesn't wait for requests to end if close is used. Hopefully switching to server.stop resolves the race condition: https://github.com/SgtPooki/js-ipfs-examples/actions/runs/3246253813/jobs/5324789350

@SgtPooki
Copy link
Collaborator Author

@hugomrdias @achingbrain any insight you two have for this service worker example failing in CI but not locally? I’ve been diving through playwright docs trying to figure out whats going on and haven’t made much progress.

@SgtPooki
Copy link
Collaborator Author

SgtPooki commented Oct 14, 2022

@SgtPooki
Copy link
Collaborator Author

seems to be aborting the network request. https://github.com/SgtPooki/js-ipfs-examples/actions/runs/3251450530/jobs/5336452807#step:6:83, increasing timeout to see if things resolve within a larger timeframe

@SgtPooki
Copy link
Collaborator Author

SgtPooki commented Oct 14, 2022

resolved the issues by removing the dependence on the iframe entirely as that seems to cause issues with playwright in the github CI. see https://github.com/SgtPooki/js-ipfs-examples/actions/runs/3251560808 for the success.

using npm run build && PWDEBUG=1 playwright test tests --debug we can see that the response does come from the service worker:

image

remove waitForSelectors and ensure service worker response is done before checking for text

debugging: query /view directly

this removes the need for playwright navigating iframes and still utilizes the serviceWorker in the test
@SgtPooki SgtPooki marked this pull request as ready for review October 14, 2022 17:24
@SgtPooki
Copy link
Collaborator Author

@SgtPooki SgtPooki merged commit 49250ad into ipfs-examples:master Oct 14, 2022
@SgtPooki SgtPooki deleted the test-pr-workflows branch October 14, 2022 18:22
@SgtPooki
Copy link
Collaborator Author

merging because this is essential to unblock kubo-rpc-client, and CI is passing, a lot of work done here and this is a QOL improvement

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant