Page MenuHomePhabricator

Appveyor builds failing to build requests[security]
Closed, ResolvedPublic

Description

The Appveyor builds are failing to build the dependencies needed for requests[security].

https://ci.appveyor.com/project/wikimedia/pywikibot-core/build/1.0.108

Event Timeline

jayvdb raised the priority of this task from to Needs Triage.
jayvdb updated the task description. (Show Details)
jayvdb subscribed.
jayvdb renamed this task from Appveyor builds failing to Appveyor builds failing to build requests[security].Jul 22 2015, 12:17 PM
jayvdb set Security to None.
WARNING: The pip version pre-installed in the Python folders pre-installed by AppVeyor cannot be modified.

Reference: https://github.com/ogrisel/python-appveyor-demo/commit/fcac7b92dbd47999ca4461e77449c7640626d3b0

Change 227239 had a related patch set uploaded (by VcamX):
[FIX] Failed Appveyor builds

https://gerrit.wikimedia.org/r/227239

WARNING: The pip version pre-installed in the Python folders pre-installed by AppVeyor cannot be modified.

Reference: https://github.com/ogrisel/python-appveyor-demo/commit/fcac7b92dbd47999ca4461e77449c7640626d3b0

Yea, that was a failed quick fix for

https://github.com/ogrisel/python-appveyor-demo/issues/15

Is the version of pip problematic?

Yes. Pip supports environment markers since 6.0 but the pre-installed version in some tests is <6.0.
Reference: https://pip.pypa.io/en/latest/news.html

What happened on Python 2.7.0 is so weird.
https://github.com/wikimedia/pywikibot-core/blob/master/tests/namespace_tests.py#L222

self.assertEqual(Namespace.resolve([6]), [file_ns])

The first argument is what Namespace.resolve([6]) returns, that's a list only containing a Namespace object. The second argument is a list only containing file_ns, that's also a Namespace object.
These two object should be equal:
__eq__ of Namespace: https://github.com/wikimedia/pywikibot-core/blob/master/pywikibot/site.py#L332
[file_ns]: https://ci.appveyor.com/project/VcamX/pywikibot-core/build/1.0.96#L4553
Namespace.resolve([6]): https://ci.appveyor.com/project/VcamX/pywikibot-core/build/1.0.94#L4516

If we just test self.assertEqual(Namespace.resolve([6])[0], file_ns), that will be fine.
If we wrap them inside a list, they're not equal.

@jayvdb the problem of pip has been fixed by using virtualenv, only test on python 2.7.0 failed.
https://ci.appveyor.com/project/VcamX/pywikibot-core/build/1.0.116

What happened on Python 2.7.0 is so weird.
https://github.com/wikimedia/pywikibot-core/blob/master/tests/namespace_tests.py#L222

self.assertEqual(Namespace.resolve([6]), [file_ns])

The first argument is what Namespace.resolve([6]) returns, that's a list only containing a Namespace object. The second argument is a list only containing file_ns, that's also a Namespace object.
These two object should be equal:
__eq__ of Namespace: https://github.com/wikimedia/pywikibot-core/blob/master/pywikibot/site.py#L332
[file_ns]: https://ci.appveyor.com/project/VcamX/pywikibot-core/build/1.0.96#L4553
Namespace.resolve([6]): https://ci.appveyor.com/project/VcamX/pywikibot-core/build/1.0.94#L4516

If we just test self.assertEqual(Namespace.resolve([6])[0], file_ns), that will be fine.
If we wrap them inside a list, they're not equal.

@jayvdb, @XZise, I think it's about TestCase of Python 2.7.0 and CapturingTestCase of Pywikibot:

Related methods of TestCase in unittest lib of Python 2.7.0:

__init__: https://github.com/python/cpython/blob/6d9a901fd68d467a5d91889b7a11e3a29a134068/Lib/unittest/case.py#L173

def __init__(self, methodName='runTest'):
    ......
    
    self._type_equality_funcs = {}
    self.addTypeEqualityFunc(dict, self.assertDictEqual)
    self.addTypeEqualityFunc(list, self.assertListEqual)
    self.addTypeEqualityFunc(tuple, self.assertTupleEqual)
    self.addTypeEqualityFunc(set, self.assertSetEqual)
    self.addTypeEqualityFunc(frozenset, self.assertSetEqual)
    self.addTypeEqualityFunc(unicode, self.assertMultiLineEqual)

_getAssertEqualityFunc: https://github.com/python/cpython/blob/6d9a901fd68d467a5d91889b7a11e3a29a134068/Lib/unittest/case.py#L458

def _getAssertEqualityFunc(self, first, second):
    if type(first) is type(second):
        asserter = self._type_equality_funcs.get(type(first))
        if asserter is not None:
            return asserter
    return self._baseAssertEqual

assertEqual: https://github.com/python/cpython/blob/6d9a901fd68d467a5d91889b7a11e3a29a134068/Lib/unittest/case.py#L489

def assertEqual(self, first, second, msg=None):
    assertion_func = self._getAssertEqualityFunc(first, second)
    assertion_func(first, second, msg=msg)

NOTE: some self.assertXXXEqual methods are referenced without called in __init__.

Related methods of`CapturingTestCase` in tests.aspects of Pywikibot:

patch_assert: https://github.com/wikimedia/pywikibot-core/blob/master/tests/aspects.py#L986

def patch_assert(self, assertion):
    """Execute process_assert when the assertion is called."""
    def inner_assert(*args, **kwargs):
        assert self._patched is False
        self._patched = True
        try:
            self.process_assert(assertion, *args, **kwargs)
        finally:
            self._patched = False
    return inner_assert

__getattribute__ of CapturingTestCase: https://github.com/wikimedia/pywikibot-core/blob/master/tests/aspects.py#L997:

def __getattribute__(self, attr):
    """Patch assertions if enabled."""
    result = super(CapturingTestCase, self).__getattribute__(attr)
    if attr.startswith('assert') and not self._patched:
        return self.patch_assert(result)
    else:
        return result

__getattribute__ will check the name of attribute and filter the prefix of assert if it's not patched, then patch it. That's the problem.

Remember that in __init__ of TestCase, some self.assertXXXEqual methods are referenced without called. In __init__, self._type_equality_funcs is a dict mapping from type to type-related method which is used to get the corresponding assert method for type. Because of the custom __getattribute__, that causes self._type_equality_funcs store the patched methods instead of the right ones.

When we call assertEqual, we get the patched one, which set self._patch to True. Then the original assertEqual will get the patched assertListEqual, which will check self._patch first before executing the main body. So the assertion will fail.

@VcamX, can you run a build with 2.7.1, 2.7.2, etc to find out which versions this bug appears in.

@jayvdb Only 2.7.0 2.7.0, 2.7.1, 2.7.2
The version (0.8.0) of unittest2 we specify for python 2.6 will work well. So I'd like to use unittest2 0.8.0.

As we plan to drop support for 2.7.0 and 2.7.1 anyway I don't think this is a large problem: https://gerrit.wikimedia.org/r/#/c/218884/

But thanks for that analysis. I wouldn't have guessed we could get that type of problem so hopefully if we get in the future an error somewhere else similar to this we might already know what is wrong. If we want to fix it my suggestion would be to just call the normal assertion if the patched flag is already True.

Change 227239 merged by jenkins-bot:
[FIX] Failed Appveyor builds

https://gerrit.wikimedia.org/r/227239