-
Notifications
You must be signed in to change notification settings - Fork 194
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
adding getMethodByName function to Contract and Interface #583
Conversation
028a4ae
to
824c750
Compare
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.
Minor comment while we wait for firefox tests
e8c285f
to
a80f3a3
Compare
.setPreference('remote.active-protocols', 1) // Sets protocol to only WebDriver BiDi | ||
.setPreference('network.http.network-changed.timeout', 30) // In Docker tests this apparently matters |
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.
Interesting, I'd expect setting remote.active-protocols
to 2 (CDP) to be the solution here, since in Firefox 101 they changed the default from 2 to 3 (1|2).
Also, for network.http.network-changed.timeout
, have you tried more values, or just 30?
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.
to be fair 1 was the first one I tried, something about the combination is tripping us.
I did not try more settings for timeout, 30s seemed sufficient, the default is 5
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! Thanks again for all the debugging work related to Firefox.
Once @algochoi agrees, this looks good to merge, and I hope this will be the last of the flaky Firefox tests 🙏
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.
Code changes LGTM
No description provided.