-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
fix: Fixed flaky test for custom screen size #27067
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -266,7 +266,6 @@ export const getSwapSendFixtures = ( | |
) => { | ||
const ETH_CONVERSION_RATE_USD = 3010; | ||
return { | ||
driverOptions: { responsive: true }, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would like to confirm with the original author before we delete this. @BZahory, was there a reason that this was originally written so that the dev tools would be open during the test run (here is where your PR first added this: d0a0037#diff-f2647a24cd0065e584933d0fdc32081d6f28f75b7d1123533b91e02109d885fcR244_ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @danjm Sure, I agree with you we should confirm. Just to add BZahory may not reply as he is no longer working with the Extension. I am tagging the team @MetaMask/metamask-assets here for faster confirmation and this is my observation according to the PR in the screenshot and the test scenario where the extension is just clicked and hence I am thinking the size of the window is not mandatory. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can't currently see how the responsive screen size was needed, so I think it is fine to remove for now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks Dan. |
||
fixtures: new FixtureBuilder() | ||
.withPreferencesController({ | ||
featureFlags: {}, | ||
|
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 window size is not mandatory for the test, so I have removed it.