Create a unittest framework for IDLE

[terryjreedy]

BPO	15392
Nosy	@terryjreedy, @ncoghlan, @ned-deily, @ezio-melotti, @serwy, @bitdancer, @rovitotv
Files	15392_idletestframework_1.patch: Initiating Idle TestFrameWork idletest2.diff: replaces previous patch idletest3.diff.txt idletest3u.diff: 'unix/osx format' idletest4u.diff frametest.py: Tests the test framework

^{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: ```python assignee = 'https://github.com/terryjreedy' closed_at = created_at = labels = ['expert-IDLE', 'type-feature', 'tests'] title = 'Create a unittest framework for IDLE' updated_at = user = 'https://github.com/terryjreedy' ``` bugs.python.org fields: ```python activity = actor = 'python-dev' assignee = 'terry.reedy' closed = True closed_date = closer = 'terry.reedy' components = ['IDLE', 'Tests'] creation = creator = 'terry.reedy' dependencies = [] files = ['29973', '30276', '30326', '30330', '30334', '30335'] hgrepos = [] issue_num = 15392 keywords = ['patch'] message_count = 60.0 messages = ['165825', '165833', '165889', '166072', '173304', '173797', '184927', '185024', '187393', '187539', '187545', '187562', '187569', '187587', '187616', '187621', '187697', '188481', '188649', '189258', '189261', '189262', '189264', '189267', '189268', '189269', '189270', '189271', '189274', '189275', '189276', '189284', '189290', '189313', '189339', '189564', '189565', '189627', '189712', '189713', '189717', '189723', '189733', '189775', '189787', '189790', '190129', '190144', '190165', '190166', '190174', '190189', '190266', '190376', '190384', '190385', '190386', '190387', '190390', '202083'] nosy_count = 14.0 nosy_names = ['terry.reedy', 'ncoghlan', 'ned.deily', 'ezio.melotti', 'roger.serwy', 'r.david.murray', 'Todd.Rovito', 'tshepang', 'python-dev', 'francismb', 'Guilherme.Sim\xc3\xb5es', 'JayKrish', 'Tomoki.Imai', 'alex.rodas'] pr_nums = [] priority = 'normal' resolution = 'fixed' stage = 'resolved' status = 'closed' superseder = None type = 'enhancement' url = 'https://bugs.python.org/issue15392' versions = ['Python 2.7', 'Python 3.3', 'Python 3.4'] ```

[terryjreedy]

Idle needs a unittest framework for module test modules. This means a test directory with at least one test module, a runtest module that successfully runs that test module, any new support functions needed for that test module, and inclusion of test_idle in Lib/test.

I am thinking of something like Lib/tkinter/test, but without subdirectories. I presume there should be something like tkinter/test/runtests, but I do not know if it reflects current best practice, nor whether it should be in idlelib or in idlelib/test.

One feature of runtests and the tkinter framework is separation of tests that do and do not require a gui. This requires cooperation of writers of each test module. Buildbots can only run tests that do not require a gui and nearly everyone else wants the same. On the other hand, we need automated gui tests too. A complete test of idlelib/runtest requires a test module with both kinds of tests.

List/test has test_tk. It runs tests with enable_gui=false unless run directly (and intentionally) as the main module. (Testing test_idle would also require tests in both modes.) It checks that tkinter is available. Should test_idle also check that idle is available? (On Windows, one can install both or neither. I don't know the situation on other systems.)

This issue was inspired by bpo-12510 and is interdependent with it. As part of that issue, I would like to convert the expanded test set to run under unittest. bpo-12510 needs a place to put a test_calltips module. On the other hand, this issue needs a test_x module to test runtests and test_idle.

tkinter/test/support has functions that can be used in idle tests, in particular, simulate_mouse_click(). I believe a needed addition is simulate_text_entry. That is needed to really finish the calltips tests. Currently, complete testing of tooltips requires non-automated hand tests.

Would simulate_mouse_click include selecting menu items, or would that be a separate function?

With such a test framework established, we could start adding tests as part of other issues, as is normal practice.

(Note: tkinter/test is pretty skeletal, and probably could use expansion, but except for support functions immediately needed for test_calltips, that is another issue.)

[bitdancer]

All the tests should run from the standard test runner tool (currently regrtest), with the GUI tests guarded by the GUI resource, which is how it works for TK. I always run the test suite with -uall before non-trivial commits, so I do in fact run the TK gui tests on a regular basis. I would do the same with idle. I hope there are at least a few other developers that run with -uall on a regular basis :)

Whether or not there is value in a separate runner is a separate question. If there is value in having one to you and the other people working on idle, then make one. But for those of us who touch it only occasionally, the separate runner is not likely to get used. I wasn't even aware there was one for tkinter.

There was a push a while back suggesting that best practice would be to have all tests be in the 'test' subdirectory. I moved email/test into test/test_email, and I prefer having it there. On the other hand, Michael prefers having unittest/test, and has kept the unittest unit tests there. So that part is up to you, but I encourage you to consider putting them in the test subdirectory, and if you want to do that I would help out with moving the existing tkinter tests to the 'test' dir.

And yes, either way the tests should test for the presence of idle and skip if idle is not available.

[terryjreedy]

I have not really used unittest, so I only know to blindly copy what has been done. Hence I need help to do better.

Do you actually get gui tests for test_tk? When I run test/testtk from Idle editor, and hence as \_main__, I only get non-gui tests in spite of enable_gui being set True. Or maybe I just did not see them. (This is actually an improvement over the normal failure of test_tk on Windows. See bpo-10652)

Is tkinter/test/runtktests properly called a test runner? test/test_tk uses it to gather the tests to run: support.run_unittest( *runtktests.get_tests(text=False, packages=['test_tkinter'])) It is used within the ttk tests also. As I suggested above, I do not really know if we really need the equivalent.

I strongly prefer idlelib/test since it will make developing **much** easier for me on Windows. Also, it would be part of the optional install of idlelib, as tkinter/test is for tkinter.

Adding test/test_idle will not be too much use until issue bpo-10652 is resolved so it would actually run with -m test.

[bitdancer]

The screen flickers a bunch, so something involving my display is certainly happening.

As for runtktests...test suites that live in the package as opposed to the test directory require support files to gather the tests to be run. This can be done by the test_xxxx.py file in the test directory (which is what regrtest runs), or it can be done in a function imported from the packages test directory. For tkinter it looks like the test gathering is done in runtktests. Most Python test_xxxx file can be run directly to run the relevant tests, and runtktests works the same way. So yes it is a runner, but that appears to be a convenience just like for other test_xxxx files, and not anything special.

[ned-deily]

There is now one test available to be applied from bpo-16226; see http://bugs.python.org/file27613/issue16226_test.patch. It may need to be modified depending on where the tests are ultimately put in the source tree.

[f52c13af-a5a0-4c40-a7a2-6ce855459ec8]

bpo-7883 also has a test.

[terryjreedy]

Since writing this, I ran test_ttk_guionly by itself, so it would run, and saw the flickering windows. I have thought about using unitest.mock and Nick has offered to help particlarly with that.

[ncoghlan]

I'll start with a bit of philosophical guidance :)

1. As much as possible, push logic testing down into the non-GUI tests. Where unittest.mock can help here is when you have a piece of code to test that is *almost* independent of the GUI, but needs to call an API to either get input from the user or to send something to the screen. unittest.mock can then be set up to intercept that call, either to check that it happened as expected (display operations) or to provide a predetermined answer (input operations).

2. For the GUI tests, unittest.mock is likely to be most useful in providing predetermined input. There's only so much you can do with this, it's ultimately about ensuring that the code tested against mocked out APIs in the lower level tests at least doesn't throw exceptions when tested against the real APIs. Proper GUI testing is actually a much harder test automation problem, so it probably makes sense to focus on the non-GUI tests for now.

[5503c36c-3ca2-487e-9ce5-448dd0cfb599]

I'm a student thinking of participating in Google Summer of Code. And want to work to create a unittest for IDLE.

Using unittest.mock seemed to be good way to test GUI. But there is a problem. There is no unittest.mock in Python2. http://docs.python.org/2/library/unittest.html

I think using third party mock seemed to be ok, but not best way. https://pypi.python.org/pypi/mock Because, IDLE is a part of official python. I think relying on third party module is not good.

Of cource, I would forcus on non-GUI unittest. But GUI-test would be needed.

[44e89a00-efc6-4dc2-8723-4b5e6ffe36d5]

The aimed versions for this unit test frame work is python 3.3, 3.4. So as Nick said, unittest.mock may have no issues on this. As you said 3rd party modules seems not a better way.But the link you provided ( https://pypi.python.org/pypi/mock ) says .. "mock is now part of the Python standard library, available as unittest.mock in Python 3.3 onwards." And yes, GUI testings While I was writting my proposal on this project Todd.Rovito mentioned me " .. you should still complete some GUI tests so future developers will have a model. "

[44e89a00-efc6-4dc2-8723-4b5e6ffe36d5]

There is a need of a proper design in whether to put tests in test sub directory or to create idlelib/test directory.

For my GSoc proposal initial draft, I suggested to start with
Put tests in test/test_idle directory (like test/test_email would be the best practice). Test for the presence of idle and skip if idle is not available. Move the existing,pending tests for idle ( http://bugs.python.org/file27613/issue16226_test.patch , http://bugs.python.org/file24075/test_idlelib.py,....) into this directory. (which my patch 15392_idletestframework_1.patch did for me)

Considering the point terry.reedy mentioned, "Adding test/test_idle will not be too much use until issue bpo-10652 is resolved so it would actually run with -m test", my view is, the issue bpo-10652 on this message is on a healthy patch review, so it would actually run with -m test in near-future Expecting your suggestions on this design decisions.

[5503c36c-3ca2-487e-9ce5-448dd0cfb599]

Oh, no support for Python2? I think, it is too old, but still needs bug-fix supports. IDLE for Python2 is really buggy. For example, unicode problems in my environment. http://bugs.python.org/issue17348 It might be GUI related problem.

By the way, your proposal seemed good to me. My proposal is almost same. Using unittest module, make directory such as idlelib/test and write unittest there.

But I want to include Python2 and GUI support. (So, there are unittset.mock problem)

Of cource, it is very import to concentrate on one thing (i.e Python3). I cannot deside whether to drop Python2 support now.

I considered PEP-434 premit me writing unittest for Python2. http://www.python.org/dev/peps/pep-0434/

How do you think?

[bitdancer]

I think the issue of whether/how to test Python2 idle should be discussed on idle-dev. One strategy is to make the tests backward compatible (so no mock). Another possible strategy is to have an extra bit of test framework for IDLE in 2.7 that copies the tests and mock from another location in order to run tests against 2.7. That is, the tests wouldn't be an official part of the 2.7 repro and would not be run by the buildbots. The reason for doing that would be to allow the Python3 tests to be as rich as possible while still doing some testing on 2.7. But that's just a thought experiment, as I said I think the full strategy should be hashed out on idle-dev (I'm not a member of that list :)

[terryjreedy]

When I opened this last July, I intentionally left off 2.7 for multiple reasons.

1. I knew that the new-in-3.3 mock module would probably be needed for some types of testing. (As for 3.2: at that time, I expected 3.2.4 to be released in September or October, just after 3.3.0, as would have been normal.)

2. 2.7 is yesterday's horse. I hope we fix disabling bugs before maintenance stops. I do not expect all new features to be backported. I personally have no motivation to do so if a patch does not apply cleanly, or nearly so. Also, I knew that a full test suite would not happen instantly and that 2.7 maintenance would probably be tapering off by the time one was fully in place.

3. My experience with calltips bpo-12510: I fixed IDLE closing bugs for all versions. I did not backport all the text display improvements; the 2.7 code was different partly due to dealing with old-style classes. I did not backport the new tests I added in msg162510 and msg165057. They depend matching exact texts. They will need to be changed again when calltips are adjusted to the new multiline signatures in builtin-function docstrings. The will probably need to be changed if calltips are changed to use the new-in-3.3 signature objects. There are proposed 3.4 changes for compiling C functions that might impact calltips.

4. There was no PEP-434 ;-). While *I considered the relaxed backport policy to be the defacto policy then, it *was a bit fuzzy, and others disagreed. With the PEP accepted, I am more open to backporting at least some tests if someone else (like Tomoki) does most of the additional work.

(I plan to start with this issue when my development machine is back from repairs and proper set up again.)

[ezio-melotti]

If things are fixed/added/improved on 3.x, there should be tests for them, and if they are backported on 2.7, tests should be backported as well. If mock makes testing easier, I think it would be acceptable to use it and then have IDLE devs install a 2.7 mock and use it to run all the tests. The tests on 2.7 could use skip decorators to be skipped if mock is missing, without having to keep these tests separate from the rest. If necessary a script to install mock on 2.7 could be provided, and possibly used by the buildbots too.

This means that the initial framework should be backported, otherwise it won't be possible to backport any of tests that are going to use it. FWIW the attached patch should use unittest.main() instead of test_main+run_unittest.

[ncoghlan]

+1 for Ezio's suggestion regarding tests that need the mock module. To simplify backporting you may want to do something like the following in a helper module:

try:
    import unittest.mock as mock
except ImportError:
    try:
        import mock
    except ImportError:
        mock = None

Then the individual test files would retrieve the mock attribute from the helper module and check it with the skip decorators in the tests that needed mocks.

[5503c36c-3ca2-487e-9ce5-448dd0cfb599]

I have already posted idle_dev mailing list (and, no one replied :P).

I think, Ezio's suggestion is good if we will work for Python2. Using unittest.mock and mock to support Python2 and Python3. My proposal for GSoC is here. Making very initial version for Python2 and Python3 and, complete Python3 version and, backport it in the end. I'll set backporting for Python2 as optional work. My main goal is to make unittest for Python3.

[a2c68251-5f3b-4972-91b1-b78401c27569]

This issue appears like it is making progress. For a very small contribution I tested JayKrish's patch and it seems to work on my Mac. The results are documented below. Any comment from Python Core Developers on what needs to happen to get it committed? It appears to work with -m test and -v test_idle. Thanks!

/python.exe -m test -v test_idle == CPython 3.4.0a0 (default:186cf551dae5+, May 5 2013, 22:41:07) [GCC 4.2.1 Compatible Apple Clang 4.1 ((tags/Apple/clang-421.11.66))] == Darwin-12.3.0-x86_64-i386-64bit little-endian == /Volumes/SecurePython3/cpython/py34/build/test_python_15812 Testing with flags: sys.flags(debug=0, inspect=0, interactive=0, optimize=0, dont_write_bytecode=0, no_user_site=0, no_site=0, ignore_environment=0, verbose=0, bytes_warning=0, quiet=0, hash_randomization=1) [1/1] test_idle test_DirBrowserTreeItem (test.test_idle.test_PathBrowser.PathBrowserTest) ... ok

---------------------------------------------------------------------- Ran 1 test in 0.000s

OK 1 test OK.

[ncoghlan]

I've added 2.7 to the affected versions - the core unittest framework should be present in all 3 versions, so the choice of if/when to backport a fix and its test can be made on a case-by-case basis, rather than being a foregone conclusion due to the lack of IDLE test infrastructure in 2.7

If/when mock is used in any tests, then a compatibility module isn't actually needed, 3.3 and 3.4 can just use "from unittest import mock" while 2.7 can use "mock = support.import_module('mock')" (so those tests will run if you arrange to make the mock backport from PyPI available when running the tests, but will be skipped otherwise).

Terry, are you happy with that plan? If so, over to you to get the ball rolling and commit this as a starting point :)

[terryjreedy]

Attached is a 3.3 patch that I *believe* is ready to commit and push. With my Win7 repository build, I tested running one test from a file (after "if __name == '__main':") or command line ('python_d -m idlelib.PathBrowser'); all idle tests with an interactive interpreter (console or idle shell, see @template.txt for input); all idle tests from the command line ('python_d -m test.test_idle'); and idle tests as part of the CPython suite (python_d -m test). I also tested versions of all but the two batch-mode tests in my 3.3.1 installation.

From what i know, I do not believe there should be much issue with the framework working on non-Windows systems. But I would not mind verification. I presume this patch will work as is with 3.4, but ditto. 2.7 may need one tweak noted below. (Testing with 2.7 is difficult for me at the moment.) Nick: yes to your 2.7 plan. PEP-343 changes things.

Once applied to all three branches I think this patch is enough to close this issue and 'get the ball rolling'. Mock, gui testing, and any other big problems should be separate issues.

The patch adds Itest/init.py (merges doc example, part of previous __init.py) Itest/@template (for both test_x.py and startup from x.py) Itest/test_calltips.py (with first 2 of many tests) Itest/test_pathbrowser.py (revised, with 1 test, see note below) test/testidle.py (revised skeleton of previous \_init.py)

and revises CallTips.py PathBrowser.py to run the corresponding tests when run as '__main__'.

Question: "Unittest supports skipping individual test methods and even whole classes of tests" How about skipping (possibly conditionally) an entire file -- especially test_idle, which has no classes, or and test_xxx with multiple classes?

For multiple reasons related to the fact that Idle and idlelib *are* special, as recognized by PEP-343, I want to keep the individual test files in an idlelib subdirectory. as with tkinter tests. (I changed the name from 'test', so that 'import test' will a always import Lib/test.)

• Once in idlelib, changing to Itest is easier than to ../test/test_idle. With 62 .py files in idlelib, we will be opening pairs of files a lot.

• Copying code and test files to another directory, such as an installation idlelib/, will be easier. I will be doing that to run with new features and test things in the installation environment.

• PEP-343 makes most of idlelib/ a private arena. Test/ is a public tree mainly for the buildbots. Tests put there are supposed to pass. In brief, I personally feel much more comfortable mucking around in a private arena with a small public window that can selectively open and closed as needed.

• We need an Itest directory anyway for things that do not belong in test/*. @template is an example, and I have in mind a couple of non-buildbot test scripts. We may also want an idle-specific support module, as tkinter has.

• Once people install Python so it runs, some still have problems running Idle. They ask a class instructor or someone else; post here, python-list, stackoverflow, or elsewhere; or give up. Sometimes the problem is with tkinter, sometimes with idle, sometimes with their knowledge. A good diagnosis script should save support time and user frustration. Both tkinter and idle tests should be available for this. The Windows installer makes the 17 mb of test/* optional.

Other changes from the previous patch:

• Use unittest.main to run tests.

• Guard test_idle execution with tkinter import, as it is _tkinter, not idlelib that might not be built. I left the guard for idlelib since someone who deletes idlelib/* might forget to delete test/test_idle.

• Exceptions raised outside of self.assertXyz (and self.fail) calls are regarded as an error in the test code rather than a failure of the code tested (26.3.4. Organizing test code). So, there being no 'assertNotRaises' context manager, I added "try:...except ...: self.fail()" to test_pathbrowser.py so a failure will be reported as such and not as an error. If there is a better way to do this, let me know.

Since 3.x chains exceptions and prints the original traceback, no message argument is needed with self.fail to explain the failure. For 2.7, I believe one should be added.

Note: to empirically test that a test fails properly, it must be run with code that does not work properly. This is a problem with writing tests after the fact for code that works properly.

I checked all 62 idlelib/*.py files for a test to see what we have to work with: the answer is 'not much'. Less than half the files have a test. All but 2 that do bring up a window and require a human to key, click, and look according to a script that is not provided. (This is not to say that the existing code will not be helpful for some gui tests.) One of the 2 remaining text tests prints multiple pages (a config file) for a person to check. By coincidence, the only automated tests are the ones in CallTips.py, the first file I looked at, last summer.

[a2c68251-5f3b-4972-91b1-b78401c27569]

Terry I think you have a typo you mean PEP-434 (http://www.python.org/dev/peps/pep-0434/) where PEP-343 exists. Can you please confirm?

[terryjreedy]

Yes, of course. Thanks for correcting. 434, 434, 434,...

[ezio-melotti]

A few comments about the patch: 1) I would prefer a more explicit idle_test instead of Itest (assuming "I" means idle); 2) having idle_test in Lib/test would be better if possible (see bpo-10572); 3) the tests should only be in idletest, and not in the "if \_name == '__main':" of the files; 4) I'm not sure the @template file is necessary. It could be documented somewhere else though (see e.g. http://docs.python.org/3/library/test.html#writing-unit-tests-for-the-test-package).

IOW putting all tests in a separate dir is a good thing, and the dir could either be in Lib/test or in Lib/idlelib. All the tests should be inside this dir, except for Lib/test/test_idle.py that should be the entry point used to run all the tests in idle_test (see e.g. the tests for ctypes, email, and json). Individual tests in idletest can be run on their own, and they should use the "if \_name == '__main': unittest.main()" idiom.

Question: "Unittest supports skipping individual test methods and even whole classes of tests" How about skipping (possibly conditionally) an entire file -- especially test_idle, which has no classes, or and test_xxx with multiple classes?

You can raise unittest.SkipTest or use support.import_module(). This works fine if you run the tests through regrtest, but be aware that unittest itself only sees this as a skip from 3.4 (see bpo-16935).

• Exceptions raised outside of self.assertXyz (and self.fail) calls are regarded as an error in the test code rather than a failure of the code tested (26.3.4. Organizing test code). So, there being no 'assertNotRaises' context manager, I added "try:...except ...: self.fail()" to test_pathbrowser.py so a failure will be reported as such and not as an error. If there is a better way to do this, let me know.

If it's supposed to work the try/except is not necessary IMHO. By this logic every line of code should be wrapped in a try/except :)

Since 3.x chains exceptions and prints the original traceback, no message argument is needed with self.fail to explain the failure. For 2.7, I believe one should be added.

If you still want to keep the try/except, I would provide a meaningful failure message in any case.

Note: to empirically test that a test fails properly, it must be run with code that does not work properly. This is a problem with writing tests after the fact for code that works properly.

It's not difficult to break the code to test that test works though :)

I checked all 62 idlelib/*.py files for a test to see what we have to work with: the answer is 'not much'. Less than half the files have a test.

If possible I think these should all be moved to idle_test. You can also add an additional resource to regrtest to skip the ones that require manual intervention.

[a2c68251-5f3b-4972-91b1-b78401c27569]

Terry, On my Mac with "hg revert -a" and "hg pull -u" the patch fails to apply on CallTips.py and PathBrowser.py under the latest version of Python 3.4. Here is the output when I try to apply the patch:

rovitotv-pc:py34 rovitotv$ hg import --no-commit /Volumes/SecurePython3/source/15392idletests.diff applying /Volumes/SecurePython3/source/15392idletests.diff patching file Lib/idlelib/CallTips.py Hunk #1 FAILED at 263 1 out of 1 hunks FAILED -- saving rejects to file Lib/idlelib/CallTips.py.rej patching file Lib/idlelib/Itest/@template.txt patching file Lib/idlelib/Itest/init.py patching file Lib/idlelib/Itest/test_calltips.py patching file Lib/idlelib/Itest/test_pathbrowser.py patching file Lib/idlelib/PathBrowser.py Hunk #1 FAILED at 94 1 out of 1 hunks FAILED -- saving rejects to file Lib/idlelib/PathBrowser.py.rej patching file Lib/test/test_idle.py adding Lib/idlelib/Itest/@template.txt adding Lib/idlelib/Itest/init.py adding Lib/idlelib/Itest/test_calltips.py adding Lib/idlelib/Itest/test_pathbrowser.py adding Lib/test/test_idle.py abort: patch failed to apply

I even tried using the tr command to remove the ^M characters from the .diff file and that still didn't allow me to apply the patch. Maybe it is my setup??? It is late here so I am going to bed but will play with it some more tomorrow. Thanks for your hard work, the patch looks like a good start. Thanks!

[ezio-melotti]

Have you tried applying it to 3.3?

[terryjreedy]

Todd, the last two commits, both rather trivial, I merged from 3.3 to 3.4 would not apply for no reason I could see. I wonder is there is something wrong with the repository. I wrote about the problem on the committer's list a few days ago, but got no response yet. It probably fell thru the cracks. I will try applying to 3.4 on windows tomorrow.

[ezio-melotti]

I wonder is there is something wrong with the repository.

The repo looks ok to me. You could try to run "hg verify" on your machine id you want to make sure your clone is ok.

I wrote about the problem on the committer's list a few days ago, but got no response yet. It probably fell thru the cracks.

I've seen the mail but it's hard to tell what the problem was after the fact. Next time it happens I suggest you to come on #python-dev and ask there before continuing.

[ncoghlan]

Just a quick note on the directory naming: idlelib/idle_test sounds like a good option to me.

Putting it under idlelib makes it easy for distros to carve it out along with the rest of Idle, as well as making it clear that it falls under the purview of PEP-434

Expanding Itest -> idle_test is a combination of "Itest" being somewhat cryptic in the first place, and then the potential for 1/I/l/i glyph confusion (depending on font) making it worse.

Other than that, +1 to what Ezio said (I wouldn't worry about moving any more existing tests for this patch though - starting to move self-tests out of the individual IDLE files to the new test suite can be a good next step)

[terryjreedy]

Ezio, thank you for the response.

1. I care more about where the directory of tests is than what it is called. 'idle_test' or 'idletest' or whatever would be fine. Lets make that the last bikeshed item ;-).

2. I looked as bpo-10572. It was originally about moving unittest tests from unittest/xx to test/yy, and then broadened to other packages. Michael F. gave two reasons for the move.

2A. Easier to grep without recursing into the test directory. Barry already refuted this, pointing out that grep/find have options to not recurse. The Idle equivalent, Find in Files, has a little checkbox [] Recurse down subdirectories. I think Michael had this point backwards. The advantage of having tests with the code is to make grepping both at once possible. I am sure I will want to do this while developing the tests. So add this to my list of reasons to put the directory under idlelib.

2B. Would honor the Windows Install without Tests option. This is strictly a matter of saving space by disabling function. I agree that this is appropriate for nearly all packages, but I already explained why I want to evade that option: tkinter and idle are exceptional in that they are the only two packages that people regularly have problems running. So I think people should have those tests available, even if none of the others.

2 (again). Other people opined that package maintainers should have some say in the matter and that there might be exceptions.

3. Many modules have more or useless 'sanity-check' tests run with the 'if __name__' mechanism. I think *all* of these should be replaced with unittest.main(xxx, verbosity=2, exit=False) to run the full test module. If the code module test has anything valuable that is not in the test module, it should be moved there; the rest should be deleted.

I plan to eventually do this with all such 'tests' in idlelib/*.py. However, this needs to be done one by one. Some current tests have program errors; my 'file' to 'open' patch last week was the first fix. Some put up a blank window, with no indication of what a person is expected to do to perform the test. Some put up a window that will not close by normal means. All except the two in idletasks.diff require human intervention, which is not acceptible for buildbots. Question: is there a way for a test to detect being run by a buildbot?

4. I already used @template.txt to alter CallTips.py and start test_calltips.py and I want to use it for all the other 60 (or whatever) test files. I will (re)read the test-writing section and possibly suggest a patch. But Idle-specific info and code does not belong there.

(5.) unittest.SkipTest is what I was looking for. I do not believe that support.import_module() will allow skipping if, or unless, the test is running on a specific platform. I need to look at bpo-16935.

Is Idle CPython only? If so, that should be a condition of test/test_idle.py running. Ah, testing the imports of tkinter and idlelib already takes care of that.

(6.) My concern and possible confusion about raising exceptions came from Jayakrishnan's patch + def test_DirBrowserTreeItem(self): + # bpo-16226 - make sure that getting a sublist works + d = PathBrowser.DirBrowserTreeItem('') + d.GetSubList() in relation to the doc saying "Note that in order to test something, we use one of the assert*() methods provided by the TestCase base class." There is no such method. The doc follows with "If the test fails, an exception will be raised, and unittest will identify the test case as a failure. Any other exceptions will be treated as errors." The 'test above is to raise, or not, an 'other exception' and I took 'treated as errors' to mean that raising an 'other exception' is an error. It certainly makes test failure counted as an error rather than an error and indistinguishable from test code errors. I guess when the test is fleshed out, there should and will be some assert about the result of d.getsublist(). So I could delete the try-except.

(7.) A human-intervention resource. I will leave that for later. My next step after this one is moving tests from CallTips.py to the new test_calltips.py (3. above).

[terryjreedy]

For future reference, there is an idle specific issue that I do not think has been mentioned yet. Idle can run in one process, but the default mode now, and perhaps only mode sometime in the future, is two processes communicating through a socket. Testing two process communication requires two test processes, not just one. Tests will pass in one process without testing the full normal process. For instance:

    def fetch_tip(self, expression):
        try:
            rpcclt = self.editwin.flist.pyshell.interp.rpcclt
        except AttributeError:
            rpcclt = None
        if rpcclt:
            return rpcclt.remotecall("exec", "get_the_calltip",
                                     (expression,), {})
        else:
            return get_argspec(get_entity(expression))

I believe this works because fetch_tip executed in the idle process makes a remote call that results in fetch_tip being executed in the remote process, where self has no editwin and hence control falls through to the last line.

A normal, naive test of fetch_tip will 'work' because it will simply fall through to the last line, which in this case is mostly redundant with separate tests of get_entity and get_argspec. It seems to me that a full test suite must at some point test that the actual communication works, and that this cannot all be done with mocks.

[terryjreedy]

Thanks Nick. Tomorrow I should change the directory name, delete try-except, commit, and move onward on the path laid out.

[ezio-melotti]

Having idle_test under idlelib is fine with me if it makes your life easier.

3. Many modules have more or useless 'sanity-check' tests run with the 'if __name__' mechanism. I think *all* of these should be replaced with unittest.main(xxx, verbosity=2, exit=False) to run the full test module. If the code module test has anything valuable that is not in the test module, it should be moved there; the rest should be deleted.

All the sanity checks (at least the reasonably sane) should be converted to unittest and moved from the modules to the respective test* modules. The others should be removed, and I don't think we have to keep the "if \_name__" in the modules (the test modules should have it though). Specifying the verbosity=2 in all the test modules might not be a good idea. If you are running all the tests at once, that level of verbosity will just get in the way (and I'm not sure you can change it easily). If you are running individual tests modules you should be able to change the verbosity from the command line.

Question: is there a way for a test to detect being run by a buildbot?

You can handle this by adding a new resource to test.support. The tests will normally be skipped unless you explicitly run them using "./python -m test -uhumanneeded" or you run the test file directly (you can enable the resource in the "if __name__").

4. I already used @template.txt to alter CallTips.py and start test_calltips.py and I want to use it for all the other 60 (or whatever) test files. I will (re)read the test-writing section and possibly suggest a patch.

As I suggested earlier, I think the "main" function in Lib/idlelib/CallTips.py should be converted to unittest and moved to testcalltips. CallTips.py should not include any test-related code and no "if \_name__" IMHO.

But Idle-specific info and code does not belong there.

Agreed. Actually once the initial conversion is done I don't think documenting it so important, since it will be easier to look at all the others tests and see what they do than finding the documentation that explain how things should be done.

(5.) unittest.SkipTest is what I was looking for. I do not believe that support.import_module() will allow skipping if, or unless, the test is running on a specific platform. I need to look at bpo-16935.

Nope, but you can do "if sys.platform() == '...': raise unittest.SkipTest(...)". The TL;DR version of bpo-16935 is that it will break test discovery on 3.3, but that's not a big problem.

(6.) My concern and possible confusion about raising exceptions came from Jayakrishnan's patch

Either if you get a failure or an error, it still means there's something wrong :)

[terryjreedy]

Tests have at least two very different purposes. One is test-driven development of code (and tests) by developers. The other is regression detection by buildbots. "if __name" in code modules, in addition to test modules, makes the first much easier. First, the unittest.main call in the test module must be appropriate for the buildbots. Since buildbots do not execute the corresponding call in the code module, it can and and should tuned for development, which I have done. The 'if' block is also a place for any other code specific to developer tests, such as enabling a 'humanneeded' resource. Second, when editing with Idle, F5 in an editor window runs the test in the Idle shell, where right-click, click on a traceback line takes one back to the corresponding file and line. At least on Windows, using the console and console interpreter is painful by comparison. All this is true when editing any Python file, not just Idle files, so I would be disappointed if someone went through the stdlib deleting, rather than revising the 'if __name' blocks.

[ncoghlan]

Terry, the unittest and regrtest command lines are *built* for TDD - you rely on test discovery and selection to run the appropriate tests for what you're working on.

However, the point about F5 in IDLE is well made, and a reasonable rationale for ensuring at least the IDLE implementation files support running their tests that way.

While it doesn't need to be extensive, it would be good to capture some of these points in a Idle/idle_tests/README file.

[terryjreedy]

New patch: I renamed Itest to idle_test everywhere and re-ran tests; removed try-except from test_pathbrowser.py; and renamed @template to @README and rewrote. It applies cleanly to 3.4 on my system. The only problem applying to 2.7 is CallTips.py, which has different test code at the end. That will be easily fixed.

[a2c68251-5f3b-4972-91b1-b78401c27569]

I still have the same problem with the patch it will not apply for me on Python 3.4. Based on Ezio's suggestion I used hg verify where I got three warnings unrelated to IDLE, but just to make sure I did a brand new checkout. Even after a new checkout the patch failed to apply. Then I tried on my CentOS 6.4 box and the patch would not apply. I am wondering has anybody else tried to apply this patch on a Mac OS X or Linux machine and had it work? I still admit I could be doing something stupid.....

No idea where to go from here...I might try this on Windows which I think is the system Terry is using because I noticed the file has Ctrl M at the end of the lines.

[bitdancer]

I just did an hg pull/hg up in my 3.4 (default) checkout on linux:

rdmurray@hey:~/python/p34>patch -p1 \<idletest2.diff (Stripping trailing CRs from patch; use --binary to disable.) patching file Lib/idlelib/CallTips.py (Stripping trailing CRs from patch; use --binary to disable.) patching file Lib/idlelib/PathBrowser.py (Stripping trailing CRs from patch; use --binary to disable.) patching file Lib/idlelib/idle_test/@README.txt (Stripping trailing CRs from patch; use --binary to disable.) patching file Lib/idlelib/idle_test/init.py (Stripping trailing CRs from patch; use --binary to disable.) patching file Lib/idlelib/idle_test/test_calltips.py (Stripping trailing CRs from patch; use --binary to disable.) patching file Lib/idlelib/idle_test/test_pathbrowser.py (Stripping trailing CRs from patch; use --binary to disable.) patching file Lib/test/test_idle.py

However, when I try to run test_idle via regrtest I get:

... File "/home/rdmurray/python/p34/Lib/unittest/loader.py", line 113, in loadTestsFromName parent, obj = obj, getattr(obj, part)

AttributeError: 'module' object has no attribute 'test_idle'

[terryjreedy]

I was really sure that 'python_d -m test' worked a week ago, in the sense that test_idle was run without incident in its proper alphabetical sequence, before I said so and uploaded the patch. Now, there are (different) error messages with both 'python_d -m test' and 'python_d -m test test_idle'. (I did not try the latter before, only 'python_d -m test.test_idle', with the added '.' whose importance I now understand.) One of the two error messages includes what David reported. When I can, will report the other, and also will try to 'hg update' to a week ago to reproduce the remembered success. If I cannot, I will try options to determine the boundaries of the bug in the interacton between unittest and regrtest and a decent workaround that avoids duplicating code while running under both unittest and regrtest.

I sent Todd the Windows files to examine, modify, and test.

(Nick: neither unittest nor regrtest can run tests hidden within patches on the tracker. However, the point is currently moot for this issue. It might someday be a python-dev or python-ideas post.)

[terryjreedy]

Problem solved at least for me with a new patch:

D:\Python\dev\py33\PCbuild> python_d -m test test_idle [1/1] test_idle Warning -- os.environ was modified by test_idle 1 test altered the execution environment: test_idle

...> python_d -m test ... [154/373/2] test_httpservers [155/373/2] test_idle [156/373/2] test_imaplib ...

The new patch has two simple changes to test_idle.

1. The traceback David and I saw started in regrtest.runtest_inner: thepackage = \_import__(abstest, globals(), locals(), []) The rest of the calls were in unittest. This line runs fine in isolation, but it does not matter; regrtest does not want the test to run when it imports the file. Add 'if name ...' to guard main(...).

2. With no test_main present, the key line of runtest_inner is tests = unittest.TestLoader().loadTestsFromModule(the_module) Unittest.main makes the same call to look for, among other things, a load_tests function. Import load_tests from idlelib.idle_test.

[terryjreedy]

I will delete 'sim' as an abbreviation for 'support import module'. Does this otherwise seem ready to apply? This time I am really sure it works here, because I ran the 2 tests twice each and cut and pasted the evidence.

[04f55c86-88d8-4577-90c1-91a8cc46fe8a]

I'm having the same problem as Todd when I apply the patch in my Mac... I have no idea why though.

[terryjreedy]

The change to the two idlelib/.py files enables those files to run their corresponding idletest/test.py file when run as a main script. They have nothing to do with running test/test_idle.py. So please try running the latter. python -m test.test_idle # direct, without regrtest python -m test test_idle # indirect, with regrtest

I upgraded(?) TortoiseHG (2.8, with hg 2.6) and found the setting for Notepad++ to create new files in 'unix/osx' format, which I presume refers to line endings. Does the new file apply better for you? If so, I will try to always use this for uploaded patches.

--- Question: when I import this patch (either format) to 3.4, it applies the chunks and makes the changes with no apparent problem, but then 'aborts' -- but not really, because it leaves the changes: abort: edit failed: vi exited with status 1 [command returned code 255 Mon May 20 23:08:16 2013] The appear to be a failure of the commit message editor? If so, it is tolerable, but it worked a week ago (regression in upgrade?), though i have seen it before. Manual says

"If the patch you are importing does not have a commit message, Mercurial will try to launch your editor, just as if you had tried to import the patch from the command line. Your ui.editor needs to be a GUI app to make this work correctly."

I get same message after setting editor to notepad or notepad++, so 'vi' mystifies me. Anyone have any idea? ----

[04f55c86-88d8-4577-90c1-91a8cc46fe8a]

Now I can apply the patch successfully and everything seems to be working. Thanks, Terry.

[bitdancer]

regrtest now works for me, as does running test_idle.py directly and the simple minded unittest call:

./python -m unittest test.test_idle

However, running an individual test doesn't. I don't see this as a show-stopper for committing this, but rather something we should figure out how to fix later.

rdmurray@hey:~/python/p34>./python -m unittest test.test_idle ... ---------------------------------------------------------------------- Ran 3 tests in 0.003s

OK rdmurray@hey:~/python/p34>./python -m unittest -v test.test_idle test_bad_entity (idlelib.idle_test.test_calltips.Test_get_entity) ... ok test_good_entity (idlelib.idle_test.test_calltips.Test_get_entity) ... ok test_DirBrowserTreeItem (idlelib.idle_test.test_pathbrowser.PathBrowserTest) ... ok

---------------------------------------------------------------------- Ran 3 tests in 0.004s

OK
rdmurray@hey:~/python/p34>./python -m unittest -v test.test_idle.idlelib.idle_test.test_calltips.Test_get_entity.test_bad_entity
Traceback (most recent call last):
  File "/home/rdmurray/python/p34/Lib/runpy.py", line 160, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "/home/rdmurray/python/p34/Lib/runpy.py", line 73, in _run_code
    exec(code, run_globals)
  File "/home/rdmurray/python/p34/Lib/unittest/__main__.py", line 19, in <module>
    main(module=None)
  File "/home/rdmurray/python/p34/Lib/unittest/main.py", line 124, in __init__
    self.parseArgs(argv)
  File "/home/rdmurray/python/p34/Lib/unittest/main.py", line 171, in parseArgs
    self.createTests()
  File "/home/rdmurray/python/p34/Lib/unittest/main.py", line 178, in createTests
    self.module)
  File "/home/rdmurray/python/p34/Lib/unittest/loader.py", line 145, in loadTestsFromNames
    suites = [self.loadTestsFromName(name, module) for name in names]
  File "/home/rdmurray/python/p34/Lib/unittest/loader.py", line 145, in <listcomp>
    suites = [self.loadTestsFromName(name, module) for name in names]
  File "/home/rdmurray/python/p34/Lib/unittest/loader.py", line 113, in loadTestsFromName
    parent, obj = obj, getattr(obj, part)
AttributeError: 'module' object has no attribute 'idlelib'

[terryjreedy]

It works when one uses the right dotted name ;-)

D:\Python\dev\py33\PCbuild>python_d -m unittest -v idlelib.idle_test.test_calltips.Test_get_entity.test_bad_entity test_bad_entity (idlelib.idle_test.test_calltips.Test_get_entity) ... ok ---------------------------------------------------------------------- Ran 1 test in 0.000s OK

idlelib.idle_test.test_calltips.Test_get_entity works too. I did not know about these options; I added them to @README as part of revising it. I also added verbosity and exit args to all if-name unittest.main calls, which are ignored anyway when either unittest or regrtest import the modules. New patch uploaded.

With this additional confirmation, I am about ready to commit -- when I feel fresh and ready to monitor the buildbots. But I notice that the non-executable @README has 7 ways to run all or part of the suite, most of which have appeared in this issue. Even with history retrieval, I am tired of retyping to test changes. I should have started with an executable test suite test (.bat or .py using subproccess for the command lines). Then I could have just added the two cases above and re-run after today's edit. I may do this first.

[terryjreedy]

Attached file tests framework by running idle_tests 6 different ways. Run with the executable that you want to run the tests with as is uses sys.executable. I plan to move it into idle_tests.

[a2c68251-5f3b-4972-91b1-b78401c27569]

Patch does indeed apply and I get good results!!!!! The patch is well done and provides a nice example on how to write unit tests. +1 for making the commit from me

R. David Murray you used the patch command while I used "hg import --no-commit mywork.patch" as specified in the Python Developers Guide. Next time I have an issue I will use patch and see if it works better.

[bitdancer]

Heh. Yeah, I use patch because I don't just work with mercurial/python, and I find the patch command simpler to use for applying patches in general, since I never want an autocommit. (The exception would be if I'm applying a patch that involves extended diff stuff that patch doesn't recognize.) It makes sense that the devguide talks about import, though, since the patches here always ought to be things generated by hg and thus handleable by import. I'm not sure why this one would have failed for you.

[1762cc99-3127-4a62-9baf-30c3d0f51ef7]

New changeset 24c3e7e08168 by Terry Jan Reedy in branch '3.3': Issue bpo-15392: Create a unittest framework for IDLE. http://hg.python.org/cpython/rev/24c3e7e08168

[terryjreedy]

Before committing, I experimented with disabling test/test_idle. The simple and safe method is to comment out the 'load_tests' line. With no tests discovered, all pass ;-). Without verbosity, there is no indication that there were none. A little harder, and needing to be tested for typos before committing, is to add this line before that line: import unittest; raise unittest.SkipTest('skip for buildbots') Regrtest (py -m test test_idle) registers the unexpected skip. Unittest (py -m test.test_idle) exits with a traceback. This is recorded in an updated README, which also incorporates Todd's suggestion. I also touched up test/test_idle.py.

If buildbots do not break, I will work on 2.7.