Skip to content

TST: Globally ignore import error during test collection for doctest#10325

Closed
pllim wants to merge 1 commit intoastropy:masterfrom
pllim:tst-gbl-ignore-imports
Closed

TST: Globally ignore import error during test collection for doctest#10325
pllim wants to merge 1 commit intoastropy:masterfrom
pllim:tst-gbl-ignore-imports

Conversation

@pllim
Copy link
Member

@pllim pllim commented May 7, 2020

Description

This pull request is to globally ignore import errors during doctest collection by pytest. This also moves importorskip out from module level to test level.

Closes #10302 (cc @lpsinger)

Also discussed in scientific-python/pytest-doctestplus#103 (comment) and #10260 (comment) .

@lpsinger
Copy link
Contributor

lpsinger commented May 7, 2020

This would give a false negative test result for actual bugs due to unintentional imports.

TST: Handle wcsaxes test collection without matplotlib.
@pllim pllim force-pushed the tst-gbl-ignore-imports branch from 6f7cd0a to 5748a00 Compare May 7, 2020 20:42
@pllim
Copy link
Member Author

pllim commented May 7, 2020

@lpsinger , can you please clarify? With this patch, from astropy.visualization import wcsaxes does not raise error, although people wouldn't try to use it without matplotlib anyway? This is no different from how astropy.io.misc.asdf is being handled right now in master.

@lpsinger
Copy link
Contributor

lpsinger commented May 8, 2020

Let's say that someone accidentally added an unintended dependency on package foo. You would want the line import foo to generate an error in the doctests. This would silence that error.

@pllim
Copy link
Member Author

pllim commented May 8, 2020

Let's say that someone accidentally added an unintended dependency

It would be ignored during collection, but you would think the tests that try to use that dependency would error out during run time, no?

Still, if unwanted dependency is added but somehow not used in testing, then yeah, I guess that would sneak pass. I am hoping in that case, we would catch it in code review, because the coverage would report that the LOC is not tested. 🤷

@pllim
Copy link
Member Author

pllim commented May 8, 2020

In addition, the name doctest-ignore-import-errors implies that this setting only affects doctest, and not regular tests? @jdavies-st recommended that we add this explicitly so we don't need to workaround a backward compatibility breakage over at pytest-doctestplus.

https://github.com/astropy/pytest-doctestplus/blob/dfc9e9f05f8c63f003d23fb82deacec11c6e3a11/pytest_doctestplus/plugin.py#L168-L176

@pllim pllim changed the title TST: Globally ignore import error during test collection TST: Globally ignore import error during test collection for doctest May 8, 2020
@pllim
Copy link
Member Author

pllim commented May 8, 2020

Okay, I realized that my PR title and original text was misleading. Sorry! Hopefully, I clarified them.

@astropy-bot
Copy link

astropy-bot bot commented Oct 10, 2020

Hi humans 👋 - this pull request hasn't had any new commits for approximately 5 months. I plan to close this in a month if the pull request doesn't have any new commits by then.

In lieu of a stalled pull request, please consider closing this and open an issue instead if a reminder is needed to revisit in the future. Maintainers may also choose to add keep-open label to keep this PR open but it is discouraged unless absolutely necessary.

If this PR still needs to be reviewed, as an author, you can rebase it to reset the clock.

If you believe I commented on this pull request incorrectly, please report this here.

@astrofrog
Copy link
Member

I think it is safe to close this

@astrofrog astrofrog closed this Oct 10, 2020
@pllim pllim deleted the tst-gbl-ignore-imports branch October 12, 2020 17:18
@pllim
Copy link
Member Author

pllim commented Oct 12, 2020

@astrofrog , any idea for #10302 then?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants