remove the pytest_namespace hook in pytest itself without removing support in general #2280
Conversation
… builtin namespaces
RonnyPfannschmidt
changed the title
RonnyPfannschmidt
changed the title
| if config.option.strict: | ||
| pytest.mark._config = config | ||
| MARK_GEN._config = config |
There was a problem hiding this comment.
This will only support two levels, but I think it is enough (pytest "main" calling a test which uses a "testdir")
There was a problem hiding this comment.
@nicoddemus thats a bug i only half-fixed - pytest-unconfigure unsets, pytest_configure sets, whats needed is a better wrapper
There was a problem hiding this comment.
Just a stack then should do the trick, if you think it is worthwhile.
There was a problem hiding this comment.
Still plan to tackle this @RonnyPfannschmidt?
| @@ -335,3 +339,6 @@ def add_mark(self, mark): | |||
| def __iter__(self): | |||
| """ yield MarkInfo objects each relating to a marking-call. """ | |||
| return imap(MarkInfo, self._marks) | |||
|
|
|||
|
|
|||
| MARK_GEN = MarkGenerator() | |||
There was a problem hiding this comment.
perhaps mark_generator_factory would be better than MarkGenerator?
There was a problem hiding this comment.
i#d love a better name for that object in general, nothing good comes to mind tho
There was a problem hiding this comment.
Oops, I wrote that wrong, it was supposed to be:
perhaps mark_generator_factory would be better than MARK_GEN?
There was a problem hiding this comment.
i see, we can def rename the class and the constant
There was a problem hiding this comment.
Actually I was proposing just the variable:
mark_generator = MarkGenerator()But if you are willing, I think "factory" conveys a better name here (that word is also used in the docstring in fact). So perhaps:
mark_factory = MarkFactory()There was a problem hiding this comment.
Still plan to tackle this @RonnyPfannschmidt?
| ) | ||
|
|
||
|
|
||
| def _setup_collect_fakemodule(): |
There was a problem hiding this comment.
I think it would be easier to just have a collect.py module which imports the symbols then, no? Less magic here would be better IMHO. Some tools might be confused/fail/warn that they can't find the file for pytest.collect, for example.
There was a problem hiding this comment.
@nicoddemus that breaks python -m pytest on python2.6 - else i could just make it a file, move the main hack and be happy
There was a problem hiding this comment.
Just to be clear, just creating _pytest/collect.py which imports those simbols, and add a line from _pytest import collect in pytest.py breaks Python2.6? Just trying to clarify in case I didn't make my point across.
There was a problem hiding this comment.
i misunderstood, as turning pytest into a package and putting a collect filr there
There was a problem hiding this comment.
Still plan to tackle this @RonnyPfannschmidt?
|
What would the effect be if the pytest_namespace hook got called later, i.e. right at the beginning of the pytest_configure call? Would that solve the initialisation problems as well? |
|
@flub then it would no longer work as intended and break users personally i just want it gone because it cant be implemented correctly while retaining a sane life-cycle for the pluginmanager instances |
|
Sure, I understand your motivation. I was thinking as a way forward for plugin authors who still want to do this, we would not be able to stop them doing this in pytest_configure anyway so why not move where it gets invoked instead of completely removing it once the deprecation cycle is gone. OTOH that could just cause more confusion, but my guess is that most plugins won't notice or care that this hook would be called slightly later. |
|
@flub with this pr i dont want to go about making it unusable or moving it, i just want to make pytest itself stop needing it wrt moving it to pytest_configure, unless we do remove the added elements in pytest_unconfigure, it will just be a different mess, and im not willing to write the correct version of this because its error prone and complex - the gains don't justify the cost to make this work correct |
|
@RonnyPfannschmidt Yeah, fair enough |
RonnyPfannschmidt
changed the title
| modfile.strpath, | ||
| ]) | ||
| if res: | ||
| pytest.fail("command result %s" % res) |
There was a problem hiding this comment.
We should add a test that ensures the official API works:
def test_pytest_api():
import pytest
pytest.Module
pytest.Item
...This will ensure we are not breaking anything now or in the future (it is easy for someone to remove an unused import somewhere and break plugins by accident).
There was a problem hiding this comment.
__all__ and flake8 will find that one
RonnyPfannschmidt
force-pushed
the
namespaces-hook
branch
from
e4d6af1 to
ffc26af
Compare
|
due to a git push misstake i killed this pr, will reopen a new |
motivation: the pytest namespace hook requires to import the plugin system at import time in a unreasonably bad manner just to monkeypatch the namespace
this makes the pytest startup quite a mess and makes details of xdist unreliable and unfixable wrt initial node configuration
additionally the pytest_namespace hook is absolutely and completely broken for nested pytest setups that happen when testing pytest plugins