pylint-bot / astroid-unofficial

UNOFFICIAL playground for astroid github migration
GNU Lesser General Public License v2.1
0 stars 0 forks source link

test_namedtuple_advanced_inference fails on Python 3 because urlparse no longer exists #233

Closed pylint-bot closed 8 years ago

pylint-bot commented 9 years ago

Originally reported by: BitBucket: ceridwenv, GitHub: ceridwenv


The urlparse module in 2 was consolidated into the urllib module on Python 3. I tried replacing urlparse with six.moves.urllib.urlparse.

result = test_utils.extract_node("""
import six

result = __(six.moves.urllib.parse.urlparse('gopher://'))
""")

This seems to work for now, but it's not exactly a clean test case, we should consider replacing it.


pylint-bot commented 9 years ago

Original comment by Claudiu Popa (BitBucket: PCManticore, GitHub: PCManticore):


Why should it be replaced? If it works with this change, that's great, but we shouldn't change old regression tests, unless it's absolutely necessary.

pylint-bot commented 9 years ago

Original comment by BitBucket: ceridwenv, GitHub: ceridwenv:


According to the comments, this is for testing the case where a class inherits from both a namedtuple and another class. However, that's sensitive to the underlying implementation of urllib, so it's not good for testing namedtuple. We should manufacture a test that explicitly tests namedtuple multiple inheritance. If we're worried about urllib breaking, we should move this test somewhere else, maybe to a collection of tests explicitly for testing the stdlib? However, this has been broken ever since Python 3.0 and as far as I know no one's complained about it, which makes me think it's not an issue...

pylint-bot commented 9 years ago

Original comment by Claudiu Popa (BitBucket: PCManticore, GitHub: PCManticore):


It's mostly about testing that we don't broke urllib and it makes sense to be moved somewhere else. Also, the test was disabled on Python 3, so it's not a surprise that it was broken on Python 3.0.

pylint-bot commented 9 years ago

Original comment by BitBucket: ceridwenv, GitHub: ceridwenv:


Changed in b68ee1186e5f. This replaced urlparse with the appropriate six.moves module, but still depends on urllib's implementation.