New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create a unittest framework for IDLE #59597
Comments
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.) |
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. |
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/test_tk 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? 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. |
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. |
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. |
bpo-7883 also has a test. |
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. |
I'll start with a bit of philosophical guidance :)
|
I'm a student thinking of participating in Google Summer of Code. Using unittest.mock seemed to be good way to test GUI. I think using third party mock seemed to be ok, but not best way. Of cource, I would forcus on non-GUI unittest. |
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. |
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 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 |
Oh, no support for Python2? By the way, your proposal seemed good to me. But I want to include Python2 and GUI support. Of cource, it is very import to concentrate on one thing (i.e Python3). I considered PEP-434 premit me writing unittest for Python2. How do you think? |
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 :) |
When I opened this last July, I intentionally left off 2.7 for multiple reasons.
(I plan to start with this issue when my development machine is back from repairs and proper set up again.) |
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. 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. |
+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:
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. |
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. |
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 ---------------------------------------------------------------------- OK |
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 :) |
Attached is a 3.3 patch that I *believe* is ready to commit and push. 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 and revises 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.)
Other changes from the previous patch:
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. |
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? |
Yes, of course. Thanks for correcting. 434, 434, 434,... |
A few comments about the patch:
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 idle_test can be run on their own, and they should use the "if __name__ == '__main__': unittest.main()" idiom.
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).
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 :)
If you still want to keep the try/except, I would provide a meaningful failure message in any case.
It's not difficult to break the code to test that test works though :)
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. |
Terry, rovitotv-pc:py34 rovitotv$ hg import --no-commit /Volumes/SecurePython3/source/15392idletests.diff 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! |
Have you tried applying it to 3.3? |
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. |
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. |
Terry, the unittest and regrtest command lines are *built* for TDD - you However, the point about F5 in IDLE is well made, and a reasonable While it doesn't need to be extensive, it would be good to capture some of |
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. |
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. |
I just did an hg pull/hg up in my 3.4 (default) checkout on linux: rdmurray@hey:~/python/p34>patch -p1 <idletest2.diff However, when I try to run test_idle via regrtest I get: ... AttributeError: 'module' object has no attribute 'test_idle' |
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.) |
Problem solved at least for me with a new patch: D:\Python\dev\py33\PCbuild> python_d -m test test_idle ...> python_d -m test The new patch has two simple changes to test_idle.
|
I will delete 'sim' as an abbreviation for 'support import module'. |
I'm having the same problem as Todd when I apply the patch in my Mac... I have no idea why though. |
The change to the two idlelib/.py files enables those files to run their corresponding idle_test/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. 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. --- "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? |
Now I can apply the patch successfully and everything seems to be working. Thanks, Terry. |
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 OK ---------------------------------------------------------------------- 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' |
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 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. |
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. |
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. 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. |
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. |
New changeset 24c3e7e08168 by Terry Jan Reedy in branch '3.3': |
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: If buildbots do not break, I will work on 2.7. |
Last I checked, test_idle passes on the stable buildbots. But it fails on a machine without threads, such as http://buildbot.python.org/all/builders /AMD64%20Fedora%20without%20threads%203.3/builds/752/steps/test/logs/stdio The chain of imports is test_pathbrowser <- PathBrowser <- ClassBrowser <- PyShell <- threading <- _thread. Since a PyShell import is required to run Idle (PyShell.main), even with just the editor, I will put the _thread import check in test_idle itself rather than sprinkling it throughout the test suite as dependencies are discovered. (Besides the bother of the latter, the dependency could go away if PyShell only uses threading to runs user code in the same process, or delayed if it is used for other features that might not be used.) |
Building without threads is generally going to be for embedded systems without a GUI anyway, so I think it's fine to just skip the entire IDLE test suite when real threads aren't available. |
New changeset 968f6094788b by Terry Jan Reedy in branch '3.3': |
New changeset 93eb15779050 by Terry Jan Reedy in branch '2.7': |
Biggest change is no support module in 2.7, so in test_idle.py import inside try:except to skip if dependencies not present. Minor changes in CallTips.py and test_calltips.py. Buildbots are fine. This meets my initial goal; issue done. I opened bpo-18103 for consideration of gui tests. But except for improving the existing no-buildbot, human-needed tests, I consider that a lower priority right now. |
The test.support module was renamed in Python 3 from test.test_support in Python 2. While the 3x support has expanded and diverged somewhat, with a bit of try hacking it should be possible to minimize the source differences between the 2.7 and 3.x tests. |
For the other modules we just use test.test_support on 2.x and test.support in 3.x, without using try/except and without trying to maintain source compatibility with both. You might get a merge conflict every once in a while, but I don't think that's a big deal. |
New changeset 6159916c712e by Terry Jan Reedy in branch '2.7': |
New changeset 16fea8b0f8c4 by Terry Jan Reedy in branch '3.3': |
New changeset dac6aea39814 by Ned Deily in branch '2.7': New changeset e52dad892521 by Ned Deily in branch '3.3': New changeset 06239fe781fe by Ned Deily in branch 'default': |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: