Skip to content

Reintroduce get_plugin_manager() for backward-compatibility #787

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

Merged
merged 1 commit into from
Jul 8, 2015

Conversation

nicoddemus
Copy link
Member

PyCharm depends on get_plugin_manager() for its pytest runner.

Using the pytest runner in PyCharm as of 2.8.0.dev4 raises this exception:

Traceback (most recent call last):
  File "D:\Programming\PyCharm\helpers\pycharm\pytestrunner.py", line 60, in <module>
    main()
  File "D:\Programming\PyCharm\helpers\pycharm\pytestrunner.py", line 30, in main
    _pluginmanager = get_plugin_manager()
  File "D:\Programming\PyCharm\helpers\pycharm\pytestrunner.py", line 20, in get_plugin_manager
    from _pytest.core import PluginManager
ImportError: No module named '_pytest.core'

The related code in PyCharm\helpers\pycharm\pytestrunner.py is:

def get_plugin_manager():
  try:
    from _pytest.config import get_plugin_manager
    return get_plugin_manager()
  except ImportError:
    from _pytest.core import PluginManager
    return PluginManager(load=True)

get_plugin_manager was removed during the refactoring that lead to pluggy, I guess.

While it could be argued that PyCharm could fix it on their side, keeping backward compatibility in pytest is simple and would prevent a ton of PyCharm users not being able to run py.test tests when we release 2.8.0, at least until a hotfix for PyCharm is released.

@RonnyPfannschmidt
Copy link
Member

nicely done
please add a deprecationwarning with stacklevel 2, so its noticeable more easily

@hpk42
Copy link
Contributor

hpk42 commented Jun 18, 2015

sure,a dding it back is fine but could you add a test noting pycharms needs it so the next person doesn't accidentally delete this method?

nicoddemus added a commit that referenced this pull request Jun 18, 2015
PyCharm pytest runner depends on this function existing (see #787).
@nicoddemus
Copy link
Member Author

Applied suggested changes. 😄

"""deprecated, backward-compatibility for pytest < 2.8 (see #787)"""
warnings.warn('get_plugin_manager() is deprecated, use get_config().pluginmanager instead',
DeprecationWarning, stacklevel=2)
return get_config().pluginmanager
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry i didn't say this earlier but i think we don't need to bother with deprecating it. @RonnyPfannschmidt , do you insist to deprecate it? I think it's ok to offer a method for IDEs and others to get a plugin manager instance.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree we should just keep this around and remove the warning... waiting for @RonnyPfannschmidt's take on it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm wondering more about having a pytest-controller package,
that would support running py.test slave processes in various configurations

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's an interesting idea, but certainly out of the scope of this PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's strike the deprecation and rather document get_plugin_manager() as official API (in plugins.txt i guess)

nicoddemus added a commit that referenced this pull request Jun 25, 2015
PyCharm pytest runner depends on this function existing (see #787).

Added reference to get_plugin_manager() and PluginManager/PytestPluginManager to docs
PyCharm pytest runner depends on this function existing (see #787).

Added reference to get_plugin_manager() and PluginManager/PytestPluginManager to docs
@nicoddemus
Copy link
Member Author

Removed the deprecation warning and added a reference to get_plugin_manager in writing_plugins.txt, as it looked like the better fit for this particular function.

While writing the docs for get_plugin_manager I realized there are no reference in the docs to pluggy.PluginManager or PytestPluginManager, so I added some to see how they would look like. My question is, should I continue to add more docs, or that API is not stable yet so it would be better to not even mentioning them at all, leaving only the reference to the get_plugin_manager function?

@nicoddemus
Copy link
Member Author

Decided to merge, we can decide on improving the documentation later. 😄

nicoddemus added a commit that referenced this pull request Jul 8, 2015
Reintroduce get_plugin_manager() for backward-compatibility
@nicoddemus nicoddemus merged commit ae4c8b8 into master Jul 8, 2015
@nicoddemus nicoddemus deleted the pluggy-bc-fix branch July 8, 2015 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants