-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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: Verify API tests #2467
chore: Verify API tests #2467
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
||
const uri = await modalPage.getConnectUri() | ||
await walletPage.connectWithUri(uri) | ||
await expect(walletPage.page.getByText('Website flagged')).toBeVisible() |
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.
shouldn't this be on the validator?
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.
What do you mean?
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.
assertions are generally on the validator (ModalValidator)
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.
I don't think we need to over-abstract this. IMO fine as-is
911daaf
to
e2b5b1c
Compare
Coverage Report for Coverage
File CoverageNo changed files found. |
♻️ Vite-Size ♻️Size Difference
Current Size
Base Size
|
Why did we remove |
I did that in this PR as we were trying to improve test reliability and fixtures were a potential source of distrust. |
@@ -0,0 +1,56 @@ | |||
import { createWeb3Modal } from '@web3modal/wagmi/react' |
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 from our regular pages?
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.
The metadata URL is set to https://localhost:3000
and it uses a special project ID with Verify API enabled on localhost
import { ModalPage } from '../pages/ModalPage' | ||
import { timingFixture } from './timing-fixture' | ||
|
||
export const testMVerifyDomainMismatch = 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.
Why use a fixture here?
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.
Because we are making ModalPage
inside the fixture and I need to supply custom config to it, requiring a different fixture.
I could make a new ModalPage
using the same fixture, but then wouldn't it be doing unnecessary loading of the wrong page?
|
||
function getUrlByFlavor(baseUrl: string, library: string, flavor: ModalFlavor) { | ||
const urlsByFlavor: Partial<Record<ModalFlavor, string>> = { | ||
default: `${baseUrl}library/${library}/`, | ||
external: `${baseUrl}library/external/` | ||
external: `${baseUrl}library/external/`, | ||
'verify-valid': `${baseUrl}library/verify-valid/`, |
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.
Why the special cases?
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.
Because we need to load the special configurations that are on these pages
Changes
Other