Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tests for 2.3.1 fail with multidict 3.2.0 and 3.3.0 #2365

Closed
ppaeps opened this issue Oct 21, 2017 · 10 comments
Closed

Tests for 2.3.1 fail with multidict 3.2.0 and 3.3.0 #2365

ppaeps opened this issue Oct 21, 2017 · 10 comments
Labels
invalid This doesn't seem right outdated

Comments

@ppaeps
Copy link

ppaeps commented Oct 21, 2017

Long story short

Running the tests for aiohttp 2.3.1 with multidict 3.2.0 or 3.3.0 causes five tests to fail with CIMultiDict AssertionErrors. The easiest workaround is probably to pin multidict to 3.0.0 in setup.py, but the tests should probably be fixed.

Expected behaviour

The tests should not fail. They do not fail with multidict 3.0.0.

Actual behaviour

5 failed, 1888 passed, 36 skipped, 5 xfailed, 77 error in 46.54 seconds

e.g.:

create_session = <function create_session.<locals>.maker at 0x808d28598>

    def test_merge_headers(create_session):
            # Check incoming simple dict
        session = create_session(headers={"h1": "header1",
                                          "h2": "header2"})
        headers = session._prepare_headers({"h1": "h1"})

        assert isinstance(headers, CIMultiDict)
>       assert headers == {"h1": "h1", "h2": "header2"}
E       AssertionError: assert <CIMultiDict(...': 'header2')> == {'h1': 'h1', 'h2': 'header2'}
E         Use -v to get the full diff

tests/test_client_session.py:137: AssertionError

The other failures are variants on this theme.

Steps to reproduce

Run the tests for aiohttp 2.3.1 with multidict 3.0.0, 3.2.0 and 3.3.0. They will fail on 3.2.0 and 3.3.0.

Your environment

FreeBSD 10amd64, 11amd64 and 12amd64.

@asvetlov
Copy link
Member

Just re-checked.
All multidict 3.0.0, 3.1.0, 3.2.0 and 3.3.0 works fine with aiohttp 2.3.1, all tests passed.
Moreover aiohttp 2.3.1 was tested with multidict 3.3.0 on auto deploying.

@asvetlov asvetlov added the invalid This doesn't seem right label Oct 21, 2017
@ppaeps
Copy link
Author

ppaeps commented Oct 21, 2017

In that case, I'd appreciate some help debugging - or at least understanding the problem!

All of these tests are with aiohttp 2.3.1.

With multidict 3.0.0:

1970 passed, 36 skipped, 5 xfailed in 47.46 seconds

With multidict 3.2.0:

5 failed, 1965 passed, 36 skipped, 5 xfailed in 45.74 seconds

With multidict 3.3.0:

5 failed, 1965 passed, 36 skipped, 5 xfailed in 46.92 seconds

Running the tests with -v, the full diffs of the failed test cases are always the same:

create_session = <function create_session.<locals>.maker at 0x809388598>

    def test_merge_headers(create_session):
            # Check incoming simple dict
        session = create_session(headers={"h1": "header1",
                                          "h2": "header2"})
        headers = session._prepare_headers({"h1": "h1"})

        assert isinstance(headers, CIMultiDict)
>       assert headers == {"h1": "h1", "h2": "header2"}
E       AssertionError: assert <CIMultiDict(...': 'header2')> == {'h1': 'h1', 'h2': 'header2'}
E         Full diff:
E         - <CIMultiDict('H1': 'h1', 'h2': 'header2')>
E         + {'h1': 'h1', 'h2': 'header2'}

tests/test_client_session.py:137: AssertionError

In other words: the output changes to <CIMultiDict(...)> instead of {...}. Do you have any idea where that could come from?

I'm happy to believe that this is a problem on my system. Any suggestions on where I should look for the cause? Should I file this issue against multidict? Can GitHub issues be moved to another project?

Thanks for your help.

@asvetlov asvetlov reopened this Oct 21, 2017
@asvetlov
Copy link
Member

CIMultiDict has comparison operator that accepts a regular dict, so everything should be ok.

What is your environment? Do you use Cythonized multidict or pure Python?
Do you run test suite with xdist plugin?
In general what are your steps to reproduce the issue?
Rougly I do:

git clone [email protected]:aio-libs/aiohttp.git
mkvirtualenv -p python3.6 aiohttp
cd aiohttp
make test

@ppaeps
Copy link
Author

ppaeps commented Oct 21, 2017

All of the tests I've quoted are on FreeBSD 11.1 on amd64 and I've run the tests with Poudriere as follows:

[set version of www/py-multidict to 3.0.0, 3.2.0, 3.3.0, then:]
# poudriere testport -i -j 11amd64 -p default www/py-aiohttp
# cd /usr/ports/www/py-aiohttp && make test

Poudriere fires up a jail with aiohttp and its dependencies installed. The port make test command does:

python3.6 -c  "import sys; import setuptools;  __file__='setup.py'; sys.argv[0]='setup.py';  exec(compile(open(__file__, 'rb').read().replace(b'\\r\\n', b'\\n'), __file__, 'exec'))" test

I've confirmed that the Poudriere environment is not likely to be the culprit by running the tests directly with python3.6 setup.py test and with python3.6 -m pytest tests in the aiohttp directory: the failures are identical in each case.

There are no local patches against either aiohttp or multidict (i.e.: they're the tarballs of the respective releases downloaded from GitHub).

The versions of Python packages I have installed are:

root@11amd64-default:/usr/ports/www/py-aiohttp # pkg info | grep ^py36
py36-Babel-2.3.4               Collection of tools for internationalizing Python applications
py36-Jinja2-2.9.5              Fast and easy to use stand-alone template engine
py36-MarkupSafe-1.0            Implements XML/HTML/XHTML Markup safe string for Python
py36-aiohttp-2.3.1             HTTP client/server for asyncio
py36-alabaster-0.7.6           Modified Kr Sphinx theme
py36-async_timeout-1.2.1       Timeout context manager for asyncio programs
py36-chardet-3.0.4             Universal encoding detector for Python 2 and 3
py36-docutils-0.14             Python Documentation Utilities
py36-gunicorn-19.4.5           WSGI HTTP Server for UNIX
py36-imagesize-0.7.1           Python image size library
py36-mock-2.0.0                Mock unit tests for Python
py36-multidict-3.3.0           Multidict implementation
py36-pbr-3.1.1                 Python Build Reasonableness
py36-pip-9.0.1                 Tool for installing and managing Python packages
py36-py-1.4.34                 Library with cross-python path, ini-parsing, io, code, log facilities
py36-pygments-2.2.0            Syntax highlighter written in Python
py36-pystemmer-1.3.0_1         Snowball Stemming Algorithms for Information Retrieval
py36-pytest-3.2.3              Simple powerful testing with Python
py36-pytest-mock-1.6.3         Thin wrapper around the mock package for easier use with py.test
py36-pytest-timeout-1.2.0      Pytest plugin to abort hanging tests
py36-pytz-2017.2,1             World Timezone Definitions for Python
py36-setproctitle-1.1.10       Python module to customize the process title
py36-setuptools-36.5.0         Python packages installer
py36-setuptools_scm-1.15.5     Setuptools plugin to manage your versions by scm tags
py36-six-1.11.0                Python 2 and 3 compatibility utilities
py36-snowballstemmer-1.2.0_1   Snowball stemming library collection for Python
py36-sphinx-1.4.8_2,1          Python documentation generator
py36-sphinx_rtd_theme-0.2.4    Mobile-friendly py-sphinx theme
py36-yarl-0.12.0               Yet another URL library

I confirmed that the only difference between runs is the version of multidict by diffing the output of pkg info as above:

--- multidict-300.txt	2017-10-21 14:26:20.000000000 +0200
+++ multidict-330.txt	2017-10-21 14:20:27.000000000 +0200
@@ -9,7 +9,7 @@
 py36-gunicorn-19.4.5           WSGI HTTP Server for UNIX
 py36-imagesize-0.7.1           Python image size library
 py36-mock-2.0.0                Mock unit tests for Python
-py36-multidict-3.0.0           Multidict implementation
+py36-multidict-3.3.0           Multidict implementation
 py36-pbr-3.1.1                 Python Build Reasonableness
 py36-pip-9.0.1                 Tool for installing and managing Python packages
 py36-py-1.4.34                 Library with cross-python path, ini-parsing, io, code, log facilities

Both multidict and aiohttp are built with cython and I don't use xdist. I will run my tests again without cython and report back. Thanks for the suggestion. I had not thought of cython. I can try with xdist as well, but that might be more work. The plugins used are plugins: timeout-1.2.0, mock-1.6.3.

Thanks again for your help.

@ppaeps
Copy link
Author

ppaeps commented Oct 21, 2017

Just tried without cython: no change unfortunately.

I suspect a subtle interaction between different versions of dependencies higher up, but I don't see any obvious suspects.

I'll try with a git clone and virtualenv/pip and compare versions.

Unless you have an "Aha! That might be it!" idea from the data above. :)

@asvetlov
Copy link
Member

I remember failed tests like you mentioned in migration from multidict 2.x to 3.0 but they was fixed by aiohttp 2.3.0

@ppaeps
Copy link
Author

ppaeps commented Oct 21, 2017

Yes. I saw those too. That's why I was surprised to see them again in multidict 3.2.0/3.3.0.

Do you remember (or is there a record of) the specific tests that were failing in the multidict 2.x -> 3.0 transition?

I'm not having any luck figuring out why they're failing on me. I'll keep digging!

@asvetlov
Copy link
Member

The issue for transition is #1994
Fix is #2000

@ppaeps
Copy link
Author

ppaeps commented Oct 21, 2017

Thanks for the pointers. I'll read through those and see if I can find any inspiration.

@asvetlov asvetlov closed this as completed Nov 3, 2017
@lock
Copy link

lock bot commented Oct 28, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a [new issue] for related bugs.
If you feel like there's important points made in this discussion, please include those exceprts into that [new issue].
[new issue]: https://github.com/aio-libs/aiohttp/issues/new

@lock lock bot added the outdated label Oct 28, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
invalid This doesn't seem right outdated
Projects
None yet
Development

No branches or pull requests

2 participants