Two false positives in roundup unittest TestCase

[jayvdb]

I've been testing this on a few very large repos, and finding the occasional error

In roundup (http://www.roundup-tracker.org/index.html), only the following:

test/test_templating.py:350:0: P103 other string does contain unindexed parameters test/test_mailgw.py:2826:65: P103 other string does contain unindexed parameters test/test_mailgw.py:2842:65: P103 other string does contain unindexed parameters test/test_mailgw.py:2857:65: P103 other string does contain unindexed parameters

http://sourceforge.net/p/roundup/code/ci/default/tree/test/test_templating.py http://sourceforge.net/p/roundup/code/ci/default/tree/test/test_mailgw.py

Definitely within the realms of reasonable use of noqa, but maybe worth fixing.

[xZise]

The last three in test_mailgw have basically the same reason as #4. But the test_templating.py could be solved as that is not an assignment.

[jayvdb]

Detecting parent class unittest.TestCase could allow a separate code for violations in test cases, like violations in docstrings are a separate case. That would reduce the impact of #2 in pywikibot, as we could use the new code to allow unittests to use {} but prevent it in the main library.

That wont solve py.test based projects, but someone else might find devise a way to detect those, and they can always use file selection rules to prevent their tests being included.

All of the violations in https://github.com/GoogleCloudPlatform/gcloud-python (mentioned in https://github.com/google/oauth2client/pull/328 ) are in subclasses of unittest.TestCase, and I've noticed this is quite common in other projects.

[jayvdb]

A very faulty detection of pytest would be import pytest.

[xZise]

And how would you want to get the parent class? It only parses one module, so it can only work if the class does only subclasses the TestCase class or classes from that module. If there should be a test specific code (what would happen to P102 and P101? Are they detected as normal and P103 only is changed) I think a more sensible solution is to specify a test path.

At least the test_templating issue is now solved (will upload a patch shortly).

[jayvdb]

Can you see whether import unittest appears in the __init__.py of the module being checked?

Using a test file glob might be the only sensible approach. Would yoube willing to add one as a configuration option for your plugin. Otherwise people running flake8 need to run it with the different sets of options, and/or build two different flake8 rules in tox: one for tests, and one for other. Not a deal breaker, but would be nice.

IMO only P103 would be split into a separate code for 'test specific'.

[jayvdb]

With Astroid it is possible to get information about the class MRO.

>>> from astroid.test_utils import extract_node
>>> n = extract_node("""
... from unittest import TestCase
... class A(TestCase):
...    def test_foo(self):
...       pass
... """)
>>> print(n.mro())
[<ClassDef.A l.3 at 0x13ceb50>, <ClassDef.TestCase l.171 at 0x14430d0>, <ClassDef.object l.0 at 0xd6f290>]
>>> n.bases
[<Name.TestCase l.3 at 0x13cec10>]
>>> n.mro()[1].parent
<Module.unittest.case l.0 at 0x142b310>