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

Eliminate warnings from test suite #799

Merged
merged 2 commits into from
May 15, 2020

Conversation

eslavich
Copy link
Contributor

This fixes or hides the following warnings that come up when running our tests:

PytestDeprecationWarning: direct construction of AsdfSchemaFile has been deprecated, please use AsdfSchemaFile.from_parent
PytestDeprecationWarning: direct construction of AsdfSchemaItem has been deprecated, please use AsdfSchemaItem.from_parent
PytestDeprecationWarning: direct construction of AsdfSchemaExampleItem has been deprecated, please use AsdfSchemaExampleItem.from_parent

The pytest plugin has been updated to use from_parent when available.

AsdfConversionWarning: tag:stsci.edu:asdf/wcs/celestial_frame-1.1.0 is not recognized, converting to raw Python data structure
AsdfConversionWarning: tag:stsci.edu:asdf/wcs/icrs_coord-1.1.0 is not recognized, converting to raw Python data structure

Some of the tests open a file that references schemas that have been deprecated. At some point we'll want to create new test data, but it seems like we should wait until the dust settles on moving schemas around. For the time being I just asked pytest to ignore the warning.

YAMLLoadWarning: calling yaml.load() without Loader=... is deprecated, as the default Loader is unsafe. Please read https://msg.pyyaml.org/load for full details.

Changed asdf.tests.helpers to use safe_load instead.

The PR also adds a not-required Travis build that converts warnings into errors.

Resolves #789

@eslavich eslavich requested a review from jdavies-st May 15, 2020 17:19
@codecov
Copy link

codecov bot commented May 15, 2020

Codecov Report

Merging #799 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #799   +/-   ##
=======================================
  Coverage   94.07%   94.07%           
=======================================
  Files          42       42           
  Lines        5013     5013           
=======================================
  Hits         4716     4716           
  Misses        297      297           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 520a361...11fd151. Read the comment docs.

@eslavich eslavich added this to the 2.7.0 milestone May 15, 2020
Copy link
Contributor

@jdavies-st jdavies-st left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Just a question below.

[testenv:warnings]
basepython= python3.8
commands=
pytest --remote-data -W error -W ignore::asdf.exceptions.AsdfDeprecationWarning:asdf.asdftypes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the -W ignore::asdf.exceptions.AsdfDeprecationWarning:asdf.asdftypes be removed here as we capture and ignore it elsewhere?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or does this override the filterwarnings directive in [tool:pytest]?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, that's the issue -- pytest uses the last matched rule to determine if the warning should be ignored, and filterwarnings seems to get loaded first.

@eslavich eslavich merged commit 36efc21 into asdf-format:master May 15, 2020
@eslavich eslavich deleted the eslavich-fix-warnings branch May 15, 2020 19:21
@astrofrog
Copy link
Contributor

Thanks for the fix! Would it be possible to release a new version with this fix? 🙏

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

Successfully merging this pull request may close these issues.

Stop using deprecated pytest initializer
3 participants