Doctest should handle situations when test files are not readable

[bkabrda]

BPO	15178
Nosy	@tim-one, @terryjreedy, @bitdancer, @voidspace, @serhiy-storchaka
Files	doctest-dont-end-with-exception-on-unreadable-files.patch: Proposed patch to solve the issue doctest-dont-end-with-exception-on-unreadable-files-v2.patch: Catch exception in _test() doctest-dont-end-with-exception-on-unreadable-files-v3.patch doctest-dont-end-with-exception-on-unreadable-files-v4.patch doctest-dont-end-with-exception-on-unreadable-files-v5.patch doctest-dont-end-with-exception-on-unreadable-files-v6.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2012-06-25.11:01:44.820>
labels = ['type-bug', 'library']
title = 'Doctest should handle situations when test files are not readable'
updated_at = <Date 2019-03-15.22:47:29.268>
user = 'https://bugs.python.org/bkabrda'

bugs.python.org fields:

activity = <Date 2019-03-15.22:47:29.268>
actor = 'BreamoreBoy'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2012-06-25.11:01:44.820>
creator = 'bkabrda'
dependencies = []
files = ['26153', '26155', '26165', '26172', '26239', '37421']
hgrepos = []
issue_num = 15178
keywords = ['patch']
message_count = 17.0
messages = ['163936', '163964', '163965', '163970', '164036', '164043', '164078', '164120', '164441', '164447', '164506', '164508', '164585', '221298', '232500', '232519', '236043']
nosy_count = 6.0
nosy_names = ['tim.peters', 'terry.reedy', 'r.david.murray', 'michael.foord', 'serhiy.storchaka', 'bkabrda']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'needs patch'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue15178'
versions = ['Python 2.7', 'Python 3.4', 'Python 3.5']

[bkabrda]

Hi,
I think that doctest should be able to handle situations when the file that is being tested does not exist/is unreadable/etc...
Currently, doctest raises an IOError exception and the whole Python ends with an exception and backtrace.
I'm attaching a patch that fixes this and prints a simple description of what has gone wrong.

Thanks for considering!

[bitdancer]

In general in Python we let exceptions speak for themselves. It would, however, probably be reasonable to add the try/except and error message in _test, which should only be called when the module is run as a script.

[bkabrda]

Thanks for the comment David. I'm attaching second version that does the same in the _test function.

[bitdancer]

Great, thanks.

There's a loop around testfiles. I think it would be nicer to not call sys.exit. You could at the same time fix what appears to be a bug in the loop code. It looks like 1 is currently returned only if the last file fails.

Hmm, and given that there is a bug, adding tests would be great if you are up for it. I don't think there are currently any tests of the doctest command line functionality. That's more complicated than writing the fix, though :)

[bkabrda]

Sure, if you give me some time, I'll try to do everything as you suggest.

[bkabrda]

So I figured it might be best to first agree on the actual behaviour (what the patch will look like) and then I can write the tests.

So here is my 3rd version:

• It seems that returning 1 only if last file fails is intentional, as it is _inside_ the loop, so unsuccessful test immediately terminates the _test function - I thought it might be better to carry on with all of the tests, so here is what I did:
• I made a variable "failures", which represents if there were any failures during tests;
• I took the "return 1" one indentation level down, so all files are now traversed no matter how many of them fail;
• An error message is printed and "failures" set to True if a file is unopenable.

Does this look acceptable? If not, I will happily work further :) (and provide the tests once the behaviour is clear).

Thanks!

[bitdancer]

Ah, right, I misread the code when I looked at it.

There is usually a reason why something is done the way it is...though not always. But, running some example command lines, it looks to me like the current behavior is there because without it, you won't notice if the tests in the middle of your list of files failed. So, I think your second patch is more what we need. However, rather than calling sys.exit, I think it would be better to just print the message and do a return 1.

It is also possible we shouldn't change it at all. unittest (which has had some thought put in to the commnd line interface) just ends in a traceback in a similar situation (except that in unittest's case it is an ImportError...)

[bkabrda]

Ok, attaching 4th version :)

I think it is nice when the testing library can react in situations like this one and not show a traceback. IMHO it is always nicer to display a pretty user-readable message than ending with a traceback.

[terryjreedy]

Failure modes tend to get less attention that successful behavior. If I wrote a program that used doctest/unittest to test multiple files, I should like it to run an many as possible. If a filename is bad, print name as usual, say 'aborted', and '0/0 tests' run. Then at the end tell me via return value or exception about problems. In any case, having error behavior documented and consistent between modules would be good. I am not sure how much of this would be a bugfix versus enhancement.

[bitdancer]

The problem with running all the files as things stand is that the errors get lost in the other output. Changing that would definitely be an enhancement.

[bkabrda]

So maybe an optimal solution would be to add a note summarizing this to the end of output? I mean that this:

X passed and Y failed.

Could be replaced by:

X passed and Y failed, Z files were not loaded.

Then the user will know that the files were not loaded and he can search the test output for the reasons.

[bitdancer]

I think that sounds reasonable. The message should say files in all three cases', since the individual test file summary method will be similar but talking about numbers of tests.

[bkabrda]

Fifth version :)

• On failure in a loaded test, the _test function returns, so this behaviour is preserved.
• During _test function, count of both loaded and non-loaded files is kept.
• If a file fails to be loaded, the tests continue, but a non-zero return code is returned. Moreover, the results of loaded and non-loaded files are printed after each invocation.

This is a sample output:

$ python3 -m doctest -v spam c.py beans
Cannot read 'spam': [Errno 2] No such file or directory: 'spam'
Trying:
    2 * 2
Expecting:
    4
ok
1 items passed all tests:
   1 tests in c
1 tests in 1 items.
1 passed and 0 failed.
Test passed.
Cannot read 'beans': [Errno 2] No such file or directory: 'beans'
Test files read successfully: 1
Unreadable files: 2

Does this look better?

[BreamoreBoy]

@david can you pick this up given your previous involvement?

[serhiy-storchaka]

Added few nitpicks on Rietveld.

[bkabrda]

Attaching a new version of patch:

• Rebased to latest default branch
• Simplified prints
• Using OSError instead of IOError

Hopefully this is the final version :)

[serhiy-storchaka]

Doctest still failed with backtrace if argument is a file name.

$ ./python -m doctest aaa.py
Traceback (most recent call last):
  File "/home/serhiy/py/cpython/Lib/runpy.py", line 170, in _run_module_as_main
    "__main__", mod_spec)
  File "/home/serhiy/py/cpython/Lib/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/home/serhiy/py/cpython/Lib/doctest.py", line 2795, in <module>
    sys.exit(_test())
  File "/home/serhiy/py/cpython/Lib/doctest.py", line 2773, in _test
    m = __import__(filename[:-3])
ImportError: No module named 'aaa'

It would be good to have tests.