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.

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

Fix CNA automated tests #2047

Merged
merged 1 commit into from
Apr 18, 2023
Merged

Fix CNA automated tests #2047

merged 1 commit into from
Apr 18, 2023

Conversation

robinmetral
Copy link
Contributor

Purpose

The tests have been failing for about three months and I didn't notice at all (?)

This wasn't an issue with the app, which always built properly, but with the automated test suite (a.k.a. puppeteer checking that the thing opens when the app is built and ran with next start)

Approach and changes

This is a bit of a mess. Looks like at the core it's an issue that a start-server-and-test subdependency (wait-on) has with Node.js v18 (so might have started when we bumped Node.js versions in our version matrices?). Ref: jeffbski/wait-on#137

Expectedly, a lot of people seem to be running into the issue and are reporting this in the start-server-amd-test repo. This issue thread for example: bahmutov/start-server-and-test#181 suggests various solutions, which I've tried locally without success.

In the end what worked was the opposite of one of the suggested solutions: switching the watched URL from localhost:3000 to 0.0.0.0:3000. This worked locally, worked in CI (ran the workflow on this branch), so I think it's safe to merge.

I don't really understand why this solves it but this seems like a viable short-term fix—long term, if this doesn't get fixed upstream and starts happening again, I'd suggest exploring alternatives to start-server-and-test (which would probably be more straightforward and cleaner than attempting to patch the subdep).

Definition of done

  • Development completed
  • Reviewers assigned
  • (n/a) Unit and integration tests
  • (n/a) Meets minimum browser support
  • (n/a) Meets accessibility requirements

@robinmetral robinmetral requested a review from a team as a code owner April 18, 2023 13:48
@robinmetral robinmetral requested review from connor-baer and removed request for a team April 18, 2023 13:48
@changeset-bot
Copy link

changeset-bot bot commented Apr 18, 2023

⚠️ No Changeset found

Latest commit: b02d761

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Apr 18, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
oss-circuit-ui ✅ Ready (Inspect) Visit Preview Apr 18, 2023 1:48pm

@sumup-oss sumup-oss deleted a comment from sumup-clark bot Apr 18, 2023
@codecov
Copy link

codecov bot commented Apr 18, 2023

Codecov Report

Merging #2047 (b02d761) into main (f204756) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2047   +/-   ##
=======================================
  Coverage   92.16%   92.16%           
=======================================
  Files         168      168           
  Lines        3537     3537           
  Branches     1224     1224           
=======================================
  Hits         3260     3260           
  Misses        256      256           
  Partials       21       21           

@robinmetral robinmetral enabled auto-merge (squash) April 18, 2023 13:48
@robinmetral robinmetral merged commit a62c441 into main Apr 18, 2023
@robinmetral robinmetral deleted the fix-cna-tests branch April 18, 2023 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants