python-restx / flask-restx

Fork of Flask-RESTPlus: Fully featured framework for fast, easy and documented API development with Flask
https://flask-restx.readthedocs.io/en/latest/
Other
2.16k stars 335 forks source link

test_media_types_method - Test failure #547

Closed darless closed 1 year ago

darless commented 1 year ago

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:

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 commented 1 year ago

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 commented 1 year ago

@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. :)

pydevmans commented 1 year ago

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 commented 1 year ago

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

pydevmans commented 1 year ago

@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. :-)