Stop using bundled pytest version - #5694

@astrofrog - Will astropy formally add pytest to the requirements? @larrybradley raised this in
astropy/photutils#491 (comment) to have as a guidance for photutils.

@bsipocz - if we do do this (and we've by no means made that decision) then we'll add py.test as a formal requirement to run the test suite (not to just use astropy)

I opened a PR against this PR at astrofrog#63 for some stuff that might be useful but did not make it into #5688, as a reminder if nothing else. FYI.

Without extra hacks (e.g. using ASTROPY_USE_SYSTEM_PYTEST or similar), it seems that before this makes it into release, affiliates are stuck with the bundled version, as it is already imported to the pytest namespace by the time the tests get to the point of importing the system pytest.

e.g. here astropy/photutils#491

@mhvk - any idea what the failures are due to here? Seems to be using pytest 2.7.2, but not sure why that would cause the errors shown.

@astrofrog - I did change DocTestTextFilePlus to use a feature that was common to the bundled pytest and pytest 3.x, but which may well have made it incompatible with earlier versions. Indeed, @pllim found that she had to fix pytest to >=3 to avoid getting pytest 2.7 being pulled in. Perhaps it is easiest to do that here too? See 07ef877

@mhvk - indeed, the bundled version is 2.8 and on Python 3.3, pytest 2.7 is installed, so I guess our infrastructure only works for 2.8+. I'll make sure to mention that in the requirements now for testing when updating the docs.

So one option here is to actually still remove the bundled pytest and just import the built-in one into astropy.tests.helper - that way the import will still work, but affiliated packages will still need to make sure that their tests pass with pytest 3.x. However since all the plugins have been fixed here, there might not actually be anything to fix in affiliated packages. For now, I've done this here so that we can test different affiliated packages with this branch.

In addition, I've removed the workaround from #811 to check whether the bug has been fixed.

Looks like the problem reported in #811 for which we had a workaround has been fixed for a couple of years (pytest-dev/pytest#112), so I think it's safe to remove the workaround.

This is ready for feedback!

LGTM, thanks @astrofrog!

I've rebased this - @bsipocz would you be happy to review this? (to make sure I didn't mess anything up during rebase)

As discussed with @eteq and @Cadair, we should add pytest as a runtime dependency for Astropy 2.0 to make things completely backward compatible. This actually makes sense anyway since Astropy provides pytest plugins which do have a dependency on pytest.

In future (e.g. Astropy 3.0) we could then consider removing pytest from the dependencies but as @Cadair pointed out, we could even keep it indefinitely.

In any case, once this is merged we should send an email to astropy-dev to explain this change.

To add just a bit more to #5694 (comment), the argument for keeping it as a runtime dependency is primarily that we are providing plugins for pytest, so we should consider it a dependency. Although maybe we should call it an "optional dependency"?

(Doesn't hold this up, just recording some of the discussion)

Who wants to have the honours to push the green button? @astrofrog?

I think for big PRs it's best if it's not the author that merges, so feel free to do it :)

🎉

Big thanks to @astrofrog to make this happen.

I wonder whether this will need a what's new entry?

I wonder whether this will need a what's new entry?

This is equivalent to things like "dropping package x.x support", right? IMHO, if we don't put those in "what's new", I don't think it is needed here.

Well, I would argue that this is very much different. While it doesn't have any effect on the simple user, it may have huge impact on packages that use the previously recommended way of using the bundled version directly from astropy, so it's more like a significant API change than just dropping a version support for something ancient. What I couldn't decide who the target audience for the "what's new". I expect maintainers to look at the full changelog and astropy-dev lists rather than the "what's new", but is that a good enough assumption?

putting something like "astropy now depends on external pytest, this will be installed by default" shouldn't confuse the less technical users.

But that wouldn't ring a bell for me as a maintainer either :) Anyway, as I said having just the changelog and the astropy-dev e-mail is probably enough.

Maintainers like @olebole already unbundled pytest (in fact, one of the reasons we knew the version we used was getting rather old...). But perhaps he can still comment on what he would generally look at.

I would recommend to have a statement like the one suggested by @Cadair here -- for a new version I usually check the Changelog for things like this when creating a Debian release.
However, in my "affiliated-package-maintainer" role (for python-cpl), I would usually ignore this; I would just install the newest version and RTFM only if there is a problem. So, maybe this should be sent to the affiliated mailing list as well (and maybe that mailing list could be promoted a bit as being useful for all developers of packages dependent on astropy).