Merge lp:~mvo/software-center/new-pep8 into lp:software-center

[Michael Vogt]

This branch fixes the pep8 checker on quantal where the internal API has changed so that the current test_pep8.py will work even if there are pep8 errors.

The pep8 checks also got stricter I will do a separate branch for that that fixes them.

[Michael Vogt (mvo)]

Is there anything that is blocking this branch from getting reviewed?

[Kiwinote (kiwinote)]

The proposed changes are fine - looking at the totality of the file we could simplify it slightly:
- afaics there is no reason why we should setup the test env - pep8 is a static test
- we can hardcode in the softwarecenter package for simplicity - the only thing we may change in the future is testing the whole source, we presumably won't go back to testing subpackages

A simplified test_pep8.py could eg be this:
===
import os.path
import softwarecenter
import subprocess
import unittest

class PackagePep8TestCase(unittest.TestCase):
    def test_all_code(self):
        res = subprocess.call(
                ["pep8",
                 "--repeat",
                 os.path.dirname(softwarecenter.__file__)
                 ])
        self.assertEqual(res, 0)

if __name__ == "__main__":
    unittest.main()
===

Feel free to merge either version, as they both fix the test itself.

review: Approve

[Kiwinote (kiwinote)]

(looks like launchpad stripped some newlines from that code snippet, making it non-pep8 compliant..)

[lp:~mvo/software-center/new-pep8]

3078. By Michael Vogt on 2012-08-15

tests/test_pep8.py: simplify based on the feedback from kiwinote (many thanks)

[Michael Vogt (mvo)]

On Wed, Aug 15, 2012 at 09:39:20AM -0000, Kiwinote wrote:
> Review: Approve
>
> The proposed changes are fine - looking at the totality of the file we could simplify it slightly:
> - afaics there is no reason why we should setup the test env - pep8 is a static test
> - we can hardcode in the softwarecenter package for simplicity - the only thing we may change in the future is testing the whole source, we presumably won't go back to testing subpackages
[..]
> Feel free to merge either version, as they both fix the test itself.

Thanks for the feedback, this is good feedback, I simplified the code
now.

Cheers,
 Michael

=== modified file 'tests/test_pep8.py'
--- tests/test_pep8.py	2012-06-20 10:20:01 +0000
+++ tests/test_pep8.py	2012-08-15 10:02:24 +0000
@@ -1,52 +1,20 @@
 import os
-import pep8
+import subprocess
 import unittest
-from collections import defaultdict
-
-from tests.utils import (
-    setup_test_env,
-)
-setup_test_env()
-
-# Only test these two packages for now:
+
 import softwarecenter
 
 
 class PackagePep8TestCase(unittest.TestCase):
-    maxDiff = None
-    packages = [softwarecenter]
-    exclude = []
-
-    def message(self, text):
-        self.errors.append(text)
-
-    def setUp(self):
-        self.errors = []
-
-        class Options(object):
-            exclude = self.exclude
-            filename = ['*.py']
-            testsuite = ''
-            doctest = ''
-            counters = defaultdict(int)
-            messages = {}
-            verbose = 0
-            quiet = 0
-            repeat = True
-            show_source = False
-            show_pep8 = False
-            select = []
-            ignore = []
-            max_line_length = 79
-        pep8.options = Options()
-        pep8.message = self.message
-        Options.physical_checks = pep8.find_checks('physical_line')
-        Options.logical_checks = pep8.find_checks('logical_line')
 
     def test_all_code(self):
-        for package in self.packages:
-            pep8.input_dir(os.path.dirname(package.__file__))
-        self.assertEqual([], self.errors)
+        res = 0
+        res += subprocess.call(
+            ["pep8",
+             "--repeat",
+             os.path.dirname(softwarecenter.__file__)
+             ])
+        self.assertEqual(res, 0)
 
 
 if __name__ == "__main__":