[issue25768] compileall functions do not document or test return values

[Brett Cannon]

Brett Cannon added the comment:

Thanks for the patch, Nicholas! I have added you to Misc/ACKS.

I did tweak the patch, though, to have the functions return booleans instead of 
1 or 0 and thus tweaked the docs to be less specific about the return type as 
well as the tests only doing assertTrue and assertFalse instead of directly 
comparing against 1 or 0 (which would have still worked, but I prefer the 
weaker definition in case the return value changes in the future).

--
resolution:  -> fixed
stage: commit review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25768] compileall functions do not document or test return values

[Roundup Robot]

Roundup Robot added the comment:

New changeset 71f071f2e074 by Brett Cannon in branch 'default':
Issue #25768: Make compileall functions return booleans and document
https://hg.python.org/cpython/rev/71f071f2e074

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25768] compileall functions do not document or test return values

[Brett Cannon]

Brett Cannon added the comment:

The patch LGTM, Nicholas! I'll commit it when I have a chance.

As for docs on test_importlib, nothing in the tests directory is documented to 
prevent people from depending on it since all the code in that directory is 
explicitly meant for testing Python and not general use.

--
stage:  -> commit review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25768] compileall functions do not document or test return values

[Nicholas Chammas]

Nicholas Chammas added the comment:

Alright, sounds good to me. Thank you for guiding me through the process!

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25768] compileall functions do not document or test return values

[Nicholas Chammas]

Nicholas Chammas added the comment:

Ah, I see. The setup/teardown stuff runs for each test.

So this is what I did:
* Added a method to add a "bad" source file to the source directory. It gets 
cleaned up with the existing teardown method.
* Used test_importlib to temporarily mutate sys.path as you recommended.

I think this is much closer to what we want. Let me know what you think.

By the way, are there any docs on test_importlib? I couldn't find any.

--
Added file: http://bugs.python.org/file41364/compileall.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25768] compileall functions do not document or test return values

[Brett Cannon]

Brett Cannon added the comment:

Do the tests take much longer with all of the added stuff in 
setUp()/tearDown()? It's just that all of it has to run for all tests. You 
could make a mixin or put all of it in a method that you selectively call and 
which registers the proper cleanup method.

As for skip_curdir, if you look at 
https://hg.python.org/cpython/file/default/Lib/compileall.py#l188 you will 
notice it requires the current directory to be on sys.path and I don't see you 
make any such change to sys.path (and if you do you can use 
test_importlib.util.import_state to temporarily mutate sys.path 
(https://hg.python.org/cpython/file/default/Lib/test/test_importlib/util.py#l165).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25768] compileall functions do not document or test return values

[Nicholas Chammas]

Nicholas Chammas added the comment:

I've added the tests as we discussed. A couple of comments:

* I found it difficult to reuse the existing setUp() code so had to essentially 
repeat a bunch of very similar code to create "bad" files. Let me know if you 
think there is a better way to do this.
* I'm having trouble with the test for compile_path(). Specifically, it doesn't 
seem to actually use the value for skip_curdir. Do you understand why?

--
Added file: http://bugs.python.org/file41277/compileall.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25768] compileall functions do not document or test return values

[Brett Cannon]

Brett Cannon added the comment:

Your solution to the problem SGTM. And testing compile_path() can be separate 
method.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25768] compileall functions do not document or test return values

[Nicholas Chammas]

Nicholas Chammas added the comment:

Absolutely. I'll add a "bad source file" to `setUp()` [0] and check return 
values as part of the existing checks in `test_compile_files()` [1].

Does that sound like a good plan to you?

Also, I noticed that `compile_path()` has no tests. Should I test it as part of 
`test_compile_files()` or as part of a different test function?

[0] https://hg.python.org/cpython/file/tip/Lib/test/test_compileall.py#l14
[1] https://hg.python.org/cpython/file/tip/Lib/test/test_compileall.py#l57

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25768] compileall functions do not document or test return values

[Brett Cannon]

Brett Cannon added the comment:

I just realized there are no tests for this in test_compileall either. 
Nicholas, do you have any interest in attempting to add tests for the return 
values before we document them?

--
title: compileall functions do not document return values -> compileall 
functions do not document or test return values
versions: +Python 3.6 -Python 3.5

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com