-
-
Notifications
You must be signed in to change notification settings - Fork 14
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 Safari runner initialization in selenium >= 4.20 #135
Conversation
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!
for more information, see https://pre-commit.ci
…pytest-pyodide into safari-selenium-4.20
@@ -52,6 +52,7 @@ python_version = "3.10" | |||
show_error_codes = true | |||
warn_unreachable = true | |||
enable_error_code = ["ignore-without-code", "redundant-expr", "truthy-bool"] | |||
disable_error_code = ["attr-defined"] |
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 wonder if there is a way for us to type annotate our additions to pytest
rather than doing this. For instance, what about:
@dataclass
class PytestPyodideExtra:
pyodide_dist_dir: str
# ... other fields
def pytest_pyodide_extra() -> PytestPyodideExtra:
return pytest.pytest_pyodide_extra
Also, shouldn't we be adding these extra fields to the pytest config object so they aren't globals? Seems like that would be a better design and make the test mocks simpler. Probably can leave this to a followup though.
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 think we should not add attributes to pytest and instead manage those variables ourselves, such as:
SINGLETON_CONFIG = {...}
def get_config():
return SINGLETON_CONFIG
##########
from pytest_pyodide import get_config
config = get_config()
print(config.pyodide_runtimes)
I removed Node 18 from the CI as it seems to fail in jspi tests. |
Resolves #134. There was some breaking changes in
DriverFinder
class in selenium 4.20