-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
chore: test ethers verify API + AppKit in iframe verify test case #2831
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Coverage Report for Coverage
File CoverageNo changed files found. |
♻️ Vite-Size ♻️Size Difference
Current Size
Base Size
|
…/test-ethers-verify
|
@@ -30,6 +30,18 @@ export const testM = timingFixture.extend<ModalFixture>({ | |||
await use(modalPage) | |||
} | |||
}) | |||
export const testMEthers = timingFixture.extend<ModalFixture>({ |
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.
How is this different fromcthe default testM? / why does it need a special case?
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.
This should be refactored in the latest revision
…/test-ethers-verify
… specific projects only
object-src 'none'; | ||
base-uri 'self'; | ||
form-action 'self'; | ||
frame-ancestors 'none'; | ||
frame-ancestors ${verifyApiNestedIframesTestOuterDomain}; |
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.
do we need to conditionally add this on test env?
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.
iirc adding it conditionally was tricky because the app is always built with NODE_ENV
production, so it didn't know if it was building for tests or not
Didn't bother going deeper I don't think, so hardcoded an intentionally-long domain here which wouldn't be valid anyway
Description
Adds missing test coverage for Verify API on the Ethers SDK.
Also adds test coverage for https://github.com/WalletConnect/cloud-app/pull/1509
Remaining work:
Fixes APKT-1445
Type of change