Pull requests

#82 Declined
Repository
godfryd
Branch
jobs
Repository
logilab
Branch
default

added support for checking files in parallel using multiprocessing

Author
  1. Michal Nowikowski
Reviewers
Description

This a patch that introduces parallel jobs made by dahlajn (pull request #55) and then improved according to review comments.

  • Learn about pull requests

Comments (30)

  1. Claudiu Popa

    Another problem is that this feature doesn't work in Windows yet (probably due to differences between Unix and Windows regarding process creation). There's a lot of pickling errors which appears when passing -j to pylint.

    1. Claudiu Popa

      Hi, Michal.

      I don't have that much time right now, but if I don't get back in one week, please ping me. This will be part of 1.4, though, which will be launched soon.

    1. Michal Nowikowski author

      It looks that the problem is connected with Python 2.7 and its multiprocessing bug.

      See: http://bugs.python.org/issue10845

      When I run the command (using -m unittest as in tox.ini) then it fails:

      (pyve) C:\Users\gof\pylint>python -Wi -m unittest discover -s test -p test_self.py
      .......Traceback (most recent call last):
        File "<string>", line 1, in <module>
        File "c:\Python27\Lib\multiprocessing\forking.py", line 380, in main
          prepare(preparation_data)
        File "c:\Python27\Lib\multiprocessing\forking.py", line 488, in prepare
          assert main_name not in sys.modules, main_name
      AssertionError: __main__
      E..
      ======================================================================
      ERROR: test_parallel_execution (test_self.RunTC)
      ----------------------------------------------------------------------
      Traceback (most recent call last):
        File "C:\Users\gof\pylint\test\test_self.py", line 131, in test_parallel_execution
          self._runtest(['-j 2', 'pylint.lint', 'pylint.utils'], code=29)
        File "C:\Users\gof\pylint\test\test_self.py", line 66, in _runtest
          Run(args, reporter=reporter)
        File "C:\Users\gof\pylint\lint.py", line 1177, in __init__
          linter.check(args)
        File "C:\Users\gof\pylint\lint.py", line 636, in check
          self._parallel_check(files_or_modules)
        File "C:\Users\gof\pylint\lint.py", line 643, in _parallel_check
          manager = multiprocessing.Manager()  # pylint: disable=no-member
        File "c:\Python27\Lib\multiprocessing\__init__.py", line 99, in Manager
          m.start()
        File "c:\Python27\Lib\multiprocessing\managers.py", line 528, in start
          self._address = reader.recv()
      EOFError
      
      ----------------------------------------------------------------------
      Ran 10 tests in 12.651s
      
      FAILED (errors=1)
      

      When I run it using nose it passes:

      (pyve) C:\Users\gof\pylint>..\pyve\Scripts\nosetests.exe test\test_self.py
      ..........
      ----------------------------------------------------------------------
      Ran 10 tests in 35.777s
      
      OK
      

      The same with py.test:

      (pyve) C:\Users\gof\pylint>..\pyve\Scripts\py.test.exe test\test_self.py
      ============================= test session starts =============================
      platform win32 -- Python 2.7.8 -- py-1.4.24 -- pytest-2.6.2
      collected 10 items
      
      test/test_self.py ..........
      
      ========================= 10 passed in 42.90 seconds ==========================
      

      I do not know how to work-around -m unittest. I would switch from -m unittest to py.test. It is much more robust than plain -m unittest and a bit better than nosetests.

        1. Claudiu Popa

          Hey, Michal, sorry for the delay.

          I'm having other problems, using Python 3.4, but yours should be fixed as well.

          First of all, test_self.test_all fails with:

          FAIL: test_all (test_self.RunTC)
          Make pylint check itself.
          ----------------------------------------------------------------------
          Traceback (most recent call last)
            File "C:\Python34\lib\unittest\case.py", line 58, in testPartExecutor
              yield
            File "C:\Python34\lib\unittest\case.py", line 577, in run
              testMethod()
            File "C:\Python34\Lib\site-packages\pylint-1.3.0-py3.4.egg\pylint\test\test_self.py", line 96, in test_all
              self._runtest(['pylint.lint'], reporter=MultiReporter(reporters))
            File "C:\Python34\Lib\site-packages\pylint-1.3.0-py3.4.egg\pylint\test\test_self.py", line 77, in _runtest
              self.assertEqual(ex.code, code, msg)
            File "C:\Python34\lib\unittest\case.py", line 797, in assertEqual
              assertion_func(first, second, msg=msg)
            File "C:\Python34\lib\unittest\case.py", line 790, in _baseAssertEqual
              raise self.failureException(msg)
          AssertionError: 30 != 28 : expected output status 28, got 30. Below pylint output: 
          

          Second, test_all.test_parallel_execution blocks forever and must be stopped after some time. I'll try to investigate if I can find some time.

          1. Michal Nowikowski author

            This 30 != 28 is the same problem as you mentioned at the beginning of this thread i.e. it is connected with -m unittest. The second case with blocking I didn't encounter. How can I reproduce it?

            1. Claudiu Popa

              This 30 != 28 problem seems to be related to something else, I'm running the test manually and I see the same error. Probably your patch adds a couple of warnings, which results in a different status code. Regarding the second problem, I don't know how to reproduce it, I tested this on a Windows 8 machine.

      1. Claudiu Popa

        Sorry for the spam. :-) So, running test_self.py individually doesn't block on Python 3, only with -m discover. Regarding a new test runner, I don't like py.test, it's too magical and I'm not sure if it knows how to handle unittest style assertions, please correct me if I am wrong.

        1. Michal Nowikowski author

          Hello,

          I was looking into these problems. One problem is with test test_all and also with my test_parallel_execution in test_self.py. Both of them check pylint.lint module. And while code of this file changes the returned exitcode from pylint also may change. In our case it changed from expected 28 to 30. So this problem should also be present on default branch (but I didn't check it yet). I would change the tested file to something more stable than pylint.lint.

          Still investigating problem with blocking.

          1. Michal Nowikowski author

            Hello,

            I was trying to fix the problem with hung in Py3.4 on Windows. I see that this is a problem with new Message class and its pickling, and maybe with getnewargs method. Unfortunately it happens only from time to time what is very strange. Now I added a check if problem occurs and handle it gracefully so the program does not hang as before.

            1. Michal Nowikowski author

              This is some nightmare. I wanted to debug pickle so I switched to its pure python implementation because by default native one is used. And then this problem does not appear. So it looks that it may be some bug in pickle native implementation. What is so discouraging is that this problem appears only in tests. When parallel check is run from commandline then all is also working :/

  2. Michal Nowikowski author

    OK. I made the communication between processes more robust so that the Pylint does not hung if there are any problems.

    Regarding problems on Windows, there are two of them. One is connected with bug in multiprocessing on Py27, the second one is connected with pickling and multiprocessing on Py3x. Both of them are caused by the way of running tests (not the test and not the problems in jobs code). The first one does not appear if tests are run by nosetests or py.tests (this is due to this bug nature, more here: http://bugs.python.org/issue10845). The second one is connected with some problems with native version of pickle library - if I use pure python version the problem does not appear.

    So I do not have idea how to make this test working on windows. What is important here is that the test is proper and the implementation of jobs in Pylint is ok and works. So to not fail testing process I added skipping this test on windows (I added proper comment to code in this test).

    So I would like to ask you to complete the review of tihs pull-request.

    1. Claudiu Popa

      Thank you for your extraordinary work, Michal. I have some workshops going on this weekend, so I won't be able to finish the review this weekend, but I'll try to finalize your pull request at the start of the week.

  3. Claudiu Popa

    Hey, Michal. Sorry for taking that long. I finally managed to make the tests work on Windows. I'll add a couple of comments where you should apply the new modifications.