Skip to content

bpo-30870: IDLE: Add configdialog fontlist unittest #2666

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.

Sign up for GitHub

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

Merged
terryjreedy merged 12 commits into from
Jul 14, 2017
Merged

bpo-30870: IDLE: Add configdialog fontlist unittest #2666

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
ff8ab19
bpo-30870: IDLE: Add configdialog fontlist unittest
mlouielu Jul 11, 2017
bd1c784
Add focus_force to fontlist to prevent unittest failed
mlouielu Jul 12, 2017
5f8df49
revise tests
terryjreedy Jul 13, 2017
255fd75
Update test_configdialog.py
terryjreedy Jul 13, 2017
e676416
Update test_configdialog.py
terryjreedy Jul 13, 2017
65b9c25
Fix nit coding style
mlouielu Jul 13, 2017
4ed55c9
Fix misused of self in classmethod
mlouielu Jul 13, 2017
331354d
Fix comment
mlouielu Jul 13, 2017
961cc3d
Change comment to docstring in two test cases
mlouielu Jul 13, 2017
f43d396
Update test_configdialog.py
terryjreedy Jul 13, 2017
6f58026
Change x and y represent
mlouielu Jul 13, 2017
ec67c26
Revert changes for test cases' docstring
mlouielu Jul 13, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions Lib/idlelib/configdialog.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ def __init__(self, parent, title='', _htest=False, _utest=False):
self.parent = parent
if _htest:
parent.instance_dict = {}
self.withdraw()
if not _utest:
self.withdraw()

self.configure(borderwidth=5)
self.title(title or 'IDLE Preferences')
Expand Down Expand Up @@ -76,7 +77,6 @@ def __init__(self, parent, title='', _htest=False, _utest=False):
self.create_widgets()
self.resizable(height=FALSE, width=FALSE)
self.transient(parent)
self.grab_set()
self.protocol("WM_DELETE_WINDOW", self.cancel)
self.fontlist.focus_set()
# XXX Decide whether to keep or delete these key bindings.
Expand All @@ -88,6 +88,7 @@ def __init__(self, parent, title='', _htest=False, _utest=False):
self.attach_var_callbacks() # Avoid callbacks during load_configs.

if not _utest:
self.grab_set()

@terryjreedy terryjreedy Jul 11, 2017 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe at least some other dialogs have grab_set here already. Is this move also needed for event_generate?

@mlouielu mlouielu Jul 11, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only saw it at textview that control with model argument. grab_set somehow will affect event_generate, too.

@mlouielu mlouielu Jul 11, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test_textview mock out the grab_set, hmm. Should we mock out, too?

@terryjreedy terryjreedy Jul 11, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it will not be called, no need. I just read the UNM Shipman doc "Widget w grabs all events for w's application. If there was another grab in force, it goes away. See Section 54, “Events” for a discussion of grabs." Seems to be related to dialog being modal. Did not see anything in Sect.54 though. I need to make sure I know about all the calls in various inits.

self.wm_deiconify()
self.wait_window()

Expand Down
19 changes: 10 additions & 9 deletions Lib/idlelib/idle_test/mock_idle.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,25 @@
from idlelib.idle_test.mock_tk import Text

class Func:
'''Mock function captures args and returns result set by test.
'''Record call, capture args, return/raise result set by test.

Attributes:
self.called - records call even if no args, kwds passed.
self.result - set by init, returned by call.
self.args - captures positional arguments.
self.kwds - captures keyword arguments.
When mock function is called, set or use attributes:
self.called - increment call number even if no args, kwds passed.
self.args - capture positional arguments.
self.kwds - capture keyword arguments.
self.result - return or raise value set in __init__.

Most common use will probably be to mock methods.
Most common use will probably be to mock instance methods.
Given class instance, can set and delete as instance attribute.
Mock_tk.Var and Mbox_func are special variants of this.
'''
def __init__(self, result=None):
self.called = False
self.called = 0
self.result = result
self.args = None
self.kwds = None
def __call__(self, *args, **kwds):
self.called = True
self.called += 1
self.args = args
self.kwds = kwds
if isinstance(self.result, BaseException):
Expand Down
111 changes: 93 additions & 18 deletions Lib/idlelib/idle_test/test_configdialog.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
"""Test idlelib.configdialog.

Half the class creates dialog, half works with user customizations.
Coverage: 46% just by creating dialog, 56% with current tests.
Coverage: 46% just by creating dialog, 60% with current tests.
"""
from idlelib.configdialog import ConfigDialog, idleConf, changes
from test.support import requires
requires('gui')
from tkinter import Tk
import unittest
import idlelib.config as config
from idlelib.idle_test.mock_idle import Func

# Tests should not depend on fortuitous user configurations.
# They must not affect actual user .cfg files.
Expand All @@ -22,27 +23,29 @@
}

root = None
configure = None
dialog = None
mainpage = changes['main']
highpage = changes['highlight']
keyspage = changes['keys']

class TestDialog(ConfigDialog): pass # Delete?

class TestDialog(ConfigDialog):
pass # Delete?


def setUpModule():
global root, configure
global root, dialog
idleConf.userCfg = testcfg
root = Tk()
root.withdraw()
configure = TestDialog(root, 'Test', _utest=True)
# root.withdraw() # Comment out, see issue 30870
dialog = TestDialog(root, 'Test', _utest=True)


def tearDownModule():
global root, configure
global root, dialog
idleConf.userCfg = usercfg
configure.remove_var_callbacks()
del configure
dialog.remove_var_callbacks()
del dialog
root.update_idletasks()
root.destroy()
del root
Expand All @@ -58,31 +61,103 @@ def test_font(self):
default_font = idleConf.GetFont(root, 'main', 'EditorWindow')
default_size = str(default_font[1])
default_bold = default_font[2] == 'bold'
configure.font_name.set('Test Font')
dialog.font_name.set('Test Font')
expected = {'EditorWindow': {'font': 'Test Font',
'font-size': default_size,
'font-bold': str(default_bold)}}
self.assertEqual(mainpage, expected)
changes.clear()
configure.font_size.set(20)
dialog.font_size.set(20)
expected = {'EditorWindow': {'font': 'Test Font',
'font-size': '20',
'font-bold': str(default_bold)}}
self.assertEqual(mainpage, expected)
changes.clear()
configure.font_bold.set(not default_bold)
dialog.font_bold.set(not default_bold)
expected = {'EditorWindow': {'font': 'Test Font',
'font-size': '20',
'font-bold': str(not default_bold)}}
self.assertEqual(mainpage, expected)

#def test_sample(self): pass # TODO
def test_set_sample(self):
# Set_font_sample also sets highlight_sample.
pass

def test_tabspace(self):
configure.space_num.set(6)
dialog.space_num.set(6)
self.assertEqual(mainpage, {'Indent': {'num-spaces': '6'}})


class FontSelectTest(unittest.TestCase):
# These two functions test that selecting a new font in the
# list of fonts changes font_name and calls set_font_sample.
# The fontlist widget and on_fontlist_select event handler
# are tested here together.

@classmethod
def setUpClass(cls):
if dialog.fontlist.size() < 2:
cls.skipTest('need at least 2 fonts')
dialog.set_font_sample = Func() # Mask instance method

@classmethod
def tearDownClass(cls):
del dialog.set_font_sample # Unmask instance method

@terryjreedy terryjreedy Jul 13, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Old habits die hard. I added the '.'s.


def setUp(self):
dialog.set_font_sample.called = 0
changes.clear()

def test_select_font_key(self):
# Up and Down keys should select a new font.

fontlist = dialog.fontlist
fontlist.activate(0)
font = dialog.fontlist.get('active')

# Test Down key.
fontlist.focus_force()
fontlist.update()
fontlist.event_generate('<Key-Down>')
fontlist.event_generate('<KeyRelease-Down>')

down_font = fontlist.get('active')
self.assertNotEqual(down_font, font)
self.assertIn(dialog.font_name.get(), down_font.lower())
self.assertEqual(dialog.set_font_sample.called, 1)

# Test Up key.
fontlist.focus_force()
fontlist.update()
fontlist.event_generate('<Key-Up>')
fontlist.event_generate('<KeyRelease-Up>')

up_font = fontlist.get('active')
self.assertEqual(up_font, font)
self.assertIn(dialog.font_name.get(), up_font.lower())
self.assertEqual(dialog.set_font_sample.called, 2)

def test_select_font_mouse(self):
# Click on item should select that item.

fontlist = dialog.fontlist
fontlist.activate(0)

# Select next item in listbox
fontlist.focus_force()
fontlist.see(1)
fontlist.update()
x, y, dx, dy = fontlist.bbox(1)
fontlist.event_generate('<Button-1>', x=x + dx // 2, y=y + dy // 2)
fontlist.event_generate('<ButtonRelease-1>', x=x + dx // 2, y=y + dy // 2)

@terryjreedy terryjreedy Jul 13, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree. For an assignment statement, I would write
x = x + dx//2
Following math tradition here is allowed by pep8. When the space around '=' is deleted, as per PEP8, it looks weird, and visually deceptive, to leave space around '+'. I don't know if this is specifically addressed.

@mlouielu mlouielu Jul 13, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After re-reading PEP8, I concur with your way to present x = x + dx//2, but it's weird in this situation, too.

I convert it to another way to represent:

x += dx // 2
y += dy // 2
event_generate(x=y, y=y)

I hope this will be more general in this case.

@terryjreedy terryjreedy Jul 13, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With two uses, that looks better.


font1 = fontlist.get(1)
select_font = fontlist.get('anchor')
self.assertEqual(select_font, font1)
self.assertIn(dialog.font_name.get(), font1.lower())
self.assertEqual(dialog.set_font_sample.called, 1)


class HighlightTest(unittest.TestCase):

def setUp(self):
Expand All @@ -103,19 +178,19 @@ def setUp(self):
changes.clear()

def test_startup(self):
configure.radio_startup_edit.invoke()
dialog.radio_startup_edit.invoke()
self.assertEqual(mainpage,
{'General': {'editor-on-startup': '1'}})

def test_autosave(self):
configure.radio_save_auto.invoke()
dialog.radio_save_auto.invoke()
self.assertEqual(mainpage, {'General': {'autosave': '1'}})

def test_editor_size(self):
configure.entry_win_height.insert(0, '1')
dialog.entry_win_height.insert(0, '1')
self.assertEqual(mainpage, {'EditorWindow': {'height': '140'}})
changes.clear()
configure.entry_win_width.insert(0, '1')
dialog.entry_win_width.insert(0, '1')
self.assertEqual(mainpage, {'EditorWindow': {'width': '180'}})

#def test_help_sources(self): pass # TODO
Expand Down