-
Notifications
You must be signed in to change notification settings - Fork 305
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
pytest fixture base_url conflict with pytest_base_url #322
Comments
This certainly does seem like an issue and a rename seems appropriate. I'm wondering if Did you want to make these changes? I suppose one issue is that jupyter_server-extension-based projects might be referencing @echarles and @Zsailer have spent time in this area, do you see another way to address this? |
Can we check for available fixtures and provide a message in case it is found, so at least if we break things we provide a path moving forward. Is |
Is is meant to be used by external systems, that lib also adds a flag to control the value of the If we decide on that I can make a PR to change this. |
There is a use of Since server purposely exposes its pytest_plugins.py file for use by extension authors (i.e., is a "fixture provider"), we may want to entertain prefixing a number of fixtures (all perhaps?) thereby protecting our consumers from potential collisions they may encounter. The @danielfrg - is your project blocked by this? I'm wondering if we shouldn't just open a PR, prefix |
All that makes sense. Its relatively blocked because tests are not going to pass unless i either remove the pytest-plugin or we fix it on jupyter_server. |
In talking with a colleague about this, they suggested that we formalize server's pytest-plugins and create a package: pytest-jupyter-server (or something similar) and use a shorter prefix of |
Thanks for the comments and for getting together to fix this! |
Right on - thank you for your patience on this. Please feel free to join tomorrow's meeting if you're available. |
We discussed this our out last Jupyter Server meeting, @danielfrg. We're working on a separate pytest-plugin library, pytest-jupyter, that other projects (like yours) can use for testing. When I originally created the jupyter server plugin, I didn't expect that it would be used outside this library. We're learning now that it's quite useful for extensions (e.g. jupyterlab, nbclassic, etc). This new plugin library will properly namespace fixtures (using a prefix) so there is little chance of fixture collisions. We should have this plugin released by the end of the week (hopefully sooner). Thank you for the patience on this! |
@danielfrg—OK, you should be able to try out pytest-jupyter now. It provides all the fixtures you need to test your extension with jupyter_server. You'll need to update the fixture names with the prefix We'll likely release this package on PyPI later this week (and add documentation). For now, you can point at pytest-jupyter's main branch to get your CI to pass. |
Thanks for the release! I still see that This would probably make it so they are still incompatible with other pytest plugins, is this correct? |
That particular fixture is renamed to |
Makes sense, thank you! I will wait for that branch to be merged. |
@danielfrg, after a few iterations on our pytest plugin (and dependencies), Jupyter Server 1.1 was released with a new pytest plugin. All fixtures have been prefixed with To use this plugin for your extension, install the test dependencies of jupyter_server:
then add the plugin to your
I'll add this info to our documentation sometime today. |
Thanks for the heads up it totally fixed my issue. |
* bump pycryptodome * encode strings for shared keys
Looks like this package now defines a pytest fixture named
base_url
that conflicts with another plugin https://pypi.org/project/pytest-base-url/ that defines the same fixture but with different scopeI started to get this error from a CI job here with the following logs:
Not sure if there is a better way to fix this other than renaming the fixture here to something like
jupyter_base_url
.The text was updated successfully, but these errors were encountered: