Report lineno from doctest

[hongquan]

This is to fix pytest-sugar#122 issue.

[coveralls]

Coverage remained the same at 92.093% when pulling af2c153 on AgriConnect:doctest-lineno into 1cf8266 on pytest-dev:master.

[RonnyPfannschmidt]

not all supported python versions have this availiable

[The-Compiler]

@RonnyPfannschmidt Even Python 2.6 has, so what doesn't?

[RonnyPfannschmidt]

@The-Compiler please excuse my mistake, the python version issues coming from travis upgrading images got me confused, i recalled a different doctest issue not supported on all python versions

[nicoddemus]

Thanks @hongquan !

• Please add a functional test for this so we can avoid a possible regression in the future.

• Add a CHANGELOG entry in changelog, I suggest changelog/2610.bugfix with something like:

  doctests line numbers are now reported correctly, fixing `pytest-sugar#122 <https://github.com/Frozenball/pytest-sugar/issues/122>`_.
  

• Rebase on the latest master, I just pushed a fix for the Travis issue.

[hongquan]

Hi, I rebased my PR and added changelog entry.

However, I have difficulty writing test case. I tried to follow examples in testing/test_doctest.py, but I don't know how to "assert" the output.

I wanted to use result.stdout.fnmatch_lines, like other examples, but what pytest prints out to the screen doesn't contain the info from DoctestItem.reportinfo(), which my PR aims to. I cannot do "fnmatch_lines" on it.

What should I do?

[nicoddemus]

I suggest to use testdir.inline_genitems() which will return the collected items without running the tests, then you can call item.reportinfo() directly and assert its return value.

[coveralls]

Coverage remained the same at 92.092% when pulling dea671f on AgriConnect:doctest-lineno into 81ad185 on pytest-dev:master.

[RonnyPfannschmidt]

@hongquan a merge is not a rebase :/

[nicoddemus]

a merge is not a rebase :/

Well no biggie. 😁

[RonnyPfannschmidt]

important question - is that lineno the line number in the file, or on a particular doctest element

[hongquan]

Hi.

I added test case. I did "merge" instead of "rebase" because rebase will overwrite Git history and make this PR invalid. I will have to close this PR and make a new PR just for the same bug fix. "Merge" and "rebase" are for the same purpose: "bring the latest update to my branch", but "merge" produces less garbage.

important question - is that lineno the line number in the file, or on a particular doctest element

According to doctest doc:

The line number within filename where this DocTest begins, or None if the line number is unavailable. This line number is zero-based with respect to the beginning of the file.

The lineno reported here may not be useful, but it is the only "line number" I can deduce from pytest's DoctestItem. At least it doesn't return None and cause other plugin crash.

[The-Compiler]

I did "merge" instead of "rebase" because rebase will overwrite Git history and make this PR invalid. I will have to close this PR and make a new PR just for the same bug fix.

After a rebase, you can git push --force to your branch and the PR will update. That way, it produces less "noise" because you don't have the merge commit. I agree it doesn't really matter though 😉

[The-Compiler]

oh, by the way:

The lineno reported here may not be useful, but it is the only "line number" I can deduce from pytest's DoctestItem. At least it doesn't return None and cause other plugin crash.

Note the "or None if the line number is unavailable" part. This should still be fixed properly in pytest-sugar.

[hongquan]

Note the "or None if the line number is unavailable" part. This should still be fixed properly in pytest-sugar.

That is if doctest.DocTest returns None. But here doctest.DocTest actually returns a number (1)!
If doctest.DocTest returns None, pytest-sugar has to handle None. But here doctest.DocTest doesn't return None, pytest's DoctestItem should not return None.

After a rebase, you can git push --force to your branch and the PR will update.

The "Commit" tab in this page then will contain a lot of "outdate diff".
You can get rid of "merge commit" by choosing "squash" method when merging this PR. All my commits will be "compressed" to 1.

[coveralls]

Coverage decreased (-0.04%) to 92.056% when pulling d40d774 on AgriConnect:doctest-lineno into 81ad185 on pytest-dev:master.

[The-Compiler]

That is if doctest.DocTest returns None. But here doctest.DocTest actually returns a number (1)!
If doctest.DocTest returns None, pytest-sugar has to handle None. But here doctest.DocTest doesn't return None, pytest's DoctestItem should not return None.

Yes, but if doctest.DocTest.lineno is None (which can happen as per the docs), pytest-sugar is still going to break. I think this change makes sense, but it still needs to fixed properly in pytest-sugar to work in all cases.

[hongquan]

I think this change makes sense, but it still needs to fixed properly in pytest-sugar to work in all cases.

I agree with you.

[nicoddemus]

Thanks @hongquan!

[bsipocz]

FYI this breaks our doctesting in astropy and affiliated packages where we use a customized DoctestModule https://circleci.com/gh/astropy/astropy/2583

[hongquan]

@bsipocz
Do you think pytest should be fixed something?

[bsipocz]

Yes, I'm happy that this fixed pytest-sugar, but broke all the others that had used the default None. See my PR for the fix, probably a test will be needed, but I wasn't sure where to add it.

[hongquan]

Thanks.

Sorry, I thought that self.dtest would always be set.