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

Remix integration test #380

Merged
merged 9 commits into from
Feb 4, 2022
Merged

Remix integration test #380

merged 9 commits into from
Feb 4, 2022

Conversation

GregBrimble
Copy link
Member

No description provided.

@changeset-bot
Copy link

changeset-bot bot commented Feb 3, 2022 •
edited
Loading

🦋 Changeset detected

Latest commit: 7283640

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
wrangler Patch

Not sure what this means? Click here to learn what changesets are.

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

@petebacondarwin petebacondarwin force-pushed the remix-integration-test branch 2 times, most recently from 6233b40 to b1f4154 Compare February 3, 2022 22:29
@github-actions
Copy link
Contributor

github-actions bot commented Feb 3, 2022 •
edited
Loading

wrangler prerelease is available for testing:

npm install --save-dev https://prerelease-registry.developers.workers.dev/runs/1796331045/wrangler

@GregBrimble GregBrimble marked this pull request as ready for review February 3, 2022 23:27
GregBrimble
GregBrimble commented Feb 3, 2022
View reviewed changes
Copy link
Member Author

@GregBrimble GregBrimble left a comment

Choose a reason for hiding this comment

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

👍 Approve your changes @petebacondarwin

@petebacondarwin petebacondarwin force-pushed the remix-integration-test branch 2 times, most recently from e355f25 to 49d79e4 Compare February 4, 2022 17:35
GregBrimble and others added 9 commits February 4, 2022 17:42
@GregBrimble @petebacondarwin
Configure Remix example project
b78835c
@GregBrimble @petebacondarwin
Add boot-up test
8db6755
@GregBrimble @petebacondarwin
Prettify
c27e3d5
@GregBrimble @petebacondarwin
Fix linting
10d7eda
@GregBrimble @petebacondarwin
Refactor test
50c2ae1
@petebacondarwin
fix: ensure pages routes are defined correctly
ea538f7 
In e151223 we introduced a bug where the RouteKey was now an array rather than a simple URL string. When it got stringified into the routing object these were invalid.
E.g. `[':page*', undefined]` got stringified to `":page*,"` rather than `":page*"`.

Fixes #379
@GregBrimble @petebacondarwin
Add env and cwd
8e05958
@GregBrimble @petebacondarwin
Build Remix before wrangler dev server
c1b614f
@petebacondarwin
refactor: clean up pages routing
7283640
@petebacondarwin petebacondarwin merged commit aacd1c2 into main Feb 4, 2022
@petebacondarwin petebacondarwin deleted the remix-integration-test branch February 4, 2022 17:49
@github-actions github-actions bot mentioned this pull request Feb 4, 2022
Merged
@threepointone
Copy link
Contributor

The test appears to be failing in main, but not failing the jest process. expected?
image

@petebacondarwin petebacondarwin mentioned this pull request Feb 4, 2022
Closed
@petebacondarwin
Copy link
Contributor

@threepointone - is the question why is this failing? Or why is it not failing the jest process?

I think the failure could be caused by not building wrangler beforehand?

@threepointone
Copy link
Contributor

No, the test fails even if I've built wrangler. Do you not see this on your machine?

is the question why is this failing? Or why is it not failing the jest process?

Both, I suppose? just curious why I'm seeing a failing test, and it's not failing CI haha.

@petebacondarwin
Copy link
Contributor

It is passing on main on my machine... Did you npm i to get the latest xxhash package?

@threepointone
Copy link
Contributor

I had done a fresh install, but ugh, I tried again, and now it's not failing anymore lmao. Computers...

Have a good weekend!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@petebacondarwin petebacondarwin petebacondarwin approved these changes

@threepointone threepointone Awaiting requested review from threepointone

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
@GregBrimble @threepointone @petebacondarwin