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

test_media_types_method - Test failure #547

Closed
darless opened this issue Jun 27, 2023 · 5 comments · Fixed by #552
Closed

test_media_types_method - Test failure #547

darless opened this issue Jun 27, 2023 · 5 comments · Fixed by #552
Labels
bug Something isn't working

Comments

@darless
Copy link

darless commented Jun 27, 2023

Latest github actions run with failure:
https://github.com/python-restx/flask-restx/actions/runs/5385063797/jobs/9773681086

Failure seen

self = <test_api_legacy.APITest object at 0x7f26f63ece50>, app = <Flask 'tests.conftest'>, mocker = <pytest_mock.plugin.MockerFixture object at 0x7f26f5f815b0>

    def test_media_types_method(self, app, mocker):
        api = restx.Api(app)
    
        with app.test_request_context(
            "/foo", headers={"Accept": "application/xml; q=.5"}
        ):
>           assert api.mediatypes_method()(mocker.Mock()) == [
                "application/xml",
                "application/json",
            ]
E           AssertionError: assert ['application/json'] == ['application...ication/json']
E             At index 0 diff: 'application/json' != 'application/xml'
E             Right contains one more item: 'application/json'
E             Use -v to get the full diff

tests/legacy/test_api_legacy.py:113: AssertionError

Reproduce

Looks like all python versions are failing in the same way.

python3 -m venv venv
. ./venv/bin/activate
pip3 install -r requirements/dev.txt
tox

Expected Behavior

Tests expects to see mediatypes as both application/json and application/xml

Actual Behavior

Test only sees application/json

Environment

$ pip3 list
Package           Version
----------------- -------
black             23.3.0 
cachetools        5.3.1  
chardet           5.1.0  
click             8.1.3  
colorama          0.4.6  
distlib           0.3.6  
filelock          3.12.2 
mypy-extensions   1.0.0  
packaging         23.1   
pathspec          0.11.1 
pip               20.0.2 
pkg-resources     0.0.0  
platformdirs      3.8.0  
pluggy            1.2.0  
pyproject-api     1.5.2  
setuptools        44.0.0 
tomli             2.0.1  
tox               4.6.3  
typing-extensions 4.6.3  
virtualenv        20.23.1

Additional info

Failure started about 3 weeks ago:

failure_started

From what I can see nothing changed in the repo since March. Looking at the good run and the bad run, the only differences are:

  • Werkzeug (Good: 2.3.4, Bad: 2.3.5)
  • urllib (Good: 2.0.2, Bad: 2.0.3)

Changing urllib to the old version didn't fix it.
Changing Werkzeug to the old 2.3.4 version fixed the failure.

Werkzeug 2.3.5 changlog: https://werkzeug.palletsprojects.com/en/2.3.x/changes/#version-2-3-5

@darless darless added the bug Something isn't working label Jun 27, 2023
@darless
Copy link
Author

darless commented Jun 27, 2023

From 2.3.5 changelog the culprit is likely:

Improper Content-Length Parsing Werkzeug - 2716

test_media_types_method - can be fixed

Updating test_media_types_method to have q=0.5 instead of q=.5, fixes that test:

    def test_media_types_method(self, app, mocker):
        api = restx.Api(app)

        with app.test_request_context(
            "/foo", headers={"Accept": "application/xml; q=0.5"}
        ):

test_media_types_q

The 2nd test that fails is test_media_types_q, which also has q=.5. Changing that to 0.5 didn't fix it and it fails in a different way. The api.mediatypes() returns an empty list

self = <test_api_legacy.APITest object at 0x7fc0fdd8aca0>, app = <Flask 'tests.conftest'>

    def test_media_types_q(self, app):
        api = restx.Api(app)
    
        with app.test_request_context(
            "/foo", headers={"Accept": "application/json; q=1, application/xml; q=0.5"}
        ):
>           assert api.mediatypes() == ["application/json", "application/xml"]
E           AssertionError: assert ['application/xml'] == ['application...lication/xml']
E             At index 0 diff: 'application/xml' != 'application/json'
E             Right contains one more item: 'application/xml'
E             Use -v to get the full diff

tests/legacy/test_api_legacy.py:124: AssertionError
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> entering PDB >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>

>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> PDB post_mortem (IO-capturing turned off) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
> tests/legacy/test_api_legacy.py(124)test_media_types_q()
-> assert api.mediatypes() == ["application/json", "application/xml"]
(Pdb) api.mediatypes()
[]
(Pdb)

@peter-doggart
Copy link
Contributor

@darless Thanks for bringing this to my attention and the suggested fixes. For some reason, I didn't get notified by the build system! I will try and get it patched tomorrow. :)

@thefunkinator
Copy link
Contributor

Change in Werkzeug 2.3.5 as issued here caused the fail in build. As per HTTP RFC the quality-factor for HTTP Accept Header has to be like X.X. So changing them all to such format, all Unit tests and Tox test passes.

Also posted a PR for review. :-)

@peter-doggart
Copy link
Contributor

@chipndell Thanks for the PR, I never quite got the time to fix as I had planned!

@thefunkinator
Copy link
Contributor

thefunkinator commented Jul 13, 2023

@peter-doggart No worries. I loved to work on it. Also please share if you have more cool features that needs to be implemented. Learning and looking forward. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants