[Python-Dev] [Python-checkins] cpython (2.7): Issue #18441: Make test.support.requires('gui') skip when it should.

[R. David Murray]

On Sun, 28 Jul 2013 01:09:43 +0200, terry.reedy <python-checkins at python.org> wrote:
> http://hg.python.org/cpython/rev/dd9941f5fcda
> changeset:   84870:dd9941f5fcda
> branch:      2.7
> parent:      84865:c0788ee86a65
> user:        Terry Jan Reedy <tjreedy at udel.edu>
> date:        Sun Jul 21 20:13:24 2013 -0400
> summary:
>   Issue #18441: Make test.support.requires('gui') skip when it should.
> (Consolidating this check and various checks in tkinter files and moving them
> to test.support and test.regrtest will be another issue.)
> 
> files:
>   Lib/idlelib/idle_test/test_text.py |   5 +---
>   Lib/test/test_idle.py              |  20 ++++++++++++++---
>   2 files changed, 17 insertions(+), 8 deletions(-)
> 
> 
> diff --git a/Lib/idlelib/idle_test/test_text.py b/Lib/idlelib/idle_test/test_text.py
> --- a/Lib/idlelib/idle_test/test_text.py
> +++ b/Lib/idlelib/idle_test/test_text.py
> @@ -216,10 +216,7 @@
>          requires('gui')
>          from Tkinter import Tk, Text
>          cls.Text = Text
> -        try:
> -            cls.root = Tk()
> -        except TclError as msg:
> -            raise unittest.SkipTest('TclError: %s' % msg)
> +        cls.root = Tk()
>  
>      @classmethod
>      def tearDownClass(cls):
> diff --git a/Lib/test/test_idle.py b/Lib/test/test_idle.py
> --- a/Lib/test/test_idle.py
> +++ b/Lib/test/test_idle.py
> @@ -1,9 +1,21 @@
> -# Skip test if _tkinter or _thread wasn't built or idlelib was deleted.
> -from test.test_support import import_module
> -import_module('Tkinter')
> -import_module('threading')  # imported by PyShell, imports _thread
> +# Skip test if _thread or _tkinter wasn't built or idlelib was deleted.
> +from test.test_support import import_module, use_resources
> +import_module('threading')  # imported by idlelib.PyShell, imports _thread
> +tk = import_module('Tkinter')
>  idletest = import_module('idlelib.idle_test')
>  
> +# If buildbot improperly sets gui resource (#18365, #18441), remove it
> +# so requires('gui') tests are skipped while non-gui tests still run.
> +if use_resources and 'gui' in use_resources:
> +    try:
> +        root = tk.Tk()
> +        root.destroy()
> +    except TclError:
> +        while True:
> +            use_resources.delete('gui')
> +            if 'gui' not in use_resources:
> +                break

I believe that this logic is incorrect.  If regrtest is run with "-u
gui", given this code the skip message will be "requires gui
resource"...but the caller specified the gui resource, which will make
the skip message completely confusing.

Instead, if it is true that 'gui' always means 'tk must work', then the
_is_gui_available function should do the Tk() check.  Currently as far
as I can see it is indeed the case that requires('gui') always means tk
must work.  So, I believe that the correct fix is to move
check_tk_availability to test.support, and call it from
_is_gui_available.  If we ever add another gui toolkit, we can deal with
moving the tk check out into a separate 'tk' resource at that time.

> +
>  # Without test_main present, regrtest.runtest_inner (line1219) calls
>  # unittest.TestLoader().loadTestsFromModule(this_module) which calls
>  # load_tests() if it finds it. (Unittest.main does the same.)
> 
> -- 
> Repository URL: http://hg.python.org/cpython
> _______________________________________________
> Python-checkins mailing list
> Python-checkins at python.org
> http://mail.python.org/mailman/listinfo/python-checkins