Skip to content
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

Run Tkinter tests with wantobjects=False #65784

Open
serhiy-storchaka opened this issue May 26, 2014 · 11 comments
Open

Run Tkinter tests with wantobjects=False #65784

serhiy-storchaka opened this issue May 26, 2014 · 11 comments
Assignees
Labels
tests Tests in the Lib/test dir topic-tkinter type-feature A feature request or enhancement

Comments

@serhiy-storchaka
Copy link
Member

BPO 21585
Nosy @terryjreedy, @zware, @serhiy-storchaka
Dependencies
  • bpo-22236: Do not use _default_root in Tkinter tests
  • Files
  • test_wantobj.patch
  • test_ttk_guionly_wantobj.py
  • wantobj_test.patch
  • 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 = 'https://github.com/serhiy-storchaka'
    closed_at = None
    created_at = <Date 2014-05-26.20:53:59.354>
    labels = ['tests', 'type-feature', 'expert-tkinter']
    title = 'Run Tkinter tests with wantobjects=False'
    updated_at = <Date 2014-08-23.19:49:02.040>
    user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2014-08-23.19:49:02.040>
    actor = 'zach.ware'
    assignee = 'serhiy.storchaka'
    closed = False
    closed_date = None
    closer = None
    components = ['Tests', 'Tkinter']
    creation = <Date 2014-05-26.20:53:59.354>
    creator = 'serhiy.storchaka'
    dependencies = ['22236']
    files = ['35394', '35395', '36291']
    hgrepos = []
    issue_num = 21585
    keywords = ['patch', 'needs review']
    message_count = 11.0
    messages = ['219178', '219322', '219323', '224001', '224962', '225478', '225617', '225635', '225642', '225746', '225759']
    nosy_count = 6.0
    nosy_names = ['terry.reedy', 'gpolo', 'jesstess', 'zach.ware', 'serhiy.storchaka', 'Lita.Cho']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = 'needs patch'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue21585'
    versions = ['Python 3.5']

    @serhiy-storchaka
    Copy link
    Member Author

    The wantobjects attribute of the tkinter module is True by default, and all Tkinter tests run with wantobjects=True. We need to test Tkinter with wantobjects=False too. All Tkinter tests should be ran twice, in both mode.

    @serhiy-storchaka serhiy-storchaka added tests Tests in the Lib/test dir topic-tkinter type-feature A feature request or enhancement labels May 26, 2014
    @LitaCho
    Copy link
    Mannequin

    LitaCho mannequin commented May 28, 2014

    So I don't know what the best way to do this, but I changed the widget_tests.py in order to set tkinter.wantobjects = 0 if a 'wantobjects' flag was passed through test.support.use_resources.

    Then I added a new test called test_ttk_guionly_wantobj, where it turns on wantobjects. If we create a new module, then we start off with a clean tkinter object.

    Before I go this route for all the tests, I wanted to make sure if this was the correct way to go about it.

    @LitaCho
    Copy link
    Mannequin

    LitaCho mannequin commented May 28, 2014

    Patches lived in my Linux machine. I've attached my patch. I will add my module next.

    @serhiy-storchaka
    Copy link
    Member Author

    Thanks Lita, but I think it would be better if all Tkinter tests will automatically run in both wantobjects=0 and wantobjects=1 modes.

    As for your patch, ttk.Button() creates default root with current wantobjects value, this default root can be unintentionally used in tests with other wantobjects value. We should ensure that default root is not used in tests.

    @serhiy-storchaka serhiy-storchaka self-assigned this Aug 6, 2014
    @LitaCho
    Copy link
    Mannequin

    LitaCho mannequin commented Aug 6, 2014

    Hi Serhiy,

    This patch was made while I was learning tkinter. I figured out how to run the tests twice while changing wantobjects variable without creating new tests. Fortunately, all the tests seem to fast when wantobjects is 0 or 1.

    The only annoying thing is that it doesn't merge the count of the amount of tests ran within that module, since I am calling support.run_unittest twice. Hopefully, that's okay. Otherwise, I need to figure out how to call the tests twice individually within the generator.

    @terryjreedy
    Copy link
    Member

    My impression is that we are trying to move away from using test_main, or is using support.run_unittest considered a sufficient change from the old explicit suite method?

    Anyway, here is an alternate approach, adding two lines to wrap all test classes, that tags the test classes in the code and hence in error messages.

    import unittest
    for i in range(2): exec('''
    class T{0}(unittest.TestCase):
        def test_2(self):
            self.assertTrue({0} == 1)
    '''.format(i))
    unittest.main()
    >>>
    F.

    ======================================================================
    FAIL: test_2 (main.T0)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "<string>", line 4, in test_2
    AssertionError: False is not true

    Ran 2 tests in 0.016s

    @LitaCho
    Copy link
    Mannequin

    LitaCho mannequin commented Aug 21, 2014

    Hi Terry,

    I had no idea we were moving away from using test_main.

    So instead, of using support.run_unittest, we should import all the unittest from tkinter/test/ and wrap everything with that exec method, setting wantobjects=1 and again with wantobjects=0?

    Also, do you have an example of a unit test that doesn't use test_main? All the unit tests I've seen (tkinter, smtplib, nntplib) all use test_main.

    Lita

    @terryjreedy
    Copy link
    Member

    The current test/test_*.py example file in the docs intentionally does not contain test_main.
    https://docs.python.org/3/library/test.html#writing-unit-tests-for-the-test-package
    The next section explaining that test.regrtest runs unittest.TestLoader.loadTestsFromModule if test_main does not exist. I believed support.run_unittest.

    I never used test_main in test_idle.py, Grep test/test_*.py for 'test_main' or its absence to count others. Or search all issues for 'unittest discovery' for conversions.  For instance: python/cpython#62458 made this change to the five test_codecmaps*.py files.
    http://bugs.python.org/file30644/test_codecmaps_discovery.diff
    -def test_main():
    -    support.run_unittest(__name__)
    -
     if __name__ == "__main__":
    -    test_main()
    +    support.use_resources = ['urlfetch']
    +    unittest.main()

    This all said, running all tests in a file with two different values of an attribute of the tested module is an exceptional case. Zach, what do you think?

    @zware
    Copy link
    Member

    zware commented Aug 22, 2014

    I've been thinking on this for a while, and haven't come up with a solution that I like. I haven't had a chance to look at bpo-22236 yet, but I suspect it will help immensely.

    Also, I have a work-in-progress patch for rearranging the tkinter tests into a test.test_tkinter subpackage. At this point, I'm not sure if that will make this issue easier or harder to resolve, but it does take care of not using test_main for the tkinter tests. I'm hoping to have some time to get that patch into a submittable state tomorrow, then have a good look at how it, bpo-22236, and this issue interact. Anyone who wants a look at where I'm going with the test_tkinter subpackage before I get it submitted, it's at http://hg.python.org/sandbox/zware/log/mv_pkg_tests/ (see the changes by doing "hg diff --git -r default -r mv_pkg_tests Lib/test/test_tkinter"; there are changes to other test packages in that branch as well that are missed by specifying Lib/test/test_tkinter).

    @serhiy-storchaka
    Copy link
    Member Author

    Here is my plan. First, we should resolve bpo-22236. Otherwise some tests will be run with default root created with old "wantobjects" value. Then we should introduce new test resource name as in Lilo's first patch (but "wantobjects" is not correct name, "testallwantobjectsvalues" is more correct but is too long). Then we should add the load_tests() function in every test file which will generate new test classes for every wantobjects value (0, 1 and 2 if bpo-22214 will be applied) when this resource is enabled.

    Any propositions about new resource name?

    As for rearranging the tkinter tests, lets discuss this in separate issue. It looks orthogonal to this issue.

    @zware
    Copy link
    Member

    zware commented Aug 23, 2014

    I agree that bpo-22236 should be resolved first. As for resource guarding the 'wantobjects = 0' tests, do we really need to? It takes a bit under 20 seconds to run all of the tkinter tests on this machine, and I don't think 40 seconds is an unduly long runtime for such a large set of tests, so I would vote for just loading both sets of tests every time. Or if others feel the 'wantobjects' test should be guarded, I think we could just reuse the 'cpu' resource (since it's really just time that we're guarding).

    And the rearrangement will of course be a separate issue; I only mentioned it here because that patch takes care of moving away from test_main and support.run_unittest, and may have an impact on how this issue can be resolved.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    tests Tests in the Lib/test dir topic-tkinter type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants