bpo-30899: Add unittests, 100% coverage, for IDLE's two ConfigParser subclasses. (#2662)

Patch by Louie Lu.

Lib/idlelib/config.py

@@ -81,31 +81,6 @@ class IdleUserConfParser(IdleConfParser):
     IdleConfigParser specialised for user configuration handling.
     """
 
-    def AddSection(self, section):
-        "If section doesn't exist, add it."
-        if not self.has_section(section):
-            self.add_section(section)
-
-    def RemoveEmptySections(self):
-        "Remove any sections that have no options."
-        for section in self.sections():
-            if not self.GetOptionList(section):
-                self.remove_section(section)
-
-    def IsEmpty(self):
-        "Return True if no sections after removing empty sections."
-        self.RemoveEmptySections()
-        return not self.sections()
-
-    def RemoveOption(self, section, option):
-        """Return True if option is removed from section, else False.
-
-        False if either section does not exist or did not have option.
-        """
-        if self.has_section(section):
-            return self.remove_option(section, option)
-        return False
-
     def SetOption(self, section, option, value):
         """Return True if option is added or changed to value, else False.
 
@@ -123,6 +98,31 @@ class IdleUserConfParser(IdleConfParser):
             self.set(section, option, value)
             return True
 
+    def RemoveOption(self, section, option):
+        """Return True if option is removed from section, else False.
+
+        False if either section does not exist or did not have option.
+        """
+        if self.has_section(section):
+            return self.remove_option(section, option)
+        return False
+
+    def AddSection(self, section):
+        "If section doesn't exist, add it."
+        if not self.has_section(section):
+            self.add_section(section)
+
+    def RemoveEmptySections(self):
+        "Remove any sections that have no options."
+        for section in self.sections():
+            if not self.GetOptionList(section):
+                self.remove_section(section)
+
+    def IsEmpty(self):
+        "Return True if no sections after removing empty sections."
+        self.RemoveEmptySections()
+        return not self.sections()
+
     def RemoveFile(self):
         "Remove user config file self.file from disk if it exists."
         if os.path.exists(self.file):
@@ -829,9 +829,12 @@ class ConfigChanges(dict):
     def save_all(self):
         """Save configuration changes to the user config file.
 
-        Then clear self in preparation for additional changes.
+        Clear self in preparation for additional changes.
+        Return changed for testing.
         """
         idleConf.userCfg['main'].Save()
+
+        changed = False
         for config_type in self:
             cfg_type_changed = False
             page = self[config_type]
@@ -844,12 +847,14 @@ class ConfigChanges(dict):
                         cfg_type_changed = True
             if cfg_type_changed:
                 idleConf.userCfg[config_type].Save()
+                changed = True
         for config_type in ['keys', 'highlight']:
             # Save these even if unchanged!
             idleConf.userCfg[config_type].Save()
         self.clear()
         # ConfigDialog caller must add the following call
         # self.save_all_changed_extensions()  # Uses a different mechanism.
+        return changed
 
     def delete_section(self, config_type, section):
         """Delete a section from self, userCfg, and file.

Lib/idlelib/idle_test/test_config.py

 '''Test idlelib.config.
 
-Much is tested by opening config dialog live or in test_configdialog.
-Coverage: 27%
+Coverage: 46% (100% for IdleConfParser, IdleUserConfParser*, ConfigChanges).
+* Except is OSError clause in Save method.
+Much of IdleConf is exercised by ConfigDialog and test_configdialog,
+but it should be tested here.
 '''
-from test.support import captured_stderr
+import os
+import tempfile
+from test.support import captured_stderr, findfile
 import unittest
 from idlelib import config
 
@@ -26,6 +30,161 @@ def tearDownModule():
     idleConf.userCfg = usercfg
 
 
+class IdleConfParserTest(unittest.TestCase):
+    """Test that IdleConfParser works"""
+
+    config = """
+        [one]
+        one = false
+        two = true
+        three = 10
+
+        [two]
+        one = a string
+        two = true
+        three = false
+    """
+
+    def test_get(self):
+        parser = config.IdleConfParser('')
+        parser.read_string(self.config)
+        eq = self.assertEqual
+
+        # Test with type argument.
+        self.assertIs(parser.Get('one', 'one', type='bool'), False)
+        self.assertIs(parser.Get('one', 'two', type='bool'), True)
+        eq(parser.Get('one', 'three', type='int'), 10)
+        eq(parser.Get('two', 'one'), 'a string')
+        self.assertIs(parser.Get('two', 'two', type='bool'), True)
+        self.assertIs(parser.Get('two', 'three', type='bool'), False)
+
+        # Test without type should fallback to string.
+        eq(parser.Get('two', 'two'), 'true')
+        eq(parser.Get('two', 'three'), 'false')
+
+        # If option not exist, should return None, or default.
+        self.assertIsNone(parser.Get('not', 'exist'))
+        eq(parser.Get('not', 'exist', default='DEFAULT'), 'DEFAULT')
+
+    def test_get_option_list(self):
+        parser = config.IdleConfParser('')
+        parser.read_string(self.config)
+        get_list = parser.GetOptionList
+        self.assertCountEqual(get_list('one'), ['one', 'two', 'three'])
+        self.assertCountEqual(get_list('two'), ['one', 'two', 'three'])
+        self.assertEqual(get_list('not exist'), [])
+
+    def test_load_nothing(self):
+        parser = config.IdleConfParser('')
+        parser.Load()
+        self.assertEqual(parser.sections(), [])
+
+    def test_load_file(self):
+        # Borrow test/cfgparser.1 from test_configparser.
+        config_path = findfile('cfgparser.1')
+        parser = config.IdleConfParser(config_path)
+        parser.Load()
+
+        self.assertEqual(parser.Get('Foo Bar', 'foo'), 'newbar')
+        self.assertEqual(parser.GetOptionList('Foo Bar'), ['foo'])
+
+
+class IdleUserConfParserTest(unittest.TestCase):
+    """Test that IdleUserConfParser works"""
+
+    def new_parser(self, path=''):
+        return config.IdleUserConfParser(path)
+
+    def test_set_option(self):
+        parser = self.new_parser()
+        parser.add_section('Foo')
+        # Setting new option in existing section should return True.
+        self.assertTrue(parser.SetOption('Foo', 'bar', 'true'))
+        # Setting existing option with same value should return False.
+        self.assertFalse(parser.SetOption('Foo', 'bar', 'true'))
+        # Setting exiting option with new value should return True.
+        self.assertTrue(parser.SetOption('Foo', 'bar', 'false'))
+        self.assertEqual(parser.Get('Foo', 'bar'), 'false')
+
+        # Setting option in new section should create section and return True.
+        self.assertTrue(parser.SetOption('Bar', 'bar', 'true'))
+        self.assertCountEqual(parser.sections(), ['Bar', 'Foo'])
+        self.assertEqual(parser.Get('Bar', 'bar'), 'true')
+
+    def test_remove_option(self):
+        parser = self.new_parser()
+        parser.AddSection('Foo')
+        parser.SetOption('Foo', 'bar', 'true')
+
+        self.assertTrue(parser.RemoveOption('Foo', 'bar'))
+        self.assertFalse(parser.RemoveOption('Foo', 'bar'))
+        self.assertFalse(parser.RemoveOption('Not', 'Exist'))
+
+    def test_add_section(self):
+        parser = self.new_parser()
+        self.assertEqual(parser.sections(), [])
+
+        # Should not add duplicate section.
+        # Configparser raises DuplicateError, IdleParser not.
+        parser.AddSection('Foo')
+        parser.AddSection('Foo')
+        parser.AddSection('Bar')
+        self.assertCountEqual(parser.sections(), ['Bar', 'Foo'])
+
+    def test_remove_empty_sections(self):
+        parser = self.new_parser()
+
+        parser.AddSection('Foo')
+        parser.AddSection('Bar')
+        parser.SetOption('Idle', 'name', 'val')
+        self.assertCountEqual(parser.sections(), ['Bar', 'Foo', 'Idle'])
+        parser.RemoveEmptySections()
+        self.assertEqual(parser.sections(), ['Idle'])
+
+    def test_is_empty(self):
+        parser = self.new_parser()
+
+        parser.AddSection('Foo')
+        parser.AddSection('Bar')
+        self.assertTrue(parser.IsEmpty())
+        self.assertEqual(parser.sections(), [])
+
+        parser.SetOption('Foo', 'bar', 'false')
+        parser.AddSection('Bar')
+        self.assertFalse(parser.IsEmpty())
+        self.assertCountEqual(parser.sections(), ['Foo'])
+
+    def test_remove_file(self):
+        with tempfile.TemporaryDirectory() as tdir:
+            path = os.path.join(tdir, 'test.cfg')
+            parser = self.new_parser(path)
+            parser.RemoveFile()  # Should not raise exception.
+
+            parser.AddSection('Foo')
+            parser.SetOption('Foo', 'bar', 'true')
+            parser.Save()
+            self.assertTrue(os.path.exists(path))
+            parser.RemoveFile()
+            self.assertFalse(os.path.exists(path))
+
+    def test_save(self):
+        with tempfile.TemporaryDirectory() as tdir:
+            path = os.path.join(tdir, 'test.cfg')
+            parser = self.new_parser(path)
+            parser.AddSection('Foo')
+            parser.SetOption('Foo', 'bar', 'true')
+
+            # Should save to path when config is not empty.
+            self.assertFalse(os.path.exists(path))
+            parser.Save()
+            self.assertTrue(os.path.exists(path))
+
+            # Should remove the file from disk when config is empty.
+            parser.remove_section('Foo')
+            parser.Save()
+            self.assertFalse(os.path.exists(path))
+
+
 class CurrentColorKeysTest(unittest.TestCase):
     """ Test colorkeys function with user config [Theme] and [Keys] patterns.
 
@@ -179,10 +338,12 @@ class ChangesTest(unittest.TestCase):
 
     def test_save_added(self):
         changes = self.load()
-        changes.save_all()
+        self.assertTrue(changes.save_all())
         self.assertEqual(usermain['Msec']['mitem'], 'mval')
         self.assertEqual(userhigh['Hsec']['hitem'], 'hval')
         self.assertEqual(userkeys['Ksec']['kitem'], 'kval')
+        changes.add_option('main', 'Msec', 'mitem', 'mval')
+        self.assertFalse(changes.save_all())
         usermain.remove_section('Msec')
         userhigh.remove_section('Hsec')
         userkeys.remove_section('Ksec')

Lib/test/cfgparser.1

+# Also used by idlelib.test_idle.test_config.
 [Foo Bar]
 foo=newbar

Misc/NEWS.d/next/IDLE/2017-07-11-02-26-17.bpo-30899.SQmVO8.rst

+IDLE: Add tests for ConfigParser subclasses in config. Patch by Louie Lu.