IDLE Improvements: Unit test for Delegator.py

[JayKrish]

BPO	18189
Nosy	@terryjreedy, @kbkaiser, @ncoghlan, @serwy, @rovitotv
Files	test_delegator.patch: test module for Delegator.py test_delegator1.patch: requires('gui') Fails. test_delegator2.patch: requires('gui') does not skip test_gui.py

^{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 = <Date 2013-06-30.22:39:33.767>
created_at = <Date 2013-06-11.11:30:19.796>
labels = ['expert-IDLE', 'type-feature']
title = 'IDLE Improvements: Unit test for Delegator.py'
updated_at = <Date 2013-06-30.22:39:33.767>
user = 'https://bugs.python.org/JayKrish'

bugs.python.org fields:

activity = <Date 2013-06-30.22:39:33.767>
actor = 'terry.reedy'
assignee = 'none'
closed = True
closed_date = <Date 2013-06-30.22:39:33.767>
closer = 'terry.reedy'
components = ['IDLE']
creation = <Date 2013-06-11.11:30:19.796>
creator = 'JayKrish'
dependencies = []
files = ['30544', '30627', '30691', '30695']
hgrepos = []
issue_num = 18189
keywords = ['patch']
message_count = 11.0
messages = ['190960', '191241', '191367', '191373', '191375', '191793', '191822', '192083', '192084', '192088', '192089']
nosy_count = 8.0
nosy_names = ['terry.reedy', 'kbk', 'ncoghlan', 'roger.serwy', 'Todd.Rovito', 'python-dev', 'JayKrish', 'philwebster']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue18189'
versions = ['Python 2.7', 'Python 3.3', 'Python 3.4']

[JayKrish]

Continuing the IDLE unittest framework initiated in http://bugs.python.org/issue15392.

A small unit test for IDLE Delegator.py module. This patch introduces a test module named test_delegator.py inside Lib/idlelib/idle_test, considering the points README file in idle_test. The test method test_setdelegate() simply passes a text widget to Delegator by calling setdelegate(), and test the widget returned by getdelegate() for the same.

[terryjreedy]

The format looks good.

The content looks skimpy. I am not familiar with Delegator and have not yet looked to see what more is needed. But this is enough to test working with a gui test.

This *is* a gui test. See msg190576. So it must be protected by requiring the gui resource. I am still investigating this subject, but I believe the following should work for the momemnt.

from test.resource import requires
...
class ...
    @classmethod
    def setUpClass(cls):
        requires('gui')

In a console, "python -m test test_idle" should report that test_delegator.Test_delegator was skipped, follows by other stuff. Please make the change, run from the console, and copy the result here. Add '-ugui' *before* 'test_idle' and Test_delegator should run.

As written, the empty root window is left to be closed by hand. The teardown method should have self.root.destroy(). That will destroy the children also, so I believe that is all that is needed.

[JayKrish]

The "from test.resource import requires" give me errors saying no module named resource in test (Am I missing anything?)
For the moment I use "from test.support import requires" as in test_delegator1.patch
.....
from test.support import requires
requires('gui')
class Test_delegator(unittest.TestCase):

    def setUp(self):
        .....

The results follows
---------------------------

jaykrish@jaykrish-Vostro-1520:~/gsoc/cpy/cpython$ ./python -m test -ugui test_idle
[1/1] test_idle
1 test OK.
[189373 refs]

--------------------------

jaykrish@jaykrish-Vostro-1520:~/gsoc/cpy/cpython$ ./python -m test test_idle
[1/1] test_idle
test test_idle failed -- Traceback (most recent call last):
  File "/home/jaykrish/gsoc/cpy/cpython/Lib/unittest/case.py", line 384, in _executeTestPart
    function()
  File "/home/jaykrish/gsoc/cpy/cpython/Lib/unittest/loader.py", line 32, in testFailure
    raise exception
ImportError: Failed to import test module: idlelib.idle_test.test_delegator
Traceback (most recent call last):
  File "/home/jaykrish/gsoc/cpy/cpython/Lib/unittest/loader.py", line 261, in _find_tests
    module = self._get_module_from_name(name)
  File "/home/jaykrish/gsoc/cpy/cpython/Lib/unittest/loader.py", line 239, in _get_module_from_name
    __import__(name)
  File "/home/jaykrish/gsoc/cpy/cpython/Lib/idlelib/idle_test/test_delegator.py", line 8, in <module>
    requires('gui')
  File "/home/jaykrish/gsoc/cpy/cpython/Lib/test/support.py", line 390, in requires
    raise ResourceDenied(msg)
test.support.ResourceDenied: Use of the 'gui' resource not enabled

1 test failed:
test_idle
[189233 refs]
---------------------------

And yes, the teardown method using self.root.destroy() now.

[terryjreedy]

Yes, 'support', not 'resource'. However, re-read my message for properly using requires to avoid the traceback.

def setUpModule():
    requires('gui')

may also work, but I am assuming that there might be non-gui test cases added later.

[terryjreedy]

As I discovered by experiment, unittest.SkipTest and subclasses are only handled properly inside of functions recognized and called by unittest. This could be better documented.

[JayKrish]

I wrapped requires('gui') as submitted in patch2. But this time commands "python -m test test_idle" and with -ugui does not skip delegatortest (it runs successfully to both).

I took Lib/test/test_tk.py as an example.
It calls runtktests.get_tests using support.run_unittest from test module.
Exploring Lib/tkinter/test/runtktests.py the get_tests method begins like

....
def get_tests(text=True, gui=True, packages=None):
"""Yield all the tests in the modules found by get_tests_modules.
If nogui is True, only tests that do not require a GUI will be
returned."""
attrs = []
if text:
attrs.append('tests_nogui')
if gui:
attrs.append('tests_gui')
....
....
also notice the main method calles "test.support.use_resources = ['gui']"
Now the tktests have the line * require('gui') * before the class bigins, for example have a look on test_textensions.py

So do we have to come up with the same procedure to skip and play with gui and non-gui tests for IDLE ? correct me if I am wrong anywhere.

[terryjreedy]

"test.support.requires" returns True if
"called from a context with __name__ = '__main__'" or
"'gui' in test.support.use_resources".

The first clause is never true for buildbots, so the presumption is that the test was invoked by a user with a gui screen. (Windoes has an extra test that is only relevant to buildbots.) See bpo-18103 for more discussion.

Here is a minimal test_gui.py file. In what follows, I only note the print output and skip notices.

Running test_gui by itself:

F5 when file is in Idle editor; and
python -m idlelib.idle_test.test_gui
have identical output, as they should,
and print 'in gui', 'nogui', because test_gui is main.

python -m unittest idlelib.idle_test.test_gui
python -m unittest idlelib.idle_test.test_gui.FakeGuiTest
have identical output for this simple file
and print 'nogui', 'skipped=1', because test_gui is not main.

The first of these two lines is not needed; just omit 'unittest'.
The second means that we cannot pick out and run just a gui testcase from a file.

Running as part of suite:

F5 with test_idle loaded in editor window
python -m test -v test_idle
python -m test.test_idle
python -m unittest -v idlelib.idle_test
all print 'skipped', 'nogui', 'skipped=1'
(and otherwise similar output).
because verbosity is set either in the file or command line
and because main is not test_gui and 'gui' not in use_resources.

python -m test test_idle
prints only 'nogui', no skip notice
(because skips are ignored by non-verbose regrtest).

python -m test -ugui test_idle
prints 'in gui', 'nogui'
because regrtest sets use_resources to ['gui'].

python -m idlelib.idle_test does not run but would if there were a .__main__ similar to test.test_idle.

Attribute use_resources can be set by anyone, not just by regrtest.
In test_idle, after "if __name__ == '__main__':", add
from test import support; support.use_resources = ['gui']
and
F5 with test_idle loaded in editor window
python -m test.test_idle
both print 'in gui', 'nogui'.

--
I added test_delegator.py and it skips when it should, but one must get verbose output to tell. (I do not see anything on the screen even when it does run.) Perhaps you mistook the absence of a failure report and the absence of a skip report to mean the absence of skips. It does not. Here is what I see.

C:\Users\Terry>python -m test test_idle
[1/1] test_idle
nogui
Warning -- os.environ was modified by test_idle
1 test altered the execution environment:
test_idle

If I add '-ugui', the *only* visible difference is the added 'in gui' from my file. Prints are really useful for leaving a record.

With -v, the difference is
skipped "Use of the 'gui' resource not enabled"
versus
test_setdelegate (idlelib.idle_test.test_delegator.Test_delegator) ... ok
It appears that methods within a skipped testcase are not listed, whereas a skipped method within a not-skipped testcase is listed.

I believe your patch is working correctly as far is it goes, and that we should move forward.

The Delegator.py patch needs something like the use_resource line I added to test_idle in order for the gui test to run when Delegator.py is run. The line I gave will work; I am also thinking of a context manager.

As I said on idle-dev, I decided you were right to call your first TestCase 'PathBrowserTest' instead of 'Test_xxx', as I did at first. Having testcases end with 'Test' while methods begin with 'test' is less confusing. So let us make it 'DelegatorTest'.

[terryjreedy]

The test_delegator patches were useful for working out and testing the use of the gui resource.

However, they are not appropriate for testing the Delegator class. It is a standalone class that has nothing to do with tkinter; in fact, the file has no imports. The test should be a normal text-only test. The only import needed is unittest.

Delegator has two unused methods. We can delete them, leaving just 4 methods to test.

[python-dev]

New changeset a568a5426a16 by Terry Jan Reedy in branch '2.7':
bpo-18189: remove unused methods in idlelib.Delegator.Delegator.
http://hg.python.org/cpython/rev/a568a5426a16

New changeset 9d65716367c1 by Terry Jan Reedy in branch '3.3':
bpo-18189: remove unused methods in idlelib.Delegator.Delegator.
http://hg.python.org/cpython/rev/9d65716367c1

[terryjreedy]

I asked on the core-mentorship list about pydev test philosophy. I got useful answers from Nick and Antoine that can be summarized as "We are pragmatic, not dogmatic." One may have to be a list member to read this link, but here it is.
http://mail.python.org/mailman/private/core-mentorship/2013-June/001855.html

I tried out multiple test methods, but decided that I preferred the continuous scenario in the patch that follows.

Because the code was written before the addition of sets, the private cache was a dict used as a set. I changed it to a set.

[python-dev]

New changeset 231c122b44b6 by Terry Jan Reedy in branch '2.7':
Issue bpo-18189: add test_delegator for Idle Delegator class.
http://hg.python.org/cpython/rev/231c122b44b6

New changeset c7605471e8ae by Terry Jan Reedy in branch '3.3':
Issue bpo-18189: add test_delegator for Idle Delegator class.
http://hg.python.org/cpython/rev/c7605471e8ae