-
Notifications
You must be signed in to change notification settings - Fork 688
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
C3: Add e2e coverage for deployment #3658
Conversation
|
Codecov Report
@@ Coverage Diff @@
## main #3658 +/- ##
==========================================
+ Coverage 74.98% 75.08% +0.10%
==========================================
Files 194 195 +1
Lines 11301 11351 +50
Branches 2977 2999 +22
==========================================
+ Hits 8474 8523 +49
- Misses 2827 2828 +1 |
There was a problem hiding this 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 😃
One thing I am not sure about are the strings you chose for testing the deployments, they seem a bit small to me and potentially a bit unreliable (can those be for example be present also in error pages? like Create Next App
?)
On the other hand you wouldn't want them to be long and brittle/flaky, so I get that
(maybe the could be turned to more flexible and reliable regexes if need be? not necessarily right now, I don't want you to overengineer things on my account)
Anyways I trust your judgment 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks a lot for the changes, awesome stuff 😄
A wrangler prerelease is available for testing. You can install this latest build in your project with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/5685523545/npm-package-wrangler-3658 You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/5685523545/npm-package-wrangler-3658 Or you can use npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/5685523545/npm-package-wrangler-3658 dev path/to/script.js Additional artifacts:npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/5685523545/npm-package-cloudflare-pages-shared-3658 Note that these links will no longer work once the GitHub Actions artifact expires. |
@@ -157,7 +157,7 @@ const config: FrameworkConfig = { | |||
"--src-dir", | |||
"--app", | |||
"--import-alias", | |||
'"@/*"', | |||
"@/*", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this not required to ensure that weird paths don't get expanded improperly?
* C3: Add e2e coverage for deployment * PR feedback * Adding cleanup script for test projects * C3: Run framework e2e tests concurrently * Fixing project cleanup and tweaking test concurrency
Adds e2e coverage for the deployment bits of c3
What this PR solves / how to test:
Associated docs issue(s)/PR(s):
Author has included the following, where applicable:
Reviewer is to perform the following, as applicable: