From a3a970fcecc46f4c44f0d9e8ddaa66b00e0dae69 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Mon, 6 Feb 2023 05:58:51 +0000 Subject: [PATCH] Apply `filterwarnings = error` to tests and update This applies the error filter to force test failures on warnings, and does the necessary cleanup. Primarily, that means identifying the two classes of `ignore:` rules which are necessary to handle cases outside of this library's control. Secondarily, it forces the deprecation of the older url_for field specification syntax, which cannot be supported cleanly. Since this was deprecated in the last release, it is presumably safe to remove it, but the change is still noted as breaking in the changelog. --- CHANGELOG.rst | 3 ++ setup.cfg | 9 ++++++ src/flask_marshmallow/fields.py | 8 +++--- tests/conftest.py | 16 +++++++---- tests/markers.py | 6 ++-- tests/test_fields.py | 50 ++++++++++----------------------- tests/test_sqla.py | 10 +++---- 7 files changed, 50 insertions(+), 52 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 40e6b9c..72f6fe4 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -6,6 +6,9 @@ Changelog * Only support Python>=3.6, marshmallow>=3.0.0, and marshmallow-sqlalchemy>=0.24.0. * Add support for python3.11 +* *Backwards-incompatible*: ``URLFor`` and ``AbsoluteURLFor`` now do not accept + parameters for ``flask.url_for`` as top-level parameters. They must always be + passed in the ``values`` dictionary, as explained in the v0.14.0 changelog. Bug fixes: diff --git a/setup.cfg b/setup.cfg index 07f1427..2f78e75 100644 --- a/setup.cfg +++ b/setup.cfg @@ -6,3 +6,12 @@ ignore = E203, E266, E501, W503, B907 max-line-length = 110 max-complexity = 18 select = B,C,E,F,W,T4,B9 + +[tool:pytest] +filterwarnings = + # warnings are errors + error + # marshmallow-sqlalchemy triggers this on sqlalchemy 2.0 + ignore:The Query\.get\(\) method is considered legacy:sqlalchemy.exc.LegacyAPIWarning + # marshmallow triggers this when we test on marshmallow version 3.0 + ignore:distutils Version classes are deprecated\. Use packaging.version instead\.:DeprecationWarning:marshmallow diff --git a/src/flask_marshmallow/fields.py b/src/flask_marshmallow/fields.py index fa3890e..b6b404c 100755 --- a/src/flask_marshmallow/fields.py +++ b/src/flask_marshmallow/fields.py @@ -80,9 +80,9 @@ class URLFor(fields.Field): _CHECK_ATTRIBUTE = False - def __init__(self, endpoint, values=None, **kwargs): + def __init__(self, endpoint, values=None, id=None, **kwargs): self.endpoint = endpoint - self.values = values or kwargs # kwargs for backward compatibility + self.values = values or {} fields.Field.__init__(self, **kwargs) def _serialize(self, value, key, obj): @@ -115,10 +115,10 @@ class AbsoluteURLFor(URLFor): """Field that outputs the absolute URL for an endpoint.""" def __init__(self, endpoint, values=None, **kwargs): - if values: # for backward compatibility + if values: values["_external"] = True else: - kwargs["_external"] = True + values = {"_external": True} URLFor.__init__(self, endpoint=endpoint, values=values, **kwargs) diff --git a/tests/conftest.py b/tests/conftest.py index 1aa6d72..2db2d59 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,7 +1,7 @@ """Pytest fixtures for the test suite.""" +import pytest from flask import Flask from flask_marshmallow import Marshmallow -import pytest _app = Flask(__name__) _app.testing = True @@ -64,7 +64,7 @@ def book(id): return "Legend of Bagger Vance" -@pytest.yield_fixture(scope="function") +@pytest.fixture(scope="function") def app(): ctx = _app.test_request_context() ctx.push() @@ -85,10 +85,13 @@ class AuthorSchema(ma.Schema): class Meta: fields = ("id", "name", "absolute_url", "links") - absolute_url = ma.AbsoluteURLFor("author", id="") + absolute_url = ma.AbsoluteURLFor("author", values={"id": ""}) links = ma.Hyperlinks( - {"self": ma.URLFor("author", id=""), "collection": ma.URLFor("authors")} + { + "self": ma.URLFor("author", values={"id": ""}), + "collection": ma.URLFor("authors"), + } ) class BookSchema(ma.Schema): @@ -98,7 +101,10 @@ class Meta: author = ma.Nested(AuthorSchema) links = ma.Hyperlinks( - {"self": ma.URLFor("book", id=""), "collection": ma.URLFor("books")} + { + "self": ma.URLFor("book", values={"id": ""}), + "collection": ma.URLFor("books"), + } ) # So we can access schemas.AuthorSchema, etc. diff --git a/tests/markers.py b/tests/markers.py index c7a64fa..ffc32d2 100644 --- a/tests/markers.py +++ b/tests/markers.py @@ -1,9 +1,9 @@ import pytest import flask -from distutils.version import LooseVersion +from packaging.version import Version -flask_version = LooseVersion(flask.__version__) +flask_version = Version(flask.__version__) flask_1_req = pytest.mark.skipif( - flask_version < LooseVersion("0.11"), reason="flask 0.11 or higher required" + flask_version < Version("0.11"), reason="flask 0.11 or higher required" ) diff --git a/tests/test_fields.py b/tests/test_fields.py index 03fde79..59bdbd1 100644 --- a/tests/test_fields.py +++ b/tests/test_fields.py @@ -1,7 +1,7 @@ -from flask import url_for -from werkzeug.routing import BuildError import pytest +from flask import url_for from flask_marshmallow.fields import _tpl +from werkzeug.routing import BuildError @pytest.mark.parametrize( @@ -14,10 +14,6 @@ def test_tpl(template): def test_url_field(ma, mockauthor): - field = ma.URLFor("author", id="") - result = field.serialize("url", mockauthor) - assert result == url_for("author", id=mockauthor.id) - field = ma.URLFor("author", values=dict(id="")) result = field.serialize("url", mockauthor) assert result == url_for("author", id=mockauthor.id) @@ -28,13 +24,6 @@ def test_url_field(ma, mockauthor): def test_url_field_with_invalid_attribute(ma, mockauthor): - field = ma.URLFor("author", id="") - expected_msg = "{!r} is not a valid attribute of {!r}".format( - "not-an-attr", mockauthor - ) - with pytest.raises(AttributeError, match=expected_msg): - field.serialize("url", mockauthor) - field = ma.URLFor("author", values=dict(id="")) expected_msg = "{!r} is not a valid attribute of {!r}".format( "not-an-attr", mockauthor @@ -44,10 +33,6 @@ def test_url_field_with_invalid_attribute(ma, mockauthor): def test_url_field_handles_nested_attribute(ma, mockbook, mockauthor): - field = ma.URLFor("author", id="") - result = field.serialize("url", mockbook) - assert result == url_for("author", id=mockauthor.id) - field = ma.URLFor("author", values=dict(id="")) result = field.serialize("url", mockbook) assert result == url_for("author", id=mockauthor.id) @@ -56,14 +41,6 @@ def test_url_field_handles_nested_attribute(ma, mockbook, mockauthor): def test_url_field_handles_none_attribute(ma, mockbook, mockauthor): mockbook.author = None - field = ma.URLFor("author", id="") - result = field.serialize("url", mockbook) - assert result is None - - field = ma.URLFor("author", id="") - result = field.serialize("url", mockbook) - assert result is None - field = ma.URLFor("author", values=dict(id="")) result = field.serialize("url", mockbook) assert result is None @@ -74,11 +51,6 @@ def test_url_field_handles_none_attribute(ma, mockbook, mockauthor): def test_url_field_deserialization(ma): - field = ma.URLFor("author", id="", allow_none=True) - # noop - assert field.deserialize("foo") == "foo" - assert field.deserialize(None) is None - field = ma.URLFor("author", values=dict(id=""), allow_none=True) # noop assert field.deserialize("foo") == "foo" @@ -93,7 +65,10 @@ def test_invalid_endpoint_raises_build_error(ma, mockauthor): def test_hyperlinks_field(ma, mockauthor): field = ma.Hyperlinks( - {"self": ma.URLFor("author", id=""), "collection": ma.URLFor("authors")} + { + "self": ma.URLFor("author", values={"id": ""}), + "collection": ma.URLFor("authors"), + } ) result = field.serialize("_links", mockauthor) @@ -106,7 +81,10 @@ def test_hyperlinks_field(ma, mockauthor): def test_hyperlinks_field_recurses(ma, mockauthor): field = ma.Hyperlinks( { - "self": {"href": ma.URLFor("author", id=""), "title": "The author"}, + "self": { + "href": ma.URLFor("author", values={"id": ""}), + "title": "The author", + }, "collection": {"href": ma.URLFor("authors"), "title": "Authors list"}, } ) @@ -121,7 +99,7 @@ def test_hyperlinks_field_recurses(ma, mockauthor): def test_hyperlinks_field_recurses_into_list(ma, mockauthor): field = ma.Hyperlinks( [ - {"rel": "self", "href": ma.URLFor("author", id="")}, + {"rel": "self", "href": ma.URLFor("author", values={"id": ""})}, {"rel": "collection", "href": ma.URLFor("authors")}, ] ) @@ -134,7 +112,9 @@ def test_hyperlinks_field_recurses_into_list(ma, mockauthor): def test_hyperlinks_field_deserialization(ma): - field = ma.Hyperlinks({"href": ma.URLFor("author", id="")}, allow_none=True) + field = ma.Hyperlinks( + {"href": ma.URLFor("author", values={"id": ""})}, allow_none=True + ) # noop assert field.deserialize("/author") == "/author" assert field.deserialize(None) is None @@ -153,7 +133,7 @@ def test_absolute_url_deserialization(ma): def test_aliases(ma): - from flask_marshmallow.fields import UrlFor, AbsoluteUrlFor, URLFor, AbsoluteURLFor + from flask_marshmallow.fields import AbsoluteUrlFor, AbsoluteURLFor, UrlFor, URLFor assert UrlFor is URLFor assert AbsoluteUrlFor is AbsoluteURLFor diff --git a/tests/test_sqla.py b/tests/test_sqla.py index 5f11e4e..77eda45 100644 --- a/tests/test_sqla.py +++ b/tests/test_sqla.py @@ -22,7 +22,7 @@ class TestSQLAlchemy: - @pytest.yield_fixture() + @pytest.fixture def extapp(self): app_ = Flask("extapp") app_.testing = True @@ -46,15 +46,15 @@ def book(id): ctx.pop() - @pytest.fixture() + @pytest.fixture def db(self, extapp): - return extapp.extensions["sqlalchemy"].db + return extapp.extensions["sqlalchemy"] - @pytest.fixture() + @pytest.fixture def extma(self, extapp): return extapp.extensions["flask-marshmallow"] - @pytest.yield_fixture() + @pytest.fixture def models(self, db): class AuthorModel(db.Model): __tablename__ = "author"