Reimplement reportinfo() in doctestplus extension

[nicoddemus]

In pytest 3.2.0 the member dtest is used in the reportinfo() function to report the location of the test examples in the doc, but the doctestplus extension does not pass one in the constructor.

@bsipocz is absolutely right when she mentions that given pytest's DoctestItem constructor:

class DoctestItem(pytest.Item):
    def __init__(self, name, parent, runner=None, dtest=None):

One assumes dtest can be None, but it appears that this has changed over the years and dtest is always assumed to be non-None and used in some other places, for example:

    def runtest(self):
        _check_all_skipped(self.dtest)
        self.runner.run(self.dtest)

I have thought about this and I think the best solution is to fix the doctestplus plugin in astropy: allowing dtest to be None in pytest doesn't make sense in the current code because its runtest method always uses self.dtest, otherwise it wouldn't do anything.

Ref: pytest-dev/pytest#2651

[astropy-bot]

Hi there @nicoddemus 👋 - thanks for the pull request! I'm just a friendly 🤖 that checks for issues related to the changelog and making sure that this pull request is milestoned and labelled correctly. This is mainly intended for the maintainers, so if you are not a maintainer you can ignore this, and a maintainer will let you know if any action is required on your part 😃.

Everything looks good from my point of view! 👍

If there are any issues with this message, please report them here

[bsipocz]

Thank you @nicoddemus! I'm not sure how to test this though other than I'll locally double check the failing packages using pytest 3.2.

Could you please add a changelog entry for 2.0.2?

[nicoddemus]

I'm not sure how to test this though other than I'll locally double check the failing packages using pytest 3.2.

Yeah unless we unpin pytest on CI, but then there are other issues to tackle I think. I did test locally using:

python setup.py test -t docs/wcs/index.rst

Could you please add a changelog entry for 2.0.2?

Sure, I was under the impression reading CONTRIBUTING that only user-visible changes needed to be mentioned that's why I didn't bother. Just pushed the new entry, let me know what you think.

[bsipocz]

Thanks, it looks good. It's good to have a changelog here as the tests and utils modules are widely used (though it's mostly under the hood) in packages that use the astropy package-template and/or astropy-helpers.

[nicoddemus]

It's good to have a changelog here as the tests and utils modules are widely used (though it's mostly under the hood) in packages that use the astropy package-template and/or astropy-helpers.

It makes perfect sense, thanks!

[bsipocz]

Note to self: this will be a tricky backport due to #6384, maybe best done in a PR!

[bsipocz]

@nicoddemus - Reading through your comments on the pytest PR, I wonder whether you have a suggestion what dtest could be passed to self._doctest_textfile_item_cls along with the current path and parent? That line in astropy is untouched for 4 years, so may be from the era when dtest wasn't used for much.

[pllim]

@nicoddemus

It took me a while to figure this out as well, but [here] doctestplus is creating a DocTestTextfilePlus passing two arguments (path and name), so dtest ends up as None (the default).

But it is also the same case as https://github.com/pytest-dev/pytest/blob/master/_pytest/doctest.py#L56 . So what are we doing wrong here? 🤔 I agree with @bsipocz that the correct fix might be to actually pass in something that does not result in dtest=None since pytest does not expect that anymore.

[nicoddemus]

Yeah it is confusing, but you see in pytest, DoctestTextfile subclasses only pytest.Module, while astropy's DocTestTextfilePlus subclasses both pytest.Module and doctest_plugin.DoctestItem, hence the difference. pytest's once also subclassed both, but that has changed since then.

I managed to create and pass a dtest to DocTestTextfilePlus while playing around with this yesterday by doing this:

        optionflags = get_optionflags(self)
        runner = doctest.DebugRunner(verbose=0, optionflags=optionflags,
                                     checker=_get_checker())
        parser = doctest.DocTestParser()
        dtest = parser.get_doctest(text, globs, name, filename, 0)

But passing a dtest as it stands would be confusing, given that runtest() is overwritten and doesn't use dtest at all. Perhaps the entire plugin could be reviewed in the light on how pytest's implementation changed over the years.

[bsipocz]

@nicoddemus - Thanks, this is very helpful.

I'm cc-ing @drdavella as this is super useful info for his project of factoring out all the pytest related stuff from astropy to a separate package.

[nicoddemus]

That's great, glad I could help. 👍