Skip to content

refactor: simplify the e2e and script configs #342

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
Merged
merged 6 commits into from
Feb 5, 2025
Merged

refactor: simplify the e2e and script configs #342

merged 6 commits into from
Feb 5, 2025

Conversation

@vicb
Copy link
Contributor

@vicb vicb commented Feb 5, 2025 •
edited
Loading

  • On CI, e2e was rebuilding the apps (via preview), drop that as there is a build step before
  • Factor the playwright configs
  • Automatically assign a different port to each app - no need to hardcode ports in the package.json any more
  • Do not run next dev tests on multiple browsers
  • Remove dev:worker
  • Rename preview:worker to preview - to be consistent with c3 templates

This should make the e2e CI job much faster (timeout is reduced)

@changeset-bot
Copy link

changeset-bot bot commented Feb 5, 2025 •
edited
Loading

⚠️ No Changeset found

Latest commit: 094eb08

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

@pkg-pr-new
Copy link

pkg-pr-new bot commented Feb 5, 2025 •
edited
Loading

Open in Stackblitz

pnpm add https://pkg.pr.new/@opennextjs/cloudflare@342

commit: 094eb08

@vicb vicb force-pushed the simplify-configs branch from d06f957 to 6deed1a Compare February 5, 2025 07:45
@vicb vicb changed the title refactor: simplify the configs refactor: simplify the e2e and script configs Feb 5, 2025
@vicb vicb force-pushed the simplify-configs branch from 0757bd9 to a987f0e Compare February 5, 2025 08:50
@vicb vicb force-pushed the simplify-configs branch from a987f0e to f55b79f Compare February 5, 2025 08:51
vicb
vicb commented Feb 5, 2025
View reviewed changes
dario-piotrowicz
dario-piotrowicz approved these changes Feb 5, 2025
View reviewed changes
Copy link
Contributor

@dario-piotrowicz dario-piotrowicz left a comment

Choose a reason for hiding this comment

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

Very nice improvement 🫶

dario-piotrowicz
dario-piotrowicz approved these changes Feb 5, 2025
View reviewed changes
Copy link
Contributor

@dario-piotrowicz dario-piotrowicz left a comment

Choose a reason for hiding this comment

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

Very nice improvement 🫶

vicb
vicb commented Feb 5, 2025
View reviewed changes
dario-piotrowicz
dario-piotrowicz reviewed Feb 5, 2025
View reviewed changes
@dario-piotrowicz
Copy link
Contributor

@vicb this is unrelated to this PR but I wanted to mention that here I notice that we use npm instead of pnpm for some of the examples, we even have some package-lock files: https://github.com/opennextjs/opennextjs-cloudflare/blob/simplify-configs/examples/bugs/gh-219/package-lock.json, that's (I hope we all agree) clearly wrong and we should just consolidate everything to use the pnpm workspace

@vicb
Copy link
Contributor Author

vicb commented Feb 5, 2025

@vicb this is unrelated to this PR but I wanted to mention that here I notice that we use npm instead of pnpm for some of the examples, we even have some package-lock files: https://github.com/opennextjs/opennextjs-cloudflare/blob/simplify-configs/examples/bugs/gh-219/package-lock.json, that's (I hope we all agree) clearly wrong and we should just consolidate everything to use the pnpm workspace

Good catch.

Yep, we should use pnpm everywhere. I didn't pay attention when importing the apps for e2e.
I'll handle that in a follow up PR.

@vicb
Copy link
Contributor Author

vicb commented Feb 5, 2025

Thanks for the review Dario!

@vicb vicb merged commit 1fd08b3 into main Feb 5, 2025
7 checks passed
@vicb vicb deleted the simplify-configs branch February 5, 2025 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dario-piotrowicz dario-piotrowicz dario-piotrowicz approved these changes

@james-elicx james-elicx Awaiting requested review from james-elicx

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@vicb @dario-piotrowicz