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

A couple of fixes to make CI tests more resilient #211

Merged
merged 5 commits into from
Sep 23, 2022

Conversation

dairiki
Copy link
Collaborator

@dairiki dairiki commented Sep 23, 2022

(This PR in an alternative to #209 and also includes #207.)

Main Changes

Explicitly add pytest’s rootdir to MYPYPYTHONPATH in our mypy tests.

This helps mypy find marshmallow_dataclass when it is installed in editable mode. (Ref #209, #207.)

Do not fail tests upon warnings from external dependencies

We were configuring pytest with -W error. This means tests failed if any warning was emitted by any dependency.
(E.g. marshmallow<3.15 elicits a warning from its use of distutils.)

Here we update the warning filter so that we only fail on warnings from marshmallow_dataclass or our tests.

Other warnings will still be reported by pytest but will not cause our tests to fail.


Other Changes

In order to get the CI tests to pass, we also cherry-pick PR #207 from @vit-zikmund

To get pre-commit checks to pass, we update the type of the second parameter to Union._serialize
to match changes made to Field._serialize typing in marshmallow 3.18.0.
(This keeps the pre-commit mypy check from failing due to "Liskov substitution principle" violation.)

Finally, we update the --target-version config for black (though I'm not sure that this makes any practical difference.)

@lovasoa
Copy link
Owner

lovasoa commented Sep 23, 2022

Wow, great ! But it looks like the tests still fail in the ci

This prevents issues when marshmallow_dataclass is installed
in editable mode. (See lovasoa#209, lovasoa#207.)
E.g. marshmallow<3.15 generates a deprecation warning from distutils.
There's no reason that should cause a test failure for us.
I have no idea whether this actually affects anything, but
might as well keep it up-to-date.
This tracks a change in type of the `attr` parameter to
`Field._serialize` made in marshmallow 3.18.0.

Here we also switch to using keyword args to typeguard.check_type
in an attempt to protect against upcoming
[changes in its signature](https://typeguard.readthedocs.io/en/latest/api.html#typeguard.check_type).
@dairiki
Copy link
Collaborator Author

dairiki commented Sep 23, 2022

Wow, great ! But it looks like the tests still fail in the ci

Of course they do! LOL

I've fixed things again so that they now pass. I had to include the changes from #207 to get that to happen. I've updated my original description of this PR to match what's currently in it.

@lovasoa: Do you want to continue support for python 3.6 (EOL 2021-12), or can that be cleaned out?

@lovasoa lovasoa merged commit 32299c8 into lovasoa:master Sep 23, 2022
@lovasoa
Copy link
Owner

lovasoa commented Sep 23, 2022

Thank you very much @dairiki and @vit-zikmund :)

@lovasoa
Copy link
Owner

lovasoa commented Sep 23, 2022

For python 3.6, it looks like there still are hundreds of daily downloads of marshmallow-dataclass with this python version: https://pypistats.org/packages/marshmallow-dataclass

But eventually, when python 3.6 becomes too much of a burden to support, and there are fewer daily downloads, we'll drop support, yes :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants