classification
Title: Doctest should handle situations when test files are not readable
Type: behavior Stage: needs patch
Components: Library (Lib) Versions: Python 3.4, Python 3.5, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: bkabrda, michael.foord, r.david.murray, serhiy.storchaka, terry.reedy, tim.peters
Priority: normal Keywords: patch

Created on 2012-06-25 11:01 by bkabrda, last changed 2019-03-15 22:47 by BreamoreBoy.

Files
File name Uploaded Description Edit
doctest-dont-end-with-exception-on-unreadable-files.patch bkabrda, 2012-06-25 11:01 Proposed patch to solve the issue review
doctest-dont-end-with-exception-on-unreadable-files-v2.patch bkabrda, 2012-06-25 13:20 Catch exception in _test() review
doctest-dont-end-with-exception-on-unreadable-files-v3.patch bkabrda, 2012-06-26 06:21 review
doctest-dont-end-with-exception-on-unreadable-files-v4.patch bkabrda, 2012-06-27 05:26 review
doctest-dont-end-with-exception-on-unreadable-files-v5.patch bkabrda, 2012-07-03 07:17 review
doctest-dont-end-with-exception-on-unreadable-files-v6.patch bkabrda, 2014-12-12 08:23 review
Messages (17)
msg163936 - (view) Author: Bohuslav "Slavek" Kabrda (bkabrda) * Date: 2012-06-25 11:01
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!
msg163964 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-06-25 13:06
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.
msg163965 - (view) Author: Bohuslav "Slavek" Kabrda (bkabrda) * Date: 2012-06-25 13:20
Thanks for the comment David. I'm attaching second version that does the same in the _test function.
msg163970 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-06-25 14:01
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 :)
msg164036 - (view) Author: Bohuslav "Slavek" Kabrda (bkabrda) * Date: 2012-06-26 05:21
Sure, if you give me some time, I'll try to do everything as you suggest.
msg164043 - (view) Author: Bohuslav "Slavek" Kabrda (bkabrda) * Date: 2012-06-26 06:21
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!
msg164078 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-06-26 14:11
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...)
msg164120 - (view) Author: Bohuslav "Slavek" Kabrda (bkabrda) * Date: 2012-06-27 05:26
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.
msg164441 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2012-07-01 01:03
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.
msg164447 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-07-01 03:21
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.
msg164506 - (view) Author: Bohuslav "Slavek" Kabrda (bkabrda) * Date: 2012-07-02 05:39
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.
msg164508 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-07-02 06:48
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.
msg164585 - (view) Author: Bohuslav "Slavek" Kabrda (bkabrda) * Date: 2012-07-03 07:17
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?
msg221298 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2014-06-22 19:40
@David can you pick this up given your previous involvement?
msg232500 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-12-11 22:28
Added few nitpicks on Rietveld.
msg232519 - (view) Author: Bohuslav "Slavek" Kabrda (bkabrda) * Date: 2014-12-12 08:23
Attaching a new version of patch:
- Rebased to latest default branch
- Simplified prints
- Using OSError instead of IOError

Hopefully this is the final version :)
msg236043 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-02-15 14:37
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.
History
Date User Action Args
2019-03-15 22:47:29BreamoreBoysetnosy: - BreamoreBoy
2015-02-23 06:59:26serhiy.storchakasetstage: patch review -> needs patch
2015-02-15 14:37:58serhiy.storchakasetmessages: + msg236043
2014-12-12 08:23:22bkabrdasetfiles: + doctest-dont-end-with-exception-on-unreadable-files-v6.patch

messages: + msg232519
2014-12-11 22:28:08serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg232500
2014-12-11 22:07:58serhiy.storchakasetstage: patch review
versions: + Python 3.4, Python 3.5, - Python 3.2, Python 3.3
2014-06-23 02:57:13rhettingersetnosy: + tim.peters
2014-06-22 19:40:20BreamoreBoysetnosy: + BreamoreBoy
messages: + msg221298
2012-07-03 07:17:01bkabrdasetfiles: + doctest-dont-end-with-exception-on-unreadable-files-v5.patch

messages: + msg164585
2012-07-02 06:48:52r.david.murraysetmessages: + msg164508
2012-07-02 05:39:01bkabrdasetmessages: + msg164506
2012-07-01 03:21:32r.david.murraysetmessages: + msg164447
2012-07-01 01:03:36terry.reedysetnosy: + terry.reedy
messages: + msg164441
2012-06-27 05:26:26bkabrdasetfiles: + doctest-dont-end-with-exception-on-unreadable-files-v4.patch

messages: + msg164120
2012-06-26 14:11:05r.david.murraysetnosy: + michael.foord
messages: + msg164078
2012-06-26 06:21:21bkabrdasetfiles: + doctest-dont-end-with-exception-on-unreadable-files-v3.patch

messages: + msg164043
2012-06-26 05:21:35bkabrdasetmessages: + msg164036
2012-06-25 14:01:04r.david.murraysetmessages: + msg163970
2012-06-25 13:20:22bkabrdasetfiles: + doctest-dont-end-with-exception-on-unreadable-files-v2.patch

messages: + msg163965
2012-06-25 13:06:02r.david.murraysetnosy: + r.david.murray

messages: + msg163964
versions: + Python 3.2, Python 3.3
2012-06-25 11:01:44bkabrdacreate