fixtures and pylint W0621
Hi, Defining a fixture in the same file as a test that uses it triggers a pylint complaint: "W0621:*Redefining name %r from outer scope (line %s)* Used when a variable’s name hide a name defined in the outer scope." If the fixture was in conftest.py it would be fine. But it is nice to have fixtures in the same file if they are only used there. Does everybody just disable this message? I find it useful in general, I would prefer not to disable it completely. thanks Brianna -- They've just been waiting in a mountain for the right moment: http://modernthings.org/
Hi Brianna, On Tue, Nov 26, 2013 at 14:45 +1100, Brianna Laugher wrote:
Hi,
Defining a fixture in the same file as a test that uses it triggers a pylint complaint:
"W0621:*Redefining name %r from outer scope (line %s)* Used when a variable’s name hide a name defined in the outer scope."
If the fixture was in conftest.py it would be fine. But it is nice to have fixtures in the same file if they are only used there. Does everybody just disable this message? I find it useful in general, I would prefer not to disable it completely.
I use flakes and pep8 (pytest-flakes and pytest-pep8) and don't get this error. What you can immediately do but which is not pretty is to use the old naming scheme together with a decorator: import pytest @pytest.fixture def pytest_funcarg__somefixture(...): ... This should avoid the pylint warning. Alternatively, putting the fixture on a class-level might avoid the problem: class TestClass: @pytest.fixture def fix(self): .... def test_fix(self, fix): ... but i haven't tried if pylint likes that. best, holger
Hi, On 26 November 2013 18:58, holger krekel <holger@merlinux.eu> wrote:
I use flakes and pep8 (pytest-flakes and pytest-pep8) and don't get this error. What you can immediately do but which is not pretty is to use the old naming scheme together with a decorator:
import pytest
@pytest.fixture def pytest_funcarg__somefixture(...): ...
This should avoid the pylint warning.
Ah, that's cool - I didn't realise that was possible. However, it doesn't work the way you describe. :) Not sure if your description or the code is backwards! At the moment (pytest-2.3.5) if I have two fixtures like this: @py.test.fixture def pytest_funcarg__foo2(monkeypatch): return monkeypatch def pytest_funcarg__foo3(monkeypatch): return monkeypatch foo3 works (!). foo2 causes an assertion error on collection. The code in parsefactories has a couple of if blocks, maybe one is inverted. :) Don't know if this is a good idea, but you could tell if it was new style/old style based on if the parameter was 'request' or nothing/other fixtures. cheers Brianna -- They've just been waiting in a mountain for the right moment: http://modernthings.org/
On Tue, Dec 03, 2013 at 14:51 +1100, Brianna Laugher wrote:
On 26 November 2013 18:58, holger krekel <holger@merlinux.eu> wrote:
I use flakes and pep8 (pytest-flakes and pytest-pep8) and don't get this error. What you can immediately do but which is not pretty is to use the old naming scheme together with a decorator:
import pytest
@pytest.fixture def pytest_funcarg__somefixture(...): ...
This should avoid the pylint warning.
Ah, that's cool - I didn't realise that was possible.
However, it doesn't work the way you describe. :) Not sure if your description or the code is backwards!
At the moment (pytest-2.3.5) if I have two fixtures like this:
@py.test.fixture def pytest_funcarg__foo2(monkeypatch): return monkeypatch
def pytest_funcarg__foo3(monkeypatch): return monkeypatch
foo3 works (!). foo2 causes an assertion error on collection.
The code in parsefactories has a couple of if blocks, maybe one is inverted. :)
Right, it seems that when we introduced @pytest.fixture we decided you can either use the prefix or the marker. That could be lifted but i wonder if we should rather go for a different convention because pytest_funcarg__ is not a beautiful prefix. What do you think of pytest stripping the "__" prefix? @pytest.fixture def __foo2(monkeypatch): return monkeypatch This fixture would become accessible via the "foo2" name. Using "__foo2" would yield a lookup error and the error would indicate there is a "foo2" available. If you don't like it, any other suggestions?
Don't know if this is a good idea, but you could tell if it was new style/old style based on if the parameter was 'request' or nothing/other fixtures.
There is no difference between old-style and @pytest.fixture(scope="function") and the presence of "request" indicates nothing particular. best, holger
cheers Brianna
-- They've just been waiting in a mountain for the right moment: http://modernthings.org/
On 3 December 2013 18:39, holger krekel <holger@merlinux.eu> wrote:
Right, it seems that when we introduced @pytest.fixture we decided you can either use the prefix or the marker. That could be lifted but i wonder if we should rather go for a different convention because pytest_funcarg__ is not a beautiful prefix. What do you think of pytest stripping the "__" prefix?
@pytest.fixture def __foo2(monkeypatch): return monkeypatch
This fixture would become accessible via the "foo2" name. Using "__foo2" would yield a lookup error and the error would indicate there is a "foo2" available. If you don't like it, any other suggestions?
Hmm. My main concern here would be that fixture definitions are easily 'findable' in a global search. The fixture decorator is easy to search for, as is 'pytest_funcarg__'. Such an ugly prefix is also far easier to google. Double underscore, not so much. The ugly prefix is also good for backwards compat. So, I don't really mind what happens as long as each fixture has at least one of fixture decorator/ugly prefix. cheers Brianna -- They've just been waiting in a mountain for the right moment: http://modernthings.org/
On Wed, Dec 04, 2013 at 13:29 +1100, Brianna Laugher wrote:
On 3 December 2013 18:39, holger krekel <holger@merlinux.eu> wrote:
Right, it seems that when we introduced @pytest.fixture we decided you can either use the prefix or the marker. That could be lifted but i wonder if we should rather go for a different convention because pytest_funcarg__ is not a beautiful prefix. What do you think of pytest stripping the "__" prefix?
@pytest.fixture def __foo2(monkeypatch): return monkeypatch
This fixture would become accessible via the "foo2" name. Using "__foo2" would yield a lookup error and the error would indicate there is a "foo2" available. If you don't like it, any other suggestions?
Hmm. My main concern here would be that fixture definitions are easily 'findable' in a global search. The fixture decorator is easy to search for, as is 'pytest_funcarg__'. Such an ugly prefix is also far easier to google. Double underscore, not so much. The ugly prefix is also good for backwards compat.
So, I don't really mind what happens as long as each fixture has at least one of fixture decorator/ugly prefix.
Well, ok. Pending further input, i made pytest accept pytest.fixture decorated pytest_funcarg__ prefixed functions, see https://bitbucket.org/hpk42/pytest/commits/aa1f0505a3156b9feca43cd67c5afc956... I am a bit unhappy because apart from the pylint warning it's also a real issue for beginners to write something like this:: @pytest.fixture def somename(): return 42 def test_something(): assert somename == 42 The "somename" is accessible (as a global) but is a function object. The resulting error message is irritating if you are not proficient in python. To prevent this irritation we would need to recommend a different default way of declaring the fixture and neither pytest_funcarg__NAME nor __NAME sound like we would like to suggest this as the main way in the docs. Then again, my general recommendation is to put fixture functions into conftest.py files and you then get a clean "NameError" in a test module for the above example. The docs use the "everything contained in test module" method, though, to make it easier to present getting-started examples. Maybe it makes sense to mention this recommended conftest/test module split earlier on. best, holger
perhaps pytest.fixture could return a special object that rejects all operations, rather than the decorated function object; such that `theobject == 5` would produce a big flashy error in theobject.__eq__ indicating you need to actually use the fixture for this to work, maybe even with an example. Repeat for all magic methods that make sense. this object would maybe have a _pytest_fixture attribute, or conform to whatever protocol already exists for detecting fixture objects, such that the only thing you could do with such an object is check if it's a fixture and get the original function object from somewhere inside it. Any other operation would result in a flashy error. On Tue, Dec 3, 2013 at 11:26 PM, holger krekel <holger@merlinux.eu> wrote:
On 3 December 2013 18:39, holger krekel <holger@merlinux.eu> wrote:
Right, it seems that when we introduced @pytest.fixture we decided you can either use the prefix or the marker. That could be lifted but i wonder if we should rather go for a different convention because
On Wed, Dec 04, 2013 at 13:29 +1100, Brianna Laugher wrote: pytest_funcarg__
is not a beautiful prefix. What do you think of pytest stripping the "__" prefix?
@pytest.fixture def __foo2(monkeypatch): return monkeypatch
This fixture would become accessible via the "foo2" name. Using "__foo2" would yield a lookup error and the error would indicate there is a "foo2" available. If you don't like it, any other suggestions?
Hmm. My main concern here would be that fixture definitions are easily 'findable' in a global search. The fixture decorator is easy to search for, as is 'pytest_funcarg__'. Such an ugly prefix is also far easier to google. Double underscore, not so much. The ugly prefix is also good for backwards compat.
So, I don't really mind what happens as long as each fixture has at least one of fixture decorator/ugly prefix.
Well, ok. Pending further input, i made pytest accept pytest.fixture decorated pytest_funcarg__ prefixed functions, see
https://bitbucket.org/hpk42/pytest/commits/aa1f0505a3156b9feca43cd67c5afc956...
I am a bit unhappy because apart from the pylint warning it's also a real issue for beginners to write something like this::
@pytest.fixture def somename(): return 42
def test_something(): assert somename == 42
The "somename" is accessible (as a global) but is a function object. The resulting error message is irritating if you are not proficient in python. To prevent this irritation we would need to recommend a different default way of declaring the fixture and neither pytest_funcarg__NAME nor __NAME sound like we would like to suggest this as the main way in the docs.
Then again, my general recommendation is to put fixture functions into conftest.py files and you then get a clean "NameError" in a test module for the above example. The docs use the "everything contained in test module" method, though, to make it easier to present getting-started examples. Maybe it makes sense to mention this recommended conftest/test module split earlier on.
best, holger _______________________________________________ Pytest-dev mailing list Pytest-dev@python.org https://mail.python.org/mailman/listinfo/pytest-dev
Hi, On 4 December 2013 15:05, lahwran <lahwran@lahwran.net> wrote:
perhaps pytest.fixture could return a special object that rejects all operations, rather than the decorated function object; such that `theobject == 5` would produce a big flashy error in theobject.__eq__ indicating you need to actually use the fixture for this to work, maybe even with an example. Repeat for all magic methods that make sense. this object would maybe have a _pytest_fixture attribute, or conform to whatever protocol already exists for detecting fixture objects, such that the only thing you could do with such an object is check if it's a fixture and get the original function object from somewhere inside it. Any other operation would result in a flashy error.
This seems like a quite nice idea to me.
On Tue, Dec 3, 2013 at 11:26 PM, holger krekel <holger@merlinux.eu> wrote:
Well, ok. Pending further input, i made pytest accept pytest.fixture decorated pytest_funcarg__ prefixed functions, see
https://bitbucket.org/hpk42/pytest/commits/aa1f0505a3156b9feca43cd67c5afc956...
I don't really like this change. IIRC this was considered when the decorator was introduced and the reason it was not allowed originally is because the signature is different between the different ways of defining fixtures. That can be very confusing too. My first reaction on this pylint issue was to see if this can not be fixed wit a pylint plugin. And I still think this would be nicer then the other proposals of trying to figure out how to name fixtures differently. The way of declaring fixtures right now is very nice and readable. Especially if lahwran's suggestion is implemented as that would make any mistakes obvious (and to an experience python developer they already are obvious).
Then again, my general recommendation is to put fixture functions into conftest.py files and you then get a clean "NameError" in a test module for the above example. The docs use the "everything contained in test module" method, though, to make it easier to present getting-started examples. Maybe it makes sense to mention this recommended conftest/test module split earlier on.
Personally I tend to recommend people to declare their fixtures as close to the code using it. So if a fixture is only used in one class declare it in that class. I find it easier to manage which fixtures are needed and know about the relevant ones without having to search around. Also if you have all test modules in package, like py.test, you quickly end up with colliding fixture names and a giant and hard to manage collection of fixtures in conftest.py. Regards, Floris -- Debian GNU/Linux -- The Power of Freedom www.debian.org | www.gnu.org | www.kernel.org
Hi Floris, all, On Fri, Dec 06, 2013 at 10:07 +0000, Floris Bruynooghe wrote:
On 4 December 2013 15:05, lahwran <lahwran@lahwran.net> wrote:
perhaps pytest.fixture could return a special object that rejects all operations, rather than the decorated function object; such that `theobject == 5` would produce a big flashy error in theobject.__eq__ indicating you need to actually use the fixture for this to work, maybe even with an example. Repeat for all magic methods that make sense. this object would maybe have a _pytest_fixture attribute, or conform to whatever protocol already exists for detecting fixture objects, such that the only thing you could do with such an object is check if it's a fixture and get the original function object from somewhere inside it. Any other operation would result in a flashy error.
This seems like a quite nice idea to me.
Not quite sure. It would mean that if you want to unittest fixture functions, it's slightly hairy. More importantly, i think sometimes i use (and probably others as well) direct calling of fixture functions for other purposes already: @pytest.fixture def arg(request): ... @pytest.fixture def arg2(request): if some_condition: val = arg(request) ... which is currently valid code. Arguably one could use getfuncargvalue but IIRC i had some reasons to not do it at the time. (Can't locate the code currently). That being sad, i am open to trying it -- so if somebody does a PR introducing the "exploding" @pytest.fixture return value, i'd merge it temporarily and see if there are any problems. It would be very nice if random errors of forgetting to specify an argument would result in a clear error message.
On Tue, Dec 3, 2013 at 11:26 PM, holger krekel <holger@merlinux.eu> wrote:
Well, ok. Pending further input, i made pytest accept pytest.fixture decorated pytest_funcarg__ prefixed functions, see
https://bitbucket.org/hpk42/pytest/commits/aa1f0505a3156b9feca43cd67c5afc956...
I don't really like this change. IIRC this was considered when the decorator was introduced and the reason it was not allowed originally is because the signature is different between the different ways of defining fixtures. That can be very confusing too.
Not sure i follow. What is different? The only difference is that the decorator allows to specify a caching scope, params etc. It does not change anything about the fixture functions own signature.
My first reaction on this pylint issue was to see if this can not be fixed wit a pylint plugin. And I still think this would be nicer then the other proposals of trying to figure out how to name fixtures differently. The way of declaring fixtures right now is very nice and readable. Especially if lahwran's suggestion is implemented as that would make any mistakes obvious (and to an experience python developer they already are obvious).
Aiming for better pylint integration eg. through a plugin sounds like a great plan. And one that does not require deep pytest knowledge -- so if someone here on the list is interested, i am willing to help design it.
Then again, my general recommendation is to put fixture functions into conftest.py files and you then get a clean "NameError" in a test module for the above example. The docs use the "everything contained in test module" method, though, to make it easier to present getting-started examples. Maybe it makes sense to mention this recommended conftest/test module split earlier on.
Personally I tend to recommend people to declare their fixtures as close to the code using it. So if a fixture is only used in one class declare it in that class. I find it easier to manage which fixtures are needed and know about the relevant ones without having to search around. Also if you have all test modules in package, like py.test, you quickly end up with colliding fixture names and a giant and hard to manage collection of fixtures in conftest.py.
True as well. Although for my 1K-10K LOC projects it's not really an issue yet. Maybe some day we can think about namespacing in some way. best, holger
On 8 December 2013 09:40, holger krekel <holger@merlinux.eu> wrote:
On Tue, Dec 3, 2013 at 11:26 PM, holger krekel <holger@merlinux.eu> wrote:
Well, ok. Pending further input, i made pytest accept pytest.fixture decorated pytest_funcarg__ prefixed functions, see
https://bitbucket.org/hpk42/pytest/commits/aa1f0505a3156b9feca43cd67c5afc956...
I don't really like this change. IIRC this was considered when the decorator was introduced and the reason it was not allowed originally is because the signature is different between the different ways of defining fixtures. That can be very confusing too.
Not sure i follow. What is different? The only difference is that the decorator allows to specify a caching scope, params etc. It does not change anything about the fixture functions own signature.
I was assuming the old fixtures do not allow requesting other fixtures via funcargs. But I might well be wrong on that. Regards, Floris -- Debian GNU/Linux -- The Power of Freedom www.debian.org | www.gnu.org | www.kernel.org
Hi, On 8 December 2013 20:40, holger krekel <holger@merlinux.eu> wrote:
My first reaction on this pylint issue was to see if this can not be fixed wit a pylint plugin. And I still think this would be nicer then the other proposals of trying to figure out how to name fixtures differently. The way of declaring fixtures right now is very nice and readable. Especially if lahwran's suggestion is implemented as that would make any mistakes obvious (and to an experience python developer they already are obvious).
Aiming for better pylint integration eg. through a plugin sounds like a great plan. And one that does not require deep pytest knowledge -- so if someone here on the list is interested, i am willing to help design it.
I agree, I think a plugin is the way to go. I have made a bit of an initial stab at a plugin: https://bitbucket.org/pfctdayelise/pylint-pytest/ I am not sure the string building approach will get us very far, but I just wanted to get something that would at least run (a lot of the pylint docs are out of date). Someone should probably run pylint over the pytest src to see what other pylint messages are common. cheers Brianna -- They've just been waiting in a mountain for the right moment: http://modernthings.org/
On Sun, Dec 08, 2013 at 20:49 +0000, Floris Bruynooghe wrote:
On 8 December 2013 09:40, holger krekel <holger@merlinux.eu> wrote:
On Tue, Dec 3, 2013 at 11:26 PM, holger krekel <holger@merlinux.eu> wrote:
Well, ok. Pending further input, i made pytest accept pytest.fixture decorated pytest_funcarg__ prefixed functions, see
https://bitbucket.org/hpk42/pytest/commits/aa1f0505a3156b9feca43cd67c5afc956...
I don't really like this change. IIRC this was considered when the decorator was introduced and the reason it was not allowed originally is because the signature is different between the different ways of defining fixtures. That can be very confusing too.
Not sure i follow. What is different? The only difference is that the decorator allows to specify a caching scope, params etc. It does not change anything about the fixture functions own signature.
I was assuming the old fixtures do not allow requesting other fixtures via funcargs. But I might well be wrong on that.
Indeed, old pytest_funcarg__fixture already accepted other fixtures as arguments, so no difference there. However, i now think and agree that adding @pytest.fixture markers to old-style pytest_funcarg__NAME fixtures is a bit backwards. Going for a pylint plugin probably makes more sense. So i just backed out the change (so now you cannot use @pytest.fixture on pytest_funcarg__NAME). best, holger
Regards, Floris
-- Debian GNU/Linux -- The Power of Freedom www.debian.org | www.gnu.org | www.kernel.org
Hi Brianna, On 9 December 2013 03:22, Brianna Laugher <brianna.laugher@gmail.com> wrote:
I agree, I think a plugin is the way to go.
I have made a bit of an initial stab at a plugin:
I get a message saying I have no permission to access the repository when following that URL. Regards, Floris -- Debian GNU/Linux -- The Power of Freedom www.debian.org | www.gnu.org | www.kernel.org
Sorry, I accidentally made it private - should be accessible now. Brianna On 10/12/2013 8:30 PM, "Floris Bruynooghe" <flub@devork.be> wrote:
Hi Brianna,
On 9 December 2013 03:22, Brianna Laugher <brianna.laugher@gmail.com> wrote:
I agree, I think a plugin is the way to go.
I have made a bit of an initial stab at a plugin:
I get a message saying I have no permission to access the repository when following that URL.
Regards, Floris
-- Debian GNU/Linux -- The Power of Freedom www.debian.org | www.gnu.org | www.kernel.org
participants (4)
-
Brianna Laugher -
Floris Bruynooghe -
holger krekel -
lahwran