Fix: decoding error + gemname search pattern with rubygems > 3.0

• Fix decoding error in Python2.

  Use subprocess.check_output with universal_newlines option to prevent decoding errors.

• Store mocked gem dependency calls return in files rather than in a dictionnary

• Fix gems constraints detection failures by extending the regex to also search for symbols < and ~> in the second constraint.

• Fix: make gem_search_pattern compatible with rubygems >= 3.0

  Use search pattern ^gemname$ for rubygems >= 3.0 instead of /^gemname$/

• Extend gem constraints regex to handle version exclusion ( != symbol)

tests/test_data/dependency_activesupport-3.2.0.txt

+Gem activesupport-3.2.0
+  i18n (~> 0.6)
+  multi_json (~> 1.0)
+

tests/test_data/dependency_atomic-0.0.1.txt

+Gem atomic-0.0.1
+

tests/test_data/dependency_backports-3.8.0.txt

+Gem backports-3.8.0
+

tests/test_data/dependency_cucumber-core-3.0.0.txt

+Gem cucumber-core-3.0.0
+  backports (>= 3.8.0)
+  bundler (>= 1.3.5, development)
+  coveralls (~> 0.7, development)
+  cucumber-tag_expressions (>= 1.0.1)
+  gherkin (>= 4.1.3)
+  kramdown (~> 1.4.2, development)
+  rake (>= 0.9.2, development)
+  rspec (~> 3, development)
+  unindent (>= 1.0, development)
+  yard (>= 0, development)
+

tests/test_data/dependency_cucumber-core-7.1.0.txt

+Gem cucumber-core-7.1.0
+  coveralls (>= 0.8.23, ~> 0.8, development)
+  cucumber-gherkin (>= 14.0.1, ~> 14.0)
+  cucumber-messages (>= 12.2.0, ~> 12.2)
+  cucumber-tag-expressions (>= 2.0.4, ~> 2.0)
+  rake (>= 13.0.0, ~> 13.0, development)
+  rspec (>= 3.9.0, ~> 3.9, development)
+  rubocop (>= 0.78.0, ~> 0.78, development)
+  unindent (>= 1.0, ~> 1.0, development)
+

[Léo-Paul Géneau]

/cc @jerome @jm

[Jérome Perrin]

Setting 'RUBYLIB' in env looks to sometimes prevent gem dependency to run remotely eventhough using '-r' option.

this is mysterious :) probably you had RUBYLIB set in environment ? Python also have similar environment variables to tell interpreter where to look for packages, this was causing problems when running buildout at the beginning of slapos, but since long time slapos node remove all similar environment variables.

Anyway, this looks OK to me, but I still don't know much about ruby packaging

[Léo-Paul Géneau]

I ecountered this problem trying to supply gitlab with rubygemsrecipe v0.4.1 . The command run by rubygemsrecipe to check for dependency is $rubypath $gempath dependency -rv $version $gemname. Looking at $gempath provided by gitlab I found out it was a file containing the following lines :

#!/bin/sh
export PATH='/srv/slapgrid/slappart11/srv/runner/software/a12cdf3481c4202ae72cb255d8a6c183/parts/yarn/bin:/srv/slapgrid/slappart11/srv/runner/software/a12cdf3481c4202ae72cb255d8a6c183/parts/ruby2.3/bin[...]:/srv/slapgrid/slappart11/srv/runner/software/a12cdf3481c4202ae72cb255d8a6c183/parts/bundler-4gitlab/bin'
export RUBYLIB=':/srv/slapgrid/slappart11/srv/runner/software/a12cdf3481c4202ae72cb255d8a6c183/parts/bundler-4gitlab/lib:/srv/slapgrid/slappart11/srv/runner/software/a12cdf3481c4202ae72cb255d8a6c183/parts/bundler-4gitlab/lib/ruby:/srv/slapgrid/slappart11/srv/runner/software/a12cdf3481c4202ae72cb255d8a6c183/parts/bundler-4gitlab/lib/site_ruby/1.8'
export GEM_HOME='/srv/slapgrid/slappart11/srv/runner/software/a12cdf3481c4202ae72cb255d8a6c183/parts/bundler-4gitlab/lib/ruby/gems/1.8'
/srv/slapgrid/slappart11/srv/runner/software/a12cdf3481c4202ae72cb255d8a6c183/parts/bundler-4gitlab/bin/gem "$@"

So I know from where comes the RUBYLIB value in this situation but I neither understand while this makes gem dependency -r fail. What troubles me is that we define RUBYLIB in rubygems.py :

    def _get_env(self):
        s = {
            'PATH': os.environ.get('PATH', ''),
            'PREFIX': self.options['location'],
            'RUBYLIB': os.environ.get('RUBYLIB', ''),
        }
        env = {
            'GEM_HOME': '%(PREFIX)s/lib/ruby/gems/1.8' % s,
            'RUBYLIB': self._join_paths(
                '%(RUBYLIB)s',
                '%(PREFIX)s/lib',
                '%(PREFIX)s/lib/ruby',
                '%(PREFIX)s/lib/site_ruby/1.8',
            ) % s,
            'PATH': self._join_paths(
                '%(PATH)s',
                '%(PREFIX)s/bin',
            ) % s,
        }

and it does not prevent gem dependency command to work as expected but gitlab RUBYLIB value does.

[Thomas Gambier]

I just assigned @kirr here as I do not know anything about ruby environment.

[Kirill Smelkov]

I just assigned @kirr here as I do not know anything about ruby environment.

I hadn't been touching ruby for a long time, but if you ask:

@lpgeneau, this patch is big and does several things at once. While some parts are easy to extract, like "Using decode method on subprocess.checkoutput() throws an encoding error in Python2", others are scattered and intermixed. I suggest to split this patch into several ones that each does one logical thing. Splitting into patches should be easy with e.g. starting on a fresh branch and then git checkout -p parts interactively from original-branch of this patch. With every logical change coming as separate patch it should be easier to review and understand them.

• Setting 'RUBYLIB' in env looks to sometimes prevent gem dependency to run remotely eventhough using '-r' option.
  Fix it by removing 'RUBYLIB' from env before checking for depencencies.
  ...
  What troubles me is that we define RUBYLIB in rubygems.py ... and it does not prevent gem dependency command to work as expected but gitlab RUBYLIB value does.

If you ask me I would suggest to put this change separately, and to provide more details in its description: which gem should be taken from remote domain, but is being taken from local domain + other context. Providing full details and explaining things in the description helps both reviewer, and it also sometimes helps to patch author, as e.g. via this way I frequently find mistakes and misthinkings on my side.

[Léo-Paul Géneau]

@kirr

I splitted the commits trying to make this patch more understandable.

About from which remote domain should gem be taken I do not really understand the question. This depends on how the user sets the environnement. So gem dependency should look for dependencies at the specified domain(s). However what I think happened here is that for some values of RUBYLIB, gem dependency checks for dependencies locally (in a directory of installed gems) instead of looking for it remotely.

[Kirill Smelkov]

@lpgeneau thanks for splitting the change into patches. Let me comment on them one by one:

Re patch 1 (https://lab.nexedi.com/lpgeneau/rubygemsrecipe/commit/a3a423bc):

Fix gems constraints detection failures

Fix some gems constraints detection failures by extending the regex

You see, "some" is not very helpful to understand why a change was needed. If instead of that "some" in the description accompanying the change, an example input is provided - with which current version of the software fails, and whose handling is improved by the patch, it becomes more clear what the change does and why it was needed.

If I understand correctly you are adding handling for "<" and "~>" somewhere in gem_regex. I suggest to include an example gemspec input with those constructs that was not previously handled correctly. Also, if we are fixing something, it is almost always a good idea to include corresponding test for the fix into the patch itself. This will make, once again, the patch to be more clear, and it will also make sure that future changes won't break the fix accidentally. The latter point is so important in software engineering, that in general, when you are working on any fix, it is advised to first develop corresponding test, make sure that test fails without the fix, and only then make sure the fix cures that test failure. Please see https://www.erp5.com/documentation/developer/guideline/commit/erp5-Guideline.Commit.Best.Practice#testing for more details on this topic.

---

Re patch 2 (https://lab.nexedi.com/lpgeneau/rubygemsrecipe/commit/b9735746):

Probably ok.

---

Re patch 3 (https://lab.nexedi.com/lpgeneau/rubygemsrecipe/commit/0a476c90):

I think I could understand what is going on in that patch: previously gem dependency output was saved inside the process, and now it is being retrieved via cat command via subprocess from external file instead, with change intent being to actually verify subprocess codepath for non-ASCII decoding, likely to exercise patch 2. There is much code churn in patch 3 for which I did not delved into detail, but I was curious whether tests actually catch those decoding errors. So I tried to kind of revert patch 2 with

--- a/rubygems.py
+++ b/rubygems.py
@@ -59,7 +59,7 @@ def run(self, cmd, environ=None):
             env.update(environ)

         try:
-            return subprocess.check_output(cmd, env=env, universal_newlines=True)
+            return subprocess.check_output(cmd, env=env).decode()
         except OSError as e:
             raise zc.buildout.UserError(
                 'System error, command failed: %s: %s' % (e, cmd))

and see if it breaks with "encoding error in Python2", but instead of UnicodeDecodeError I see failures like

ERROR: test_deployment_rash030_version_dependency_error (tests.test_rubygems.RubyGemsDeploymentTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/kirr/src/wendelin/slapos/rubygemsrecipe/tests/test_rubygems.py", line 37, in wrapper
    func(test, self.tempdir, self.patches, buildout, name, options)
  File "/home/kirr/src/wendelin/slapos/rubygemsrecipe/tests/test_rubygems.py", line 341, in test_deployment_rash030_version_dependency_error
    recipe.install
  File "/usr/lib/python2.7/unittest/case.py", line 994, in assertRaisesRegexp
    callable_obj(*args, **kwargs)
  File "/home/kirr/src/wendelin/slapos/rubygemsrecipe/rubygems.py", line 326, in install
    self._install_rubygems()
  File "/home/kirr/src/wendelin/slapos/rubygemsrecipe/rubygems.py", line 231, in _install_rubygems
    self.run(cmd, env)
  File "/home/kirr/src/wendelin/slapos/rubygemsrecipe/rubygems.py", line 62, in run
    return subprocess.check_output(cmd, env=env).decode()
AttributeError: 'NoneType' object has no attribute 'decode'

If we were fixing check_output wrt decoding, and unmocked that check_output with real call to subprocess('cat'), why check_output returns None instead of output of ran command?

I might be missing something, because I did not looked into details, but at this point I got really confused. Maybe it is just me being "not clever enough" to understand, but on the other hand isn't the fact - that something is not so easy to grasp - speaks for itself and suggests the need for improvement? This way I suggest to present changes in such a way that for a reader(*) it is easy to see what is going on and how without turning on cleverness.

(*) and reader might be you when you look at you change 6 months after it was committed, the context was forgotten and everything has to be refreshed from scratch.

---

Re patch 4 (https://lab.nexedi.com/lpgeneau/rubygemsrecipe/commit/33462eae):

Fix: prevent gem dependency to run locally

Setting 'RUBYLIB' in env looks to sometimes prevent gem dependency to run remotely eventhough using '-r' option.

Fix it by removing 'RUBYLIB' from env before checking for depencencies.

About from which remote domain should gem be taken I do not really understand the question. This depends on how the user sets the environnement. So gem dependency should look for dependencies at the specified domain(s). However what I think happened here is that for some values of RUBYLIB, gem dependency checks for dependencies locally (in a directory of installed gems) instead of looking for it remotely.

I was meaning that we need to see particular example of "how user sets the environnement", "that for some values of RUBYLIB, gem dependency checks for dependencies locally (in a directory of installed gems) instead of looking for it remotely."

And it would be better if that particular example could be turned into a test, for the same reasons as explained for patch 1.

Kirill

[Léo-Paul Géneau]

@kirr

About patch 3, did you also revert the changes of the tests (the check_output_test method) ?

[Léo-Paul Géneau]

@kirr

About patch 4, I found out value RUBYLIB=':/srv/slapgrid/slappart11/srv/runner/software/a12cdf3481c4202ae72cb255d8a6c183/parts/bundler-4gitlab/lib:/srv/slapgrid/slappart11/srv/runner/software/a12cdf3481c4202ae72cb255d8a6c183/parts/bundler-4gitlab/lib/ruby:/srv/slapgrid/slappart11/srv/runner/software/a12cdf3481c4202ae72cb255d8a6c183/parts/bundler-4gitlab/lib/site_ruby/1.8' was causing the mentionned problem but I do not get why this particular value and not another. As long as I do not understand what is going on I do not know how writing a test about it.

[Kirill Smelkov]

As long as I do not understand what is going on I do not know how writing a test about it.

@lpgeneau in such situations it helps to try to minimize the amount of external factors while still reproducing the problem. For example I would suggest to first trim $RUBYLIB until it has the minimum amount of entries (hopefully just one) with which the problem still reproduces. Then, after you have that minimal $RUBYLIB, go into that ruby directories and try to remove things there to the minimum until the problem still reproduces. Going that way takes some time and experimentation, but in the end, after having a minimal reproducer, it is very likely that it will be easier to understand what is going on.

It also helps to use kind of binary search when going through variants.

[Kirill Smelkov]

About patch 3, did you also revert the changes of the tests (the check_output_test method) ?

I tried it both. When I only revert the main code but not check_output_test, that test was expectingly failing in addition to the failures I was talking about.

[Kirill Smelkov]

@lpgeneau, I assume you have done your round of updates and expect me to have a look. If it is not the case and your changes are still in the works, maybe my feedback could still be useful a bit. In the future please explicitly state when you are done with amendments and next turn should be on reviewer side.

The patches

• "Fix gems constraints detection failures" (cf2bccea), and
• "Handle version exclusion constraint" (6eae1bc8)

look good on the first glance: they explain what they are fixing, and they come with corresponding tests. Thanks for doing this rework.

For "Unmock checkoutput method for dependencies check" (2044f0b2) I'm still not sure it is all good: I've tried to run tests with patch1+patch2 (state = 2044f0b2) and for me, even without any kind of manual reverts or other edits, it fails on both py2 and py3 with e.g.

FAIL: test_deployment_more_or_equal_constraint_error (tests.test_rubygems.RubyGemsDeploymentTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/kirr/src/wendelin/slapos/rubygemsrecipe/tests/test_rubygems.py", line 37, in wrapper
    func(test, self.tempdir, self.patches, buildout, name, options)
  File "/home/kirr/src/wendelin/slapos/rubygemsrecipe/tests/test_rubygems.py", line 358, in test_deployment_more_or_equal_constraint_error
    self.assertRaisesRegexp(
AssertionError: UserError not raised by install

Can we please get to the state, where after every patch the tests continue to pass without any breakage? Doing it this way it helps to test every change on its own, it helps to later find out causes for breakages with git bisect, and in general it is just a more clean development style.

For the reference - those test failures go away after patch3 ("Fix gems constraints detection failures") is applied.

But I still see None instead of UnicodeDecodeError if I kind of revert "Fix encoding error in Python2" with:

--- a/rubygems.py
+++ b/rubygems.py
@@ -59,7 +59,7 @@ def run(self, cmd, environ=None):
             env.update(environ)
 
         try:
-            return subprocess.check_output(cmd, env=env, universal_newlines=True)
+            return subprocess.check_output(cmd, env=env).decode()
         except OSError as e:
             raise zc.buildout.UserError(
                 'System error, command failed: %s: %s' % (e, cmd))

, or if I fully revert "Fix encoding error in Python2", but leave RubyGemsTests -> RubyGemsTestCase change that is expected by further patches with:

diff --git a/rubygems.py b/rubygems.py
index ad5c5fb..3924fbc 100644
--- a/rubygems.py
+++ b/rubygems.py
@@ -59,7 +59,7 @@ def run(self, cmd, environ=None):
             env.update(environ)
 
         try:
-            return subprocess.check_output(cmd, env=env, universal_newlines=True)
+            cmd_result = subprocess.check_output(cmd, env=env).decode()
         except OSError as e:
             raise zc.buildout.UserError(
                 'System error, command failed: %s: %s' % (e, cmd))
@@ -75,6 +75,7 @@ def run(self, cmd, environ=None):
                     'System error, command failed with exit code %s: %s' 
                     % (e.returncode, e.cmd)
                 )
+        return cmd_result
 
     def update(self):
         pass
diff --git a/tests/test_rubygems.py b/tests/test_rubygems.py
index 283f4b9..21e0b04 100644
--- a/tests/test_rubygems.py
+++ b/tests/test_rubygems.py
@@ -91,9 +91,17 @@ def check_output_test(self, check_output_mock, expected_arg_list_list):
         self.assertEqual(check_output_mock.call_count,
                           len(expected_arg_list_list))
 
-        for mock_call, expected_arg_list in zip(check_output_mock.mock_calls,
+        for command_nb, expected_arg_list in enumerate(
                                                 expected_arg_list_list):
-            self.assertEqual(mock_call[1][0], expected_arg_list)
+            # half of the mock calls come from the use of 'decode' method
+            self.assertEqual(
+                check_output_mock.mock_calls[2*command_nb][1][0],
+                expected_arg_list
+            )
+            self.assertEqual(
+                str(check_output_mock.mock_calls[2*command_nb+1]),
+                'call().decode()'
+            )
 
     def install_with_default_rubygems_test(self, path, patches,
                                             expected_install_arg_list_list):

The failures show themselves as e.g.

ERROR: test_deployment_subdependency_constraint_error (tests.test_rubygems.RubyGemsDeploymentTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/kirr/src/wendelin/slapos/rubygemsrecipe/tests/test_rubygems.py", line 37, in wrapper
    func(test, self.tempdir, self.patches, buildout, name, options)
  File "/home/kirr/src/wendelin/slapos/rubygemsrecipe/tests/test_rubygems.py", line 438, in test_deployment_subdependency_constraint_error
    recipe.install
  File "/usr/lib/python2.7/unittest/case.py", line 994, in assertRaisesRegexp
    callable_obj(*args, **kwargs)
  File "/home/kirr/src/wendelin/slapos/rubygemsrecipe/rubygems.py", line 327, in install
    self._install_rubygems()
  File "/home/kirr/src/wendelin/slapos/rubygemsrecipe/rubygems.py", line 232, in _install_rubygems
    self.run(cmd, env)
  File "/home/kirr/src/wendelin/slapos/rubygemsrecipe/rubygems.py", line 62, in run
    cmd_result = subprocess.check_output(cmd, env=env).decode()
AttributeError: 'NoneType' object has no attribute 'decode'

Kirill

[Léo-Paul Géneau]

I still have to investigate about 'gem dependency' searching for gems locally and write a test about it but the other commits should be fine now.

[Léo-Paul Géneau]

@kirr I think everything is fine for review now.

[Kirill Smelkov]

Hello, @lpgeneau.

I still have to investigate about 'gem dependency' searching for gems locally and write a test about it but the other commits should be fine now.

The first patch indeed looks ok now - it passes the tests, and the tests reproduce UnicodeDecodeError if I change the main code back to how it was before. So I've applied this patch to master: 96ddcde9.

Checking the other patches had less luck: I've moved on to verifying the second one (b6046988) and tried to run the tests, and, unfortunately, as in !6 (comment 137549) the tests still don't pass:

(neo) (z-dev) (g.env) kirr@deca:~/src/wendelin/slapos/rubygemsrecipe$ python setup.py test
running test
WARNING: Testing via this command is deprecated and will be removed in a future version. Users looking for a generic test entry point independent of test runner are encouraged to use tox.
running egg_info
writing requirements to rubygemsrecipe.egg-info/requires.txt
writing rubygemsrecipe.egg-info/PKG-INFO
writing top-level names to rubygemsrecipe.egg-info/top_level.txt
writing dependency_links to rubygemsrecipe.egg-info/dependency_links.txt
writing entry points to rubygemsrecipe.egg-info/entry_points.txt
reading manifest file 'rubygemsrecipe.egg-info/SOURCES.txt'
reading manifest template 'MANIFEST.in'
writing manifest file 'rubygemsrecipe.egg-info/SOURCES.txt'
running build_ext
test_executables (tests.test_rubygems.RubyGemsDefaultTestCase) ... ok
test_invalid_environment (tests.test_rubygems.RubyGemsDefaultTestCase) ... ok
test_missing_gems (tests.test_rubygems.RubyGemsDefaultTestCase) ... ok
test_mkdir_error (tests.test_rubygems.RubyGemsDefaultTestCase) ... ok
test_no_version (tests.test_rubygems.RubyGemsDefaultTestCase) ... ok
test_non_ascii_encoding (tests.test_rubygems.RubyGemsDefaultTestCase) ... ok
test_non_zero_exitcode (tests.test_rubygems.RubyGemsDefaultTestCase) ... No handlers could be found for logger "rubygems"
ok
test_oserror (tests.test_rubygems.RubyGemsDefaultTestCase) ... ok
test_pinned_versions (tests.test_rubygems.RubyGemsDefaultTestCase) ... ok
test_signal_received (tests.test_rubygems.RubyGemsDefaultTestCase) ... ok
test_success (tests.test_rubygems.RubyGemsDefaultTestCase) ... ok
test_update (tests.test_rubygems.RubyGemsDefaultTestCase) ... ok
test_version (tests.test_rubygems.RubyGemsDefaultTestCase) ... ok
test_version_from_url (tests.test_rubygems.RubyGemsDefaultTestCase) ... ok
test_deployment_less_constraint_error (tests.test_rubygems.RubyGemsDeploymentTestCase) ... ok
test_deployment_less_constraint_success (tests.test_rubygems.RubyGemsDeploymentTestCase) ... ok
test_deployment_more_or_equal_constraint_error (tests.test_rubygems.RubyGemsDeploymentTestCase) ... FAIL
test_deployment_more_or_equal_constraint_success (tests.test_rubygems.RubyGemsDeploymentTestCase) ... ok
test_deployment_not_pinned_dependency_error (tests.test_rubygems.RubyGemsDeploymentTestCase) ... ok
test_deployment_not_pinned_subdependency_error (tests.test_rubygems.RubyGemsDeploymentTestCase) ... FAIL
test_deployment_not_pinned_version_error (tests.test_rubygems.RubyGemsDeploymentTestCase) ... ok
test_deployment_pinned_subdependency_success (tests.test_rubygems.RubyGemsDeploymentTestCase) ... ok
test_deployment_similar_constraint_error (tests.test_rubygems.RubyGemsDeploymentTestCase) ... ok
test_deployment_similar_constraint_success (tests.test_rubygems.RubyGemsDeploymentTestCase) ... ok
test_deployment_subdependency_constraint_error (tests.test_rubygems.RubyGemsDeploymentTestCase) ... FAIL
test_deployment_subdependency_constraint_success (tests.test_rubygems.RubyGemsDeploymentTestCase) ... ok

======================================================================
FAIL: test_deployment_more_or_equal_constraint_error (tests.test_rubygems.RubyGemsDeploymentTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/kirr/src/wendelin/slapos/rubygemsrecipe/tests/test_rubygems.py", line 37, in wrapper
    func(test, self.tempdir, self.patches, buildout, name, options)
  File "/home/kirr/src/wendelin/slapos/rubygemsrecipe/tests/test_rubygems.py", line 367, in test_deployment_more_or_equal_constraint_error
    recipe.install
AssertionError: UserError not raised

======================================================================
FAIL: test_deployment_not_pinned_subdependency_error (tests.test_rubygems.RubyGemsDeploymentTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/kirr/src/wendelin/slapos/rubygemsrecipe/tests/test_rubygems.py", line 37, in wrapper
    func(test, self.tempdir, self.patches, buildout, name, options)
  File "/home/kirr/src/wendelin/slapos/rubygemsrecipe/tests/test_rubygems.py", line 407, in test_deployment_not_pinned_subdependency_error
    recipe.install
AssertionError: UserError not raised

======================================================================
FAIL: test_deployment_subdependency_constraint_error (tests.test_rubygems.RubyGemsDeploymentTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/kirr/src/wendelin/slapos/rubygemsrecipe/tests/test_rubygems.py", line 37, in wrapper
    func(test, self.tempdir, self.patches, buildout, name, options)
  File "/home/kirr/src/wendelin/slapos/rubygemsrecipe/tests/test_rubygems.py", line 434, in test_deployment_subdependency_constraint_error
    recipe.install
AssertionError: UserError not raised

----------------------------------------------------------------------
Ran 26 tests in 0.060s

FAILED (failures=3)
Test failed: <unittest.runner.TextTestResult run=26 errors=0 failures=3>
error: Test failed: <unittest.runner.TextTestResult run=26 errors=0 failures=3>

[Léo-Paul Géneau]

I mixed up changes from patch 2 and patch 3 but it is ok now, tests passed for each commit on my side.

[Kirill Smelkov]

Thanks, @lpgeneau. I've applied all patches to master.

[Kirill Smelkov]

( please feel free to reopen if I missed something )