From 8055c4c60134b9b03f1854327b7f12152554737a Mon Sep 17 00:00:00 2001 From: Cheryl Sabella Date: Sat, 24 Jun 2017 07:03:30 -0400 Subject: [PATCH 1/5] bpo-6739: IDLE: Check for valid keybinding in config_keys --- Lib/idlelib/config_key.py | 45 ++++++++++++++++++------ Lib/idlelib/idle_test/test_config_key.py | 33 +++++++++++++++-- 2 files changed, 66 insertions(+), 12 deletions(-) diff --git a/Lib/idlelib/config_key.py b/Lib/idlelib/config_key.py index 479d6ad313e35f2..23e98f8d72b0bce 100644 --- a/Lib/idlelib/config_key.py +++ b/Lib/idlelib/config_key.py @@ -7,7 +7,12 @@ import string import sys + class GetKeysDialog(Toplevel): + + # Dialog title for invalid key sequence + keyerror_title = 'Key Sequence Error' + def __init__(self, parent, title, action, currentKeySequences, _htest=False, _utest=False): """ @@ -219,33 +224,34 @@ def TranslateKey(self, key, modifiers): return key def OK(self, event=None): - if self.advanced or self.KeysOK(): # doesn't check advanced string yet - self.result=self.keyString.get() + key = self.keyString.get() + key.strip() + if not key: + tkMessageBox.showerror(title=self.keyerror_title, parent=self, + message="No keys specified.") + return + if (self.advanced or self.KeysOK(key)) and self.sequence_ok(key): + self.result = self.keyString.get() self.destroy() def Cancel(self, event=None): self.result='' self.destroy() - def KeysOK(self): + def KeysOK(self, keys): '''Validity check on user's 'basic' keybinding selection. Doesn't check the string produced by the advanced dialog because 'modifiers' isn't set. ''' - keys = self.keyString.get() - keys.strip() finalKey = self.listKeysFinal.get(ANCHOR) modifiers = self.GetModifiers() # create a key sequence list for overlap check: keySequence = keys.split() keysOK = False - title = 'Key Sequence Error' - if not keys: - tkMessageBox.showerror(title=title, parent=self, - message='No keys specified.') - elif not keys.endswith('>'): + title = self.keyerror_title + if not keys.endswith('>'): tkMessageBox.showerror(title=title, parent=self, message='Missing the final Key') elif (not modifiers @@ -265,7 +271,26 @@ def KeysOK(self): keysOK = True return keysOK + def sequence_ok(self, keys): + """Verify if Tcl accepts the new keys.""" + accepted = False + + try: + binding = self.bind(keys, lambda: None) + except TclError as err: + tkMessageBox.showerror( + title=self.keyerror_title, parent=self, + message=(f'The entered key sequence is not accepted.\n\n' + f'Error: {err}')) + else: + self.unbind(keys, binding) + accepted = True + + return accepted + if __name__ == '__main__': + import unittest + unittest.main('idlelib.idle_test.test_config_key', verbosity=2, exit=False) from idlelib.idle_test.htest import run run(GetKeysDialog) diff --git a/Lib/idlelib/idle_test/test_config_key.py b/Lib/idlelib/idle_test/test_config_key.py index 8a24c9632b7224e..c856fa438c0bc75 100644 --- a/Lib/idlelib/idle_test/test_config_key.py +++ b/Lib/idlelib/idle_test/test_config_key.py @@ -4,15 +4,16 @@ ''' from idlelib import config_key from test.support import requires -requires('gui') import unittest from tkinter import Tk +from idlelib.idle_test.mock_tk import Mbox_func class GetKeysTest(unittest.TestCase): @classmethod def setUpClass(cls): + requires('gui') cls.root = Tk() cls.root.withdraw() @@ -22,12 +23,40 @@ def tearDownClass(cls): cls.root.destroy() del cls.root - def test_init(self): dia = config_key.GetKeysDialog( self.root, 'test', '<>', [''], _utest=True) dia.Cancel() +class SequenceOKTest(unittest.TestCase): + + @classmethod + def setUpClass(cls): + requires('gui') + cls.orig_error = config_key.tkMessageBox.showerror + config_key.tkMessageBox.showerror = Mbox_func() + cls.root = Tk() + cls.root.withdraw() + cls.dialog = config_key.GetKeysDialog( + cls.root, 'test', '<>', [''], _utest=True) + + @classmethod + def tearDownClass(cls): + config_key.tkMessageBox.showerror = cls.orig_error + del cls.orig_error + del cls.dialog + cls.root.update_idletasks() + cls.root.destroy() + del cls.root + + def test_sequence_ok_valid(self): + self.assertTrue(self.dialog.sequence_ok('')) + + def test_sequence_ok_invalid(self): + self.assertFalse(self.dialog.sequence_ok('')) + self.assertEqual(config_key.tkMessageBox.showerror.title, + self.dialog.keyerror_title) + if __name__ == '__main__': unittest.main(verbosity=2) From 122c77afee32bc3f737a43a1cec94cf5e2cadc74 Mon Sep 17 00:00:00 2001 From: Terry Jan Reedy Date: Sun, 25 Jun 2017 01:49:04 -0400 Subject: [PATCH 2/5] WIP Tests partly added but need more --- Lib/idlelib/config_key.py | 46 +++++++------- Lib/idlelib/idle_test/test_config_key.py | 76 ++++++++++++++++-------- 2 files changed, 71 insertions(+), 51 deletions(-) diff --git a/Lib/idlelib/config_key.py b/Lib/idlelib/config_key.py index 23e98f8d72b0bce..acc89ba925c17ac 100644 --- a/Lib/idlelib/config_key.py +++ b/Lib/idlelib/config_key.py @@ -3,7 +3,7 @@ """ from tkinter import * from tkinter.ttk import Scrollbar -import tkinter.messagebox as tkMessageBox +from tkinter.messagebox import showerror import string import sys @@ -224,15 +224,14 @@ def TranslateKey(self, key, modifiers): return key def OK(self, event=None): - key = self.keyString.get() - key.strip() - if not key: - tkMessageBox.showerror(title=self.keyerror_title, parent=self, - message="No keys specified.") + keys = self.keyString.get().strip() + if not keys: + showerror(title=self.keyerror_title, parent=self, + message="No key specified.") return - if (self.advanced or self.KeysOK(key)) and self.sequence_ok(key): - self.result = self.keyString.get() - self.destroy() + if (self.advanced or self.KeysOK(keys)) and self.bind_ok(keys): + self.result = keys + self.destroy() def Cancel(self, event=None): self.result='' @@ -252,41 +251,38 @@ def KeysOK(self, keys): keysOK = False title = self.keyerror_title if not keys.endswith('>'): - tkMessageBox.showerror(title=title, parent=self, - message='Missing the final Key') + showerror(title=title, parent=self, + message='Missing the final Key') elif (not modifiers and finalKey not in self.functionKeys + self.moveKeys): - tkMessageBox.showerror(title=title, parent=self, - message='No modifier key(s) specified.') + showerror(title=title, parent=self, + message='No modifier key(s) specified.') elif (modifiers == ['Shift']) \ and (finalKey not in self.functionKeys + self.moveKeys + ('Tab', 'Space')): msg = 'The shift modifier by itself may not be used with'\ ' this key symbol.' - tkMessageBox.showerror(title=title, parent=self, message=msg) + showerror(title=title, parent=self, message=msg) elif keySequence in self.currentKeySequences: msg = 'This key combination is already in use.' - tkMessageBox.showerror(title=title, parent=self, message=msg) + showerror(title=title, parent=self, message=msg) else: keysOK = True return keysOK - def sequence_ok(self, keys): - """Verify if Tcl accepts the new keys.""" - accepted = False + def bind_ok(self, keys): + "Return True if Tcl accepts the new keys else show message." try: binding = self.bind(keys, lambda: None) except TclError as err: - tkMessageBox.showerror( - title=self.keyerror_title, parent=self, - message=(f'The entered key sequence is not accepted.\n\n' - f'Error: {err}')) + showerror(title=self.keyerror_title, parent=self, + message=(f'The entered key sequence is not accepted.\n\n' + f'Error: {err}')) + return False else: self.unbind(keys, binding) - accepted = True - - return accepted + return True if __name__ == '__main__': diff --git a/Lib/idlelib/idle_test/test_config_key.py b/Lib/idlelib/idle_test/test_config_key.py index c856fa438c0bc75..a18305f6df7180b 100644 --- a/Lib/idlelib/idle_test/test_config_key.py +++ b/Lib/idlelib/idle_test/test_config_key.py @@ -4,8 +4,10 @@ ''' from idlelib import config_key from test.support import requires +import sys import unittest from tkinter import Tk +from tkinter import messagebox from idlelib.idle_test.mock_tk import Mbox_func @@ -14,49 +16,71 @@ class GetKeysTest(unittest.TestCase): @classmethod def setUpClass(cls): requires('gui') + config_key.showerror = Mbox_func() cls.root = Tk() cls.root.withdraw() + cls.dialog = config_key.GetKeysDialog( + cls.root, 'test', '<>', [''], _utest=True) @classmethod def tearDownClass(cls): - cls.root.update() # Stop "can't run event command" warning. + del cls.dialog + cls.root.update_idletasks() cls.root.destroy() del cls.root + config_key.showerror = messagebox.showerror + + def setUp(self): + config_key.showerror.message = '' def test_init(self): dia = config_key.GetKeysDialog( - self.root, 'test', '<>', [''], _utest=True) + self.root, 'test', '<>', [['']], _utest=True) dia.Cancel() + self.assertEqual(config_key.showerror.message, '') + def test_ok_empty(self): + self.dialog.keyString.set(' ') + self.dialog.OK() + self.assertEqual(self.dialog.result, '') + self.assertEqual(config_key.showerror.message, 'No key specified.') -class SequenceOKTest(unittest.TestCase): +# Getting this test to work requires ability to get KeyOK to return True. +# This requires an ability to manipulate modifiers and final key +# or a refactoring of code to make it more easily tested +# or temporary mocks. +## def test_ok_good(self): +## self.dialog.keyString.set('Key-F11') +## self.dialog.OK() +## self.assertEqual(self.dialog.result, '') +## self.assertEqual(config_key.showerror.message, '') - @classmethod - def setUpClass(cls): - requires('gui') - cls.orig_error = config_key.tkMessageBox.showerror - config_key.tkMessageBox.showerror = Mbox_func() - cls.root = Tk() - cls.root.withdraw() - cls.dialog = config_key.GetKeysDialog( - cls.root, 'test', '<>', [''], _utest=True) + def test_keys_short(self): + self.assertFalse(self.dialog.KeysOK('')) + self.assertIn('No modifier', config_key.showerror.message) + +# Need tests for shift, duplication, and success this or test_keys_shift or test_keys_ok to work +# +# The attempt below failed. Code should be made easier to test. +## def test_keys_dup(self): +## modifier_index = 1 if sys.platform == 'darwin' else 0 +## self.dialog.modifier_vars[modifier_index].set('Control') +## self.assertFalse(self.dialog.KeysOK('')) +## self.assertIn('already in use', config_key.showerror.message) + + + def test_bind_ok(self): + self.assertTrue(self.dialog.bind_ok('')) + self.assertEqual(config_key.showerror.message, '') - def test_sequence_ok_valid(self): - self.assertTrue(self.dialog.sequence_ok('')) + def test_bind_not_ok(self): + self.assertFalse(self.dialog.bind_ok('')) + self.assertIn('not accepted', config_key.showerror.message) - def test_sequence_ok_invalid(self): - self.assertFalse(self.dialog.sequence_ok('')) - self.assertEqual(config_key.tkMessageBox.showerror.title, - self.dialog.keyerror_title) if __name__ == '__main__': unittest.main(verbosity=2) From 9ca8a2133ae58a3232c4a3565e1cce37ad6cef91 Mon Sep 17 00:00:00 2001 From: Terry Jan Reedy Date: Sun, 25 Jun 2017 18:54:05 -0400 Subject: [PATCH 3/5] Make showerror a class attribute.\nUse subclass with mocks for tests. --- Lib/idlelib/config_key.py | 29 ++++--- Lib/idlelib/idle_test/test_config_key.py | 101 +++++++++++++---------- 2 files changed, 74 insertions(+), 56 deletions(-) diff --git a/Lib/idlelib/config_key.py b/Lib/idlelib/config_key.py index acc89ba925c17ac..8d575ec43ca8339 100644 --- a/Lib/idlelib/config_key.py +++ b/Lib/idlelib/config_key.py @@ -3,7 +3,7 @@ """ from tkinter import * from tkinter.ttk import Scrollbar -from tkinter.messagebox import showerror +from tkinter import messagebox import string import sys @@ -59,6 +59,10 @@ def __init__(self, parent, title, action, currentKeySequences, self.deiconify() #geometry set, unhide self.wait_window() + def showerror(self, *args, **kwargs): + # Make testing easier. Replace in #30751. + messagebox.showerror(*args, **kwargs) + def CreateWidgets(self): frameMain = Frame(self,borderwidth=2,relief=SUNKEN) frameMain.pack(side=TOP,expand=TRUE,fill=BOTH) @@ -226,8 +230,8 @@ def TranslateKey(self, key, modifiers): def OK(self, event=None): keys = self.keyString.get().strip() if not keys: - showerror(title=self.keyerror_title, parent=self, - message="No key specified.") + self.showerror(title=self.keyerror_title, parent=self, + message="No key specified.") return if (self.advanced or self.KeysOK(keys)) and self.bind_ok(keys): self.result = keys @@ -251,21 +255,21 @@ def KeysOK(self, keys): keysOK = False title = self.keyerror_title if not keys.endswith('>'): - showerror(title=title, parent=self, - message='Missing the final Key') + self.showerror(title, parent=self, + message='Missing the final Key') elif (not modifiers and finalKey not in self.functionKeys + self.moveKeys): - showerror(title=title, parent=self, - message='No modifier key(s) specified.') + self.showerror(title=title, parent=self, + message='No modifier key(s) specified.') elif (modifiers == ['Shift']) \ and (finalKey not in self.functionKeys + self.moveKeys + ('Tab', 'Space')): msg = 'The shift modifier by itself may not be used with'\ ' this key symbol.' - showerror(title=title, parent=self, message=msg) + self.showerror(title=title, parent=self, message=msg) elif keySequence in self.currentKeySequences: msg = 'This key combination is already in use.' - showerror(title=title, parent=self, message=msg) + self.showerror(title=title, parent=self, message=msg) else: keysOK = True return keysOK @@ -276,9 +280,10 @@ def bind_ok(self, keys): try: binding = self.bind(keys, lambda: None) except TclError as err: - showerror(title=self.keyerror_title, parent=self, - message=(f'The entered key sequence is not accepted.\n\n' - f'Error: {err}')) + self.showerror( + title=self.keyerror_title, parent=self, + message=(f'The entered key sequence is not accepted.\n\n' + f'Error: {err}')) return False else: self.unbind(keys, binding) diff --git a/Lib/idlelib/idle_test/test_config_key.py b/Lib/idlelib/idle_test/test_config_key.py index a18305f6df7180b..8fda8b8729b6752 100644 --- a/Lib/idlelib/idle_test/test_config_key.py +++ b/Lib/idlelib/idle_test/test_config_key.py @@ -7,79 +7,92 @@ import sys import unittest from tkinter import Tk -from tkinter import messagebox -from idlelib.idle_test.mock_tk import Mbox_func +from idlelib.idle_test.mock_idle import Func +from idlelib.idle_test.mock_tk import Var, Mbox_func -class GetKeysTest(unittest.TestCase): +class ValidationTest(unittest.TestCase): + "Test validation methods: OK, KeysOK, bind_ok." + + class Validator(config_key.GetKeysDialog): + def __init__(self, *args, **kwargs): + config_key.GetKeysDialog.__init__(self, *args, **kwargs) + class listKeysFinal: + get = Func() + self.listKeysFinal = listKeysFinal + GetModifiers = Func() + showerror = Mbox_func() @classmethod def setUpClass(cls): requires('gui') - config_key.showerror = Mbox_func() cls.root = Tk() cls.root.withdraw() - cls.dialog = config_key.GetKeysDialog( - cls.root, 'test', '<>', [''], _utest=True) + cls.dialog = cls.Validator( + cls.root, 'Title', '<>', [['']], _utest=True) @classmethod def tearDownClass(cls): - del cls.dialog + cls.dialog.Cancel() cls.root.update_idletasks() cls.root.destroy() - del cls.root - config_key.showerror = messagebox.showerror + del cls.dialog, cls.root def setUp(self): - config_key.showerror.message = '' - - def test_init(self): - dia = config_key.GetKeysDialog( - self.root, 'test', '<>', [['']], _utest=True) - dia.Cancel() - self.assertEqual(config_key.showerror.message, '') + self.dialog.showerror.message = '' + # A test that needs a particular final key value should set it. + # A test that sets a non-blank modifier list should reset it to []. + + def test_dummy(self): + pass def test_ok_empty(self): self.dialog.keyString.set(' ') self.dialog.OK() self.assertEqual(self.dialog.result, '') - self.assertEqual(config_key.showerror.message, 'No key specified.') - -# Getting this test to work requires ability to get KeyOK to return True. -# This requires an ability to manipulate modifiers and final key -# or a refactoring of code to make it more easily tested -# or temporary mocks. -## def test_ok_good(self): -## self.dialog.keyString.set('Key-F11') -## self.dialog.OK() -## self.assertEqual(self.dialog.result, '') -## self.assertEqual(config_key.showerror.message, '') - - def test_keys_short(self): - self.assertFalse(self.dialog.KeysOK('')) - self.assertIn('No modifier', config_key.showerror.message) + def test_ok_good(self): + self.dialog.keyString.set('') + self.dialog.listKeysFinal.get.result = 'F11' + self.dialog.OK() + self.assertEqual(self.dialog.result, '') + self.assertEqual(self.dialog.showerror.message, '') -# Need tests for shift, duplication, and success this or test_keys_shift or test_keys_ok to work -# -# The attempt below failed. Code should be made easier to test. -## def test_keys_dup(self): -## modifier_index = 1 if sys.platform == 'darwin' else 0 -## self.dialog.modifier_vars[modifier_index].set('Control') -## self.assertFalse(self.dialog.KeysOK('')) -## self.assertIn('already in use', config_key.showerror.message) + def test_keys_no_ending(self): + self.assertFalse(self.dialog.KeysOK('')) + self.assertIn('No modifier', self.dialog.showerror.message) + + def test_keys_no_modifier_ok(self): + self.dialog.listKeysFinal.get.result = 'F11' + self.assertTrue(self.dialog.KeysOK('')) + self.assertEqual(self.dialog.showerror.message, '') + + def test_keys_shift_bad(self): + self.dialog.listKeysFinal.get.result = 'a' + self.dialog.GetModifiers.result = ['Shift'] + self.assertFalse(self.dialog.KeysOK('')) + self.assertIn('shift modifier', self.dialog.showerror.message) + self.dialog.GetModifiers.result = [] + + def test_keys_dup(self): + self.dialog.listKeysFinal.get.result = 'F12' + self.dialog.GetModifiers.result = [] + self.assertFalse(self.dialog.KeysOK('')) + self.assertIn('already in use', self.dialog.showerror.message) def test_bind_ok(self): self.assertTrue(self.dialog.bind_ok('')) - self.assertEqual(config_key.showerror.message, '') + self.assertEqual(self.dialog.showerror.message, '') def test_bind_not_ok(self): self.assertFalse(self.dialog.bind_ok('')) - self.assertIn('not accepted', config_key.showerror.message) + self.assertIn('not accepted', self.dialog.showerror.message) if __name__ == '__main__': From 19db859dd7f9c7aa75cac86e8f0e4866d0b063ec Mon Sep 17 00:00:00 2001 From: Terry Jan Reedy Date: Mon, 26 Jun 2017 00:24:21 -0400 Subject: [PATCH 4/5] whitespace --- Lib/idlelib/idle_test/test_config_key.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/Lib/idlelib/idle_test/test_config_key.py b/Lib/idlelib/idle_test/test_config_key.py index 8fda8b8729b6752..a4227ed0552b7f2 100644 --- a/Lib/idlelib/idle_test/test_config_key.py +++ b/Lib/idlelib/idle_test/test_config_key.py @@ -42,9 +42,6 @@ def setUp(self): self.dialog.showerror.message = '' # A test that needs a particular final key value should set it. # A test that sets a non-blank modifier list should reset it to []. - - def test_dummy(self): - pass def test_ok_empty(self): self.dialog.keyString.set(' ') From 09b4a29ef5abbef960b91b0ee1aad26210e0a472 Mon Sep 17 00:00:00 2001 From: Terry Jan Reedy Date: Mon, 26 Jun 2017 00:29:18 -0400 Subject: [PATCH 5/5] Add news --- Misc/NEWS.d/next/IDLE/2017-06-26-00-28-59.bpo-6739.x5MfhB.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 Misc/NEWS.d/next/IDLE/2017-06-26-00-28-59.bpo-6739.x5MfhB.rst diff --git a/Misc/NEWS.d/next/IDLE/2017-06-26-00-28-59.bpo-6739.x5MfhB.rst b/Misc/NEWS.d/next/IDLE/2017-06-26-00-28-59.bpo-6739.x5MfhB.rst new file mode 100644 index 000000000000000..fee904d9e78c50e --- /dev/null +++ b/Misc/NEWS.d/next/IDLE/2017-06-26-00-28-59.bpo-6739.x5MfhB.rst @@ -0,0 +1,3 @@ +IDLE: Verify user-entered key sequences by trying to bind them with tk. Add +tests for all 3 validation functions. Original patch by G Polo. Tests added +by Cheryl Sabella.