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

Add a test for gh-119 #300

Merged
merged 1 commit into from
Jan 30, 2025
Merged

Add a test for gh-119 #300

merged 1 commit into from
Jan 30, 2025

Conversation

vicb
Copy link
Contributor

@vicb vicb commented Jan 29, 2025 •
edited
Loading

Add a test to make sure we have no regression.

The e2e runs on only one browser - do we really need to run the others on 3 different browsers?

@vicb
add test for #119
608bd43 
fixup! open-next.config.ts
fixup! format
fixup! remove --inspector-port to avoid collisions
fixup! inspector port
fixup!
fixup! bump timeout again
fixup!
@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Jan 29, 2025 •
edited
Loading

⚠️ No Changeset found

Latest commit: b0c86fe

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 pkg.pr.new
Copy link

pkg-pr-new bot commented Jan 29, 2025 •
edited
Loading

Open in Stackblitz

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

commit: 608bd43

@vicb vicb force-pushed the gh-119 branch 4 times, most recently from b0c86fe to 608bd43 Compare January 30, 2025 13:20
@vicb vicb changed the title gh-119 Add a test for gh-119 Jan 30, 2025
dario-piotrowicz
dario-piotrowicz reviewed Jan 30, 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.

Looks good to me, but I don't understand, why are you bumping the timeouts to 500_000? I really hope that that is not necessary, is it?

examples/bugs/gh-119/package.json Show resolved Hide resolved
@dario-piotrowicz
Copy link
Contributor

Also what are you adding is not an "example" I wonder if we should have maybe two directories "examples" and "fixtures"/"e2es" or something like that 🤔

or maybe just rename "examples" and simply use them as fixtures

not a big deal either way

@vicb
Copy link
Contributor Author

vicb commented Jan 30, 2025

Looks good to me, but I don't understand, why are you bumping the timeouts to 500_000? I really hope that that is not necessary, is it?

100k was still erroring and making me restart the tests multiple times.

@vicb
Copy link
Contributor Author

vicb commented Jan 30, 2025

Also what are you adding is not an "example" I wonder if we should have maybe two directories "examples" and "fixtures"/"e2es" or something like that 🤔

or maybe just rename "examples" and simply use them as fixtures

not a big deal either way

Feel free to create an issue with refactoring ideas.
For now I have dependent PRs using the same structure so I don't want to loose too much time for that.
Thanks!

@dario-piotrowicz
Copy link
Contributor

Looks good to me, but I don't understand, why are you bumping the timeouts to 500_000? I really hope that that is not necessary, is it?

100k was still erroring and making me restart the tests multiple times.

I haven't seen that... I wonder if it's the new aws apps that take a long time to build or something...

james-elicx
james-elicx approved these changes Jan 30, 2025
View reviewed changes
Copy link
Collaborator

@james-elicx james-elicx left a comment

Choose a reason for hiding this comment

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

the timeout problem is a bit depressing. i would have hoped they wouldn't be that slow

@vicb
Copy link
Contributor Author

vicb commented Jan 30, 2025

Thanks for the reviews!

@vicb vicb merged commit 4d6bf1b into main Jan 30, 2025
7 checks passed
@vicb vicb deleted the gh-119 branch January 30, 2025 16:09
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 james-elicx approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vicb @dario-piotrowicz @james-elicx