From 50c9435c9b73b39fcf79cc3e58edc58bb943d0ed Mon Sep 17 00:00:00 2001 From: Louie Lu Date: Thu, 13 Jul 2017 02:05:32 +0800 Subject: [PATCH] bpo-30899: Add unittests, 100% coverage, for IDLE's two ConfigParser subclasses. (#2662) Patch by Louie Lu. --- Lib/idlelib/config.py | 57 +++--- Lib/idlelib/idle_test/test_config.py | 169 +++++++++++++++++- Lib/test/cfgparser.1 | 1 + .../2017-07-11-02-26-17.bpo-30899.SQmVO8.rst | 1 + 4 files changed, 198 insertions(+), 30 deletions(-) create mode 100644 Misc/NEWS.d/next/IDLE/2017-07-11-02-26-17.bpo-30899.SQmVO8.rst diff --git a/Lib/idlelib/config.py b/Lib/idlelib/config.py index bbb3e3cde5..4cf44271f3 100644 --- a/Lib/idlelib/config.py +++ b/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. diff --git a/Lib/idlelib/idle_test/test_config.py b/Lib/idlelib/idle_test/test_config.py index a8e3a3b89d..f5a609f6d1 100644 --- a/Lib/idlelib/idle_test/test_config.py +++ b/Lib/idlelib/idle_test/test_config.py @@ -1,9 +1,13 @@ '''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') diff --git a/Lib/test/cfgparser.1 b/Lib/test/cfgparser.1 index 3387f52eca..0c0e0d3527 100644 --- a/Lib/test/cfgparser.1 +++ b/Lib/test/cfgparser.1 @@ -1,2 +1,3 @@ +# Also used by idlelib.test_idle.test_config. [Foo Bar] foo=newbar diff --git a/Misc/NEWS.d/next/IDLE/2017-07-11-02-26-17.bpo-30899.SQmVO8.rst b/Misc/NEWS.d/next/IDLE/2017-07-11-02-26-17.bpo-30899.SQmVO8.rst new file mode 100644 index 0000000000..665c7cd5bb --- /dev/null +++ b/Misc/NEWS.d/next/IDLE/2017-07-11-02-26-17.bpo-30899.SQmVO8.rst @@ -0,0 +1 @@ +IDLE: Add tests for ConfigParser subclasses in config. Patch by Louie Lu. -- 2.40.0