Code review comment for lp:~adiroiban/pocket-lint/1168854-google-closure

Hi,

Thanks for the review.

What versions of Python do you want to support?

I tried with 3.3 and got the following errors:

$ python test.py
...........F.............................................................................E.............................................................................ss.......................
======================================================================
ERROR: test_zpt_without_namespace (tests.test_xml.TestXML)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/adi/vcs/pocketlint-branches/1168854-google-closure/pocketlint/tests/test_xml.py", line 90, in test_zpt_without_namespace
    checker.check()
  File "/home/adi/vcs/pocketlint-branches/1168854-google-closure/pocketlint/formatcheck.py", line 433, in check
    self.handle_namespaces(parser)
  File "/home/adi/vcs/pocketlint-branches/1168854-google-closure/pocketlint/formatcheck.py", line 418, in handle_namespaces
    xparser.DefaultHandlerExpand = parser._default
AttributeError: 'xml.etree.ElementTree.XMLParser' object has no attribute '_default'

======================================================================
FAIL: test_code_with_warnings (tests.test_python.TestPyflakes)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/adi/vcs/pocketlint-branches/1168854-google-closure/pocketlint/tests/test_python.py", line 140, in test_code_with_warnings
    self.reporter.messages)
AssertionError: Lists differ: [(3, "undefined name 'b'"), (3... != []

First list contains 2 additional elements.
First extra element 0:
(3, "undefined name 'b'")

+ []
- [(3, "undefined name 'b'"),
- (3, "local variable 'a' is assigned to but never used")]

----------------------------------------------------------------------
Ran 192 tests in 1.368s

FAILED (failures=1, errors=1, skipped=2)

-----

In Python 3.2 I got the following errors:

$ python test.py
...........F..................................................................................F.......................................................................E.............
======================================================================
ERROR: tests.test_javascript (unittest.loader.ModuleImportFailure)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python3.2/unittest/case.py", line 370, in _executeTestPart
    function()
  File "/usr/lib/python3.2/unittest/loader.py", line 32, in testFailure
    raise exception
ImportError: Failed to import test module: tests.test_javascript
Traceback (most recent call last):
  File "/usr/lib/python3.2/unittest/loader.py", line 256, in _find_tests
    module = self._get_module_from_name(name)
  File "/usr/lib/python3.2/unittest/loader.py", line 234, in _get_module_from_name
    __import__(name)
  File "/home/adi/vcs/pocketlint-branches/1168854-google-closure/pocketlint/tests/test_javascript.py", line 76
    [(1, u'E:0002: Missing space before "+"')],
                                           ^
SyntaxError: invalid syntax

======================================================================
FAIL: test_code_with_warnings (tests.test_python.TestPyflakes)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/adi/vcs/pocketlint-branches/1168854-google-closure/pocketlint/tests/test_python.py", line 140, in test_code_with_warnings
    self.reporter.messages)
AssertionError: Lists differ: [(3, "undefined name 'b'"), (3... != []

First list contains 2 additional elements.
First extra element 0:
(3, "undefined name 'b'")

+ []
- [(3, "undefined name 'b'"),
- (3, "local variable 'a' is assigned to but never used")]

======================================================================
FAIL: test_compile_error_with_line (tests.test_json.TestJSON)
The line number raised by JSON module is used.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/adi/vcs/pocketlint-branches/1168854-google-closure/pocketlint/tests/test_json.py", line 99, in test_compile_error_with_line
    self.reporter.messages)
AssertionError: Lists differ: [(2, 'Expecting property name ... != [(2, 'Expecting property name:...

First differing element 0:
(2, 'Expecting property name enclosed in double quotes: line 2 column 1 (char 2)')
(2, 'Expecting property name: line 2 column 1 (char 2)')

- [(2,
- 'Expecting property name enclosed in double quotes: line 2 column 1 (char 2)')]
? ^ --------------------------

+ [(2, 'Expecting property name: line 2 column 1 (char 2)')]
? ^^^^

----------------------------------------------------------------------
Ran 180 tests in 1.376s

FAILED (failures=2, errors=1)

---------

It would be nice to have this project on travis-ci ... or at least add configuration for tox.

Adding tox suppor should be easy.
Integrating with travis-ci is complicated since it only works with GitHub... and I don't know any web based CI tool with support for Launchpad and BZR.

What do you think?
Thanks!