classification
Title: Improve test.support.import_fresh_module()
Type: enhancement Stage: needs patch
Components: Library (Lib) Versions: Python 3.4
process
Status: closed Resolution: wont fix
Dependencies: Superseder:
Assigned To: Nosy List: Arfrever, brett.cannon, eli.bendersky, eric.snow, ezio.melotti, pitrou
Priority: normal Keywords:

Created on 2013-02-06 17:53 by eric.snow, last changed 2013-03-07 19:39 by eric.snow. This issue is now closed.

Messages (4)
msg181545 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2013-02-06 17:53
(+nosy list from #17037)

In issue 17037 (related to hiding PEP 399 boilerplate), Ezio and Eli recommended improving on the API of import_fresh_module().  I agree that it could be simplified, wrapped, or otherwise improved.

import_fresh_module() is used in the stdlib tests a number of times, primarily to support testing dual-implementations a la PEP 399 (see msg180809).  However, since it is already used for testing at least once module for a non-PEP-399 case (test_bz2), we need to be careful about any changes we'd make.  Changing test_bz2 to not use import_fresh_module() just so we could make it PEP-399 specific doesn't seem right.

Of the test modules that use import_fresh_module(), several pass multiple names for "fresh" and none pass more than one for "blocked".  Presumably a Python module may be accelerated by more than one accelerator module, hence allowing for lists for the two parameters, but none do so *now*.

Consequently, it may be worth doing something like what Eli suggested in msg181515: add another helper function that *is* PEP-399-specific.  Something like this:

def import_pure_and_accelerated_modules(name, *, fresh=(),
                                        accelerated_fresh=None,
                                        accelerators=None):
    if accelerators is None:
        accelerators = ['_' + name]
    if accelerated_fresh is None:
        accelerated_fresh = list(fresh) + list(accelerators)
    py_module = import_fresh_module(name, fresh=fresh, blocked=accelerators)
    acc_module = import_fresh_module(name, fresh=accelerated_fresh)
    return py_module, acc_module

Then you could use it like this:
    
py_module, c_module = import_pure_and_accelerated_modules('module')
py_module, c_module = import_pure_and_accelerated_modules('module', accelerators=['_other'])

Simply refactoring import_fresh_module() to work this way is an option, especially since it is used almost exclusively for PEP 399 compliance.  That way test.support would not grow yet another function.  The new interface would probably look like this:

def import_fresh_module(name, *, fresh=(), accelerated_fresh=None,
                        accelerators=None, blocked=None, deprecated=False):

It would still return a 2-tuple, but the second module would be None.  For the test_bz2 case, the change would be minimal:

bz2, = support.import_fresh_module("bz2", blocked=("threading",))

Why go to all this trouble?  It saves just one line of code.  Well, it also helps with writing and maintenance of dual-implementation tests, which I expect will be a growing segment of the stdlib unit tests as time goes by.

However, ultimately the point it moot if we have a more comprehensive test helper like what's being discussed in issue 17037.
msg181639 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2013-02-07 18:16
I don't think this is worth it. I say just start with the basics of issue 17037 and that subsumes this.
msg181642 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2013-02-07 19:04
My sentiment also.
msg183703 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2013-03-07 19:39
I'm closing this as #17037 offers a better solution and it's unlikely this proposal would make it if that one doesn't.
History
Date User Action Args
2013-03-07 19:39:43eric.snowsetstatus: open -> closed
resolution: wont fix
messages: + msg183703
2013-02-07 19:04:07eric.snowsetmessages: + msg181642
2013-02-07 18:16:52brett.cannonsetmessages: + msg181639
2013-02-06 17:53:27eric.snowcreate