bpo-30993: IDLE - Improve configdialog font page and tests. (#2831)

In configdialog: Document causal pathways in create_font_tab docstring.  Simplify some attribute names. Move set_samples calls to var_changed_font (idea from Cheryl Sabella).  Move related functions to positions after the create widgets function.

In test_configdialog: Fix test_font_set so not order dependent.  Fix renamed test_indent_scale so it tests the widget.  Adjust tests for movement of set_samples call.  Add tests for load functions.  Put all font tests in one class and tab indent tests in another.  Except for two lines, these tests completely cover the related functions.

Lib/idlelib/configdialog.py

@@ -150,6 +150,7 @@ class ConfigDialog(Toplevel):
         buttons.pack(side=BOTTOM)
         return outer
 
+
     def create_page_font_tab(self):
         """Return frame of widgets for Font/Tabs tab.
 
@@ -159,29 +160,26 @@ class ConfigDialog(Toplevel):
         corresponding aspect of the font sample on this page and
         highlight sample on highlight page.
 
+        Load_font_cfg initializes font vars and widgets from
+        idleConf entries and tk.
+
         Fontlist: mouse button 1 click or up or down key invoke
-        on_fontlist_select(), which sets Var font_name and calls
-        set_samples.
+        on_fontlist_select(), which sets var font_name.
 
         Sizelist: clicking the menubutton opens the dropdown menu. A
-        mouse button 1 click or return key invokes an internal command
-        which sets Var font_size and calls set_samples.
+        mouse button 1 click or return key sets var font_size.
 
-        Bold_toggle, clicking the box toggles font_bold and calls
-        set_samples.
+        Bold_toggle: clicking the box toggles var font_bold.
 
-        Setting any of the font vars invokes var_changed_font, which
-        adds all 3 font options to changes.  Set_samples applies a new
-        font constructed from the font vars to font_sample and
-        highlight_sample on the hightlight page.
+        Changing any of the font vars invokes var_changed_font, which
+        adds all 3 font options to changes and calls set_samples.
+        Set_samples applies a new font constructed from the font vars to
+        font_sample and to highlight_sample on the hightlight page.
 
         Tabs: Enable users to change spaces entered for indent tabs.
         Changing indent_scale value with the mouse sets Var space_num,
         which invokes var_changed_space_num, which adds an entry to
-        changes.
-
-        Load_font_cfg and load_tab_cfg initialize vars and widgets from
-        idleConf entries.
+        changes.  Load_tab_cfg initializes space_num to default.
 
         Widget Structure:  (*) widgets bound to self
             frame (of tab_pages)
@@ -227,11 +225,10 @@ class ConfigDialog(Toplevel):
         scroll_font.config(command=self.fontlist.yview)
         self.fontlist.config(yscrollcommand=scroll_font.set)
         font_size_title = Label(frame_font_param, text='Size :')
-        self.sizelist = DynOptionMenu(frame_font_param, self.font_size,
-                                      None, command=self.set_samples)
+        self.sizelist = DynOptionMenu(frame_font_param, self.font_size, None)
         self.bold_toggle = Checkbutton(
-                frame_font_param, variable=self.font_bold, onvalue=1,
-                offvalue=0, text='Bold', command=self.set_samples)
+                frame_font_param, variable=self.font_bold,
+                onvalue=1, offvalue=0, text='Bold')
         frame_font_sample = Frame(frame_font, relief=SOLID, borderwidth=1)
         temp_font = tkFont.Font(parent, ('courier', 10, 'normal'))
         self.font_sample = Label(
@@ -267,6 +264,96 @@ class ConfigDialog(Toplevel):
 
         return frame
 
+    def load_font_cfg(self):
+        """Load current configuration settings for the font options.
+
+        Retrieve current font with idleConf.GetFont and font families
+        from tk. Setup fontlist and set font_name.  Setup sizelist,
+        which sets font_size.  Set font_bold.  Setting font variables
+        calls set_samples (thrice).
+        """
+        configured_font = idleConf.GetFont(self, 'main', 'EditorWindow')
+        font_name = configured_font[0].lower()
+        font_size = configured_font[1]
+        font_bold  = configured_font[2]=='bold'
+
+        # Set editor font selection list and font_name.
+        fonts = list(tkFont.families(self))
+        fonts.sort()
+        for font in fonts:
+            self.fontlist.insert(END, font)
+        self.font_name.set(font_name)
+        lc_fonts = [s.lower() for s in fonts]
+        try:
+            current_font_index = lc_fonts.index(font_name)
+            self.fontlist.see(current_font_index)
+            self.fontlist.select_set(current_font_index)
+            self.fontlist.select_anchor(current_font_index)
+            self.fontlist.activate(current_font_index)
+        except ValueError:
+            pass
+        # Set font size dropdown.
+        self.sizelist.SetMenu(('7', '8', '9', '10', '11', '12', '13', '14',
+                               '16', '18', '20', '22', '25', '29', '34', '40'),
+                              font_size)
+        # Set font weight.
+        self.font_bold.set(font_bold)
+
+    def on_fontlist_select(self, event):
+        """Handle selecting a font from the list.
+
+        Event can result from either mouse click or Up or Down key.
+        Set font_name and example displays to selection.
+        """
+        font = self.fontlist.get(
+                ACTIVE if event.type.name == 'KeyRelease' else ANCHOR)
+        self.font_name.set(font.lower())
+
+    def var_changed_font(self, *params):
+        """Store changes to font attributes.
+
+        When one font attribute changes, save them all, as they are
+        not independent from each other. In particular, when we are
+        overriding the default font, we need to write out everything.
+        """
+        value = self.font_name.get()
+        changes.add_option('main', 'EditorWindow', 'font', value)
+        value = self.font_size.get()
+        changes.add_option('main', 'EditorWindow', 'font-size', value)
+        value = self.font_bold.get()
+        changes.add_option('main', 'EditorWindow', 'font-bold', value)
+        self.set_samples()
+
+    def set_samples(self, event=None):
+        """Update update both screen samples with the font settings.
+
+        Called on font initialization and change events.
+        Accesses font_name, font_size, and font_bold Variables.
+        Updates font_sample and hightlight page highlight_sample.
+        """
+        font_name = self.font_name.get()
+        font_weight = tkFont.BOLD if self.font_bold.get() else tkFont.NORMAL
+        new_font = (font_name, self.font_size.get(), font_weight)
+        self.font_sample['font'] = new_font
+        self.highlight_sample['font'] = new_font
+
+    def load_tab_cfg(self):
+        """Load current configuration settings for the tab options.
+
+        Attributes updated:
+            space_num: Set to value from idleConf.
+        """
+        # Set indent sizes.
+        space_num = idleConf.GetOption(
+            'main', 'Indent', 'num-spaces', default=4, type='int')
+        self.space_num.set(space_num)
+
+    def var_changed_space_num(self, *params):
+        "Store change to indentation size."
+        value = self.space_num.get()
+        changes.add_option('main', 'Indent', 'num-spaces', value)
+
+
     def create_page_highlight(self):
         """Return frame of widgets for Highlighting tab.
 
@@ -729,25 +816,6 @@ class ConfigDialog(Toplevel):
                 self.startup_edit, self.autosave,):
             var.trace_remove('write', var.trace_info()[0][1])
 
-    def var_changed_font(self, *params):
-        """Store changes to font attributes.
-
-        When one font attribute changes, save them all, as they are
-        not independent from each other. In particular, when we are
-        overriding the default font, we need to write out everything.
-        """
-        value = self.font_name.get()
-        changes.add_option('main', 'EditorWindow', 'font', value)
-        value = self.font_size.get()
-        changes.add_option('main', 'EditorWindow', 'font-size', value)
-        value = self.font_bold.get()
-        changes.add_option('main', 'EditorWindow', 'font-bold', value)
-
-    def var_changed_space_num(self, *params):
-        "Store change to indentation size."
-        value = self.space_num.get()
-        changes.add_option('main', 'Indent', 'num-spaces', value)
-
     def var_changed_color(self, *params):
         "Process change to color choice."
         self.on_new_color_set()
@@ -1216,30 +1284,6 @@ class ConfigDialog(Toplevel):
         self.is_builtin_theme.set(0)
         self.set_theme_type()
 
-    def on_fontlist_select(self, event):
-        """Handle selecting a font from the list.
-
-        Event can result from either mouse click or Up or Down key.
-        Set font_name and example displays to selection.
-        """
-        font = self.fontlist.get(
-                ACTIVE if event.type.name == 'KeyRelease' else ANCHOR)
-        self.font_name.set(font.lower())
-        self.set_samples()
-
-    def set_samples(self, event=None):
-        """Update update both screen samples with the font settings.
-
-        Called on font initialization and change events.
-        Accesses font_name, font_size, and font_bold Variables.
-        Updates font_sample and hightlight page highlight_sample.
-        """
-        font_name = self.font_name.get()
-        font_weight = tkFont.BOLD if self.font_bold.get() else tkFont.NORMAL
-        new_font = (font_name, self.font_size.get(), font_weight)
-        self.font_sample['font'] = new_font
-        self.highlight_sample['font'] = new_font
-
     def set_highlight_target(self):
         """Set fg/bg toggle and color based on highlight tag target.
 
@@ -1404,61 +1448,6 @@ class ConfigDialog(Toplevel):
                     'main', 'HelpFiles', str(num),
                     ';'.join(self.user_helplist[num-1][:2]))
 
-    def load_font_cfg(self):
-        """Load current configuration settings for the font options.
-
-        Retrieve current font values from idleConf.GetFont to set
-        as initial values for font widgets.
-
-        Attributes updated:
-            fontlist: Populate with fonts from tkinter.font.
-            font_name: Set to current font.
-            sizelist: Populate valid options tuple and set
-                to current size.
-            font_bold: Set to current font weight.
-
-        Methods:
-            set_samples
-        """
-        # Set base editor font selection list.
-        fonts = list(tkFont.families(self))
-        fonts.sort()
-        for font in fonts:
-            self.fontlist.insert(END, font)
-        configured_font = idleConf.GetFont(self, 'main', 'EditorWindow')
-        font_name = configured_font[0].lower()
-        font_size = configured_font[1]
-        font_bold  = configured_font[2]=='bold'
-        self.font_name.set(font_name)
-        lc_fonts = [s.lower() for s in fonts]
-        try:
-            current_font_index = lc_fonts.index(font_name)
-            self.fontlist.see(current_font_index)
-            self.fontlist.select_set(current_font_index)
-            self.fontlist.select_anchor(current_font_index)
-            self.fontlist.activate(current_font_index)
-        except ValueError:
-            pass
-        # Set font size dropdown.
-        self.sizelist.SetMenu(('7', '8', '9', '10', '11', '12', '13', '14',
-                               '16', '18', '20', '22', '25', '29', '34', '40'),
-                              font_size)
-        # Set font weight.
-        self.font_bold.set(font_bold)
-        # Set font sample.
-        self.set_samples()
-
-    def load_tab_cfg(self):
-        """Load current configuration settings for the tab options.
-
-        Attributes updated:
-            space_num: Set to value from idleConf.
-        """
-        # Set indent sizes.
-        space_num = idleConf.GetOption(
-            'main', 'Indent', 'num-spaces', default=4, type='int')
-        self.space_num.set(space_num)
-
     def load_theme_cfg(self):
         """Load current configuration settings for the theme options.
 

Lib/idlelib/idle_test/test_configdialog.py

@@ -45,82 +45,15 @@ def tearDownModule():
     del root
 
 
-class FontTabTest(unittest.TestCase):
-    "Test that font widget enable users to make font changes."
-
-
-    def setUp(self):
-        changes.clear()
-
-    def test_font_set(self):
-        # Test that setting a font Variable results in 3 provisional
-        # change entries. Use values sure to not be defaults.
-        # Other font tests verify that user actions set Variables.
-        default_font = idleConf.GetFont(root, 'main', 'EditorWindow')
-        default_size = str(default_font[1])
-        default_bold = default_font[2] == 'bold'
-        d = dialog
-        d.font_name.set('Test Font')
-        d.font_size.set(default_size)
-        d.font_bold.set(default_bold)
-        expected = {'EditorWindow': {'font': 'Test Font',
-                                     'font-size': default_size,
-                                     'font-bold': str(default_bold)}}
-        self.assertEqual(mainpage, expected)
-        changes.clear()
-        d.font_size.set(20)
-        expected = {'EditorWindow': {'font': 'Test Font',
-                                     'font-size': '20',
-                                     'font-bold': str(default_bold)}}
-        self.assertEqual(mainpage, expected)
-        changes.clear()
-        d.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_bold_toggle_set_samples(self):
-        # Set up.
-        d = dialog
-        d.font_sample, d.highlight_sample = {}, {}  # Must undo this.
-        d.font_name.set('test')
-        d.font_size.set('5')
-        d.font_bold.set(1)
-        expected0 = {'font': ('test', '5', 'normal')}
-        expected1 = {'font': ('test', '5', 'bold')}
-
-        # Test set_samples.
-        d.set_samples()
-        self.assertTrue(d.font_sample == d.highlight_sample == expected1)
-
-        # Test bold_toggle.  If this fails, problem precedes set_samples.
-        d.bold_toggle.invoke()
-        self.assertFalse(d.font_bold.get())
-        self.assertTrue(d.font_sample == d.highlight_sample == expected0)
-        d.bold_toggle.invoke()
-        self.assertTrue(d.font_bold.get())
-        self.assertTrue(d.font_sample == d.highlight_sample == expected1)
-
-        #  Clean up.
-        del d.font_sample, d.highlight_sample
-
-    def test_indent_scale(self):
-        dialog.indent_scale.set(26)
-        self.assertEqual(dialog.space_num.get(), 16)
-        self.assertEqual(mainpage, {'Indent': {'num-spaces': '16'}})
-
-
-class FontSelectTest(unittest.TestCase):
-    # These two functions test that selecting a new font in the
-    # list of fonts changes font_name and calls set_samples.
-    # The fontlist widget and on_fontlist_select event handler
-    # are tested here together.
+class FontTest(unittest.TestCase):
+    """Test that font widgets enable users to make font changes.
 
+    Test that widget actions set vars, that var changes add three
+    options to changes and call set_samples, and that set_samples
+    changes the font of both sample boxes.
+    """
     @classmethod
     def setUpClass(cls):
-        if dialog.fontlist.size() < 2:
-            cls.skipTest('need at least 2 fonts')
         dialog.set_samples = Func()  # Mask instance method.
 
     @classmethod
@@ -128,12 +61,27 @@ class FontSelectTest(unittest.TestCase):
         del dialog.set_samples  # Unmask instance method.
 
     def setUp(self):
-        dialog.set_samples.called = 0
         changes.clear()
 
-    def test_select_font_key(self):
+    def test_load_font_cfg(self):
+        # Leave widget load test to human visual check.
+        # TODO Improve checks when add IdleConf.get_font_values.
+        d = dialog
+        d.font_name.set('Fake')
+        d.font_size.set('1')
+        d.font_bold.set(True)
+        d.set_samples.called = 0
+        d.load_font_cfg()
+        self.assertNotEqual(d.font_name.get(), 'Fake')
+        self.assertNotEqual(d.font_size.get(), '1')
+        self.assertFalse(d.font_bold.get())
+        self.assertEqual(d.set_samples.called, 3)
+
+    def test_fontlist_key(self):
         # Up and Down keys should select a new font.
 
+        if dialog.fontlist.size() < 2:
+            cls.skipTest('need at least 2 fonts')
         fontlist = dialog.fontlist
         fontlist.activate(0)
         font = dialog.fontlist.get('active')
@@ -147,7 +95,6 @@ class FontSelectTest(unittest.TestCase):
         down_font = fontlist.get('active')
         self.assertNotEqual(down_font, font)
         self.assertIn(dialog.font_name.get(), down_font.lower())
-        self.assertEqual(dialog.set_samples.called, 1)
 
         # Test Up key.
         fontlist.focus_force()
@@ -158,11 +105,12 @@ class FontSelectTest(unittest.TestCase):
         up_font = fontlist.get('active')
         self.assertEqual(up_font, font)
         self.assertIn(dialog.font_name.get(), up_font.lower())
-        self.assertEqual(dialog.set_samples.called, 2)
 
-    def test_select_font_mouse(self):
+    def test_fontlist_mouse(self):
         # Click on item should select that item.
 
+        if dialog.fontlist.size() < 2:
+            cls.skipTest('need at least 2 fonts')
         fontlist = dialog.fontlist
         fontlist.activate(0)
 
@@ -180,7 +128,88 @@ class FontSelectTest(unittest.TestCase):
         select_font = fontlist.get('anchor')
         self.assertEqual(select_font, font1)
         self.assertIn(dialog.font_name.get(), font1.lower())
-        self.assertEqual(dialog.set_samples.called, 1)
+
+    def test_sizelist(self):
+        # Click on number shouod select that number
+        d = dialog
+        d.sizelist.variable.set(40)
+        self.assertEqual(d.font_size.get(), '40')
+
+    def test_bold_toggle(self):
+        # Click on checkbutton should invert it.
+        d = dialog
+        d.font_bold.set(False)
+        d.bold_toggle.invoke()
+        self.assertTrue(d.font_bold.get())
+        d.bold_toggle.invoke()
+        self.assertFalse(d.font_bold.get())
+
+    def test_font_set(self):
+        # Test that setting a font Variable results in 3 provisional
+        # change entries and a call to set_samples. Use values sure to
+        # not be defaults.
+
+        default_font = idleConf.GetFont(root, 'main', 'EditorWindow')
+        default_size = str(default_font[1])
+        default_bold = default_font[2] == 'bold'
+        d = dialog
+        d.font_size.set(default_size)
+        d.font_bold.set(default_bold)
+        d.set_samples.called = 0
+
+        d.font_name.set('Test Font')
+        expected = {'EditorWindow': {'font': 'Test Font',
+                                     'font-size': default_size,
+                                     'font-bold': str(default_bold)}}
+        self.assertEqual(mainpage, expected)
+        self.assertEqual(d.set_samples.called, 1)
+        changes.clear()
+
+        d.font_size.set('20')
+        expected = {'EditorWindow': {'font': 'Test Font',
+                                     'font-size': '20',
+                                     'font-bold': str(default_bold)}}
+        self.assertEqual(mainpage, expected)
+        self.assertEqual(d.set_samples.called, 2)
+        changes.clear()
+
+        d.font_bold.set(not default_bold)
+        expected = {'EditorWindow': {'font': 'Test Font',
+                                     'font-size': '20',
+                                     'font-bold': str(not default_bold)}}
+        self.assertEqual(mainpage, expected)
+        self.assertEqual(d.set_samples.called, 3)
+
+    def test_set_samples(self):
+        d = dialog
+        del d.set_samples  # Unmask method for test
+        d.font_sample, d.highlight_sample = {}, {}
+        d.font_name.set('test')
+        d.font_size.set('5')
+        d.font_bold.set(1)
+        expected = {'font': ('test', '5', 'bold')}
+
+        # Test set_samples.
+        d.set_samples()
+        self.assertTrue(d.font_sample == d.highlight_sample == expected)
+
+        del d.font_sample, d.highlight_sample
+        d.set_samples = Func()  # Re-mask for other tests.
+
+
+class IndentTest(unittest.TestCase):
+
+    def test_load_tab_cfg(self):
+        d = dialog
+        d.space_num.set(16)
+        d.load_tab_cfg()
+        self.assertEqual(d.space_num.get(), 4)
+
+    def test_indent_scale(self):
+        changes.clear()
+        dialog.indent_scale.set(26)
+        self.assertEqual(dialog.space_num.get(), 16)
+        self.assertEqual(mainpage, {'Indent': {'num-spaces': '16'}})
 
 
 class HighlightTest(unittest.TestCase):
@@ -188,8 +217,6 @@ class HighlightTest(unittest.TestCase):
     def setUp(self):
         changes.clear()
 
-    #def test_colorchoose(self): pass  # TODO
-
 
 class KeysTest(unittest.TestCase):
 

Misc/NEWS.d/next/IDLE/2017-07-22-18-08-41.bpo-30993.34vJkB.rst

 IDLE - Improve configdialog font page and tests.
 
-* Document causal pathways in docstring. * Simplify some attribute names. *
-Rename test_bold_toggle_set_samples to make test_font_set fail. * Fix
-test_font_set so not order dependent. * Fix renamed test_indent_scale so it
-tests the widget.
+In configdialog: Document causal pathways in create_font_tab docstring.  Simplify some attribute names. Move set_samples calls to var_changed_font (idea from Cheryl Sabella).  Move related functions to positions after the create widgets function.
+
+In test_configdialog: Fix test_font_set so not order dependent.  Fix renamed test_indent_scale so it tests the widget.  Adjust tests for movement of set_samples call.  Add tests for load functions.  Put all font tests in one class and tab indent tests in another.  Except for two lines, these tests completely cover the related functions.