-
-
Notifications
You must be signed in to change notification settings - Fork 32.1k
bpo-30917: IDLE: Add config.IdleConf unittest #2691
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.
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
Changes from 2 commits
4d90afc
9fc7670
d4dac80
eb32d91
ddedbc7
06d8329
f29e0da
3f2fdcc
e445a0e
0f1607d
980a801
4ecfb1a
d9b3702
511b726
49e0534
7fba576
206ae1e
c850b7f
ef45060
7627733
2cbb960
c94d4b5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -159,14 +159,15 @@ class IdleConf: | |
for config_type in self.config_types: | ||
(user home dir)/.idlerc/config-{config-type}.cfg | ||
""" | ||
def __init__(self): | ||
def __init__(self, _utest=False): | ||
self.config_types = ('main', 'highlight', 'keys', 'extensions') | ||
self.defaultCfg = {} | ||
self.userCfg = {} | ||
self.cfg = {} # TODO use to select userCfg vs defaultCfg | ||
self.CreateConfigHandlers() | ||
self.LoadCfgFiles() | ||
|
||
if not _utest: | ||
self.CreateConfigHandlers() | ||
self.LoadCfgFiles() | ||
|
||
def CreateConfigHandlers(self): | ||
"Populate default and user config parser dictionaries." | ||
|
@@ -463,16 +464,8 @@ def GetExtensions(self, active_only=True, | |
|
||
def RemoveKeyBindNames(self, extnNameList): | ||
"Return extnNameList with keybinding section names removed." | ||
# TODO Easier to return filtered copy with list comp | ||
names = extnNameList | ||
kbNameIndicies = [] | ||
for name in names: | ||
if name.endswith(('_bindings', '_cfgBindings')): | ||
kbNameIndicies.append(names.index(name)) | ||
kbNameIndicies.sort(reverse=True) | ||
for index in kbNameIndicies: #delete each keybinding section name | ||
del(names[index]) | ||
return names | ||
return sorted( | ||
[n for n in extnNameList if not n.endswith(('_bindings', '_cfgBindings'))]) | ||
|
||
def GetExtnNameForEvent(self, virtualEvent): | ||
"""Return the name of the extension binding virtualEvent, or None. | ||
|
@@ -878,7 +871,8 @@ def clear(self): | |
|
||
|
||
# TODO Revise test output, write expanded unittest | ||
def _dump(): # htest # (not really, but ignore in coverage) | ||
def _dump(): # pragma: no cover | ||
# htest # (not really, but ignore in coverage) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no need to annotate things that coverage skips (it is now documented ;-). |
||
from zlib import crc32 | ||
line, crc = 0, 0 | ||
|
||
|
@@ -907,7 +901,8 @@ def dumpCfg(cfg): | |
dumpCfg(idleConf.userCfg) | ||
print('\nlines = ', line, ', crc = ', crc, sep='') | ||
|
||
if __name__ == '__main__': | ||
|
||
if __name__ == '__main__': # pragma: no cover | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This skip comes with coverage. I am reviewing now and will probably have other edits. If so, will do this too. |
||
import unittest | ||
unittest.main('idlelib.idle_test.test_config', | ||
verbosity=2, exit=False) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,7 @@ | |
import tempfile | ||
from test.support import captured_stderr, findfile | ||
import unittest | ||
from unittest import mock | ||
from idlelib import config | ||
|
||
# Tests should not depend on fortuitous user configurations. | ||
|
@@ -185,6 +186,288 @@ def test_save(self): | |
self.assertFalse(os.path.exists(path)) | ||
|
||
|
||
class IdleConfTest(unittest.TestCase): | ||
"""Test for idleConf""" | ||
|
||
def new_config(self, _utest=False): | ||
return config.IdleConf(_utest=_utest) | ||
|
||
def mock_config(self): | ||
"""Return a mocked idleConf | ||
|
||
Both default and user config used the same config-*.def | ||
""" | ||
import sys | ||
conf = config.IdleConf(_utest=True) | ||
if __name__ != '__main__': | ||
idle_dir = os.path.dirname(__file__) | ||
else: | ||
idle_dir = os.path.abspath(sys.path[0]) | ||
for ctype in conf.config_types: | ||
config_path = os.path.join(idle_dir, '../config-%s.def' % ctype) | ||
conf.defaultCfg[ctype] = config.IdleConfParser(config_path) | ||
conf.userCfg[ctype] = config.IdleUserConfParser(config_path) | ||
conf.LoadCfgFiles() | ||
|
||
return conf | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The point of this is unclear just from reading. Having duplicate info in defaultCfg and userCfg is contrary to intended reality (though users can over-ride by hand editing). In general, I think userCfg should be loaded with fake data with read_string the way I did in the CurrentColorKeysTest. Re-reading and parsing the .def files twice for each of multiple functions is needlessly slow. The files can be read once and then loaded into empty parsers. Besides being faster for reloads, not passing the actual file name prevents accidentally writing the files. I believe that idleConf never changes defaultCfg. (If true, we could in a followup isssue define a readonly IdleDefConfParser that raised on any attempt to change defaults.) If we want to change a particular default parser for a test, we should read into a particular parser in an expanded testCfg. I did not yet make any changes based on the above. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I make this to be done at |
||
|
||
def test_get_user_cfg_dir(self): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I noticed some while ago that IdleConf.GetUserCfgDir is not really a method, in that it does not use the self parameter (idleConf). I have been planning to make it a module function, and with tests written, did it, and moved the test function into a new class, along with that for _warn. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this can be reference from ipython source code, it turyly seperate this function from a class like what we do here. https://github.com/ipython/ipython/blob/master/IPython/utils/path.py#L172 I think this will need to be discuss on mailing list or bpo. But I prefer to first finish the unittest, then do other stuff to change the behavior. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In config, rename function and move to module level. +def get_user_directory():
+ """Return a filesystem directory for storing user config files.
+# etc
- self.userdir = userDir = self.GetUserCfgDir()
+ self.userdir = userDir = get_user_directory()
- def GetUserCfgDir(self):
- """Return a filesystem directory for storing user config files.
- # etc There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In test_config +class FunctionTest(unittest.TestCase):
+ "Test module functions."
+
+ def test_get_user_directory(self):
+ getdir = config.get_user_directory
+ # etc, with getdir replacing call in code as it was.
Move test_warn into same class. |
||
"Test to get user config directory" | ||
conf = self.new_config(_utest=True) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was only needed to access the function. |
||
|
||
# Check normal way should success | ||
with mock.patch('os.path.expanduser', return_value='/home/foo'): | ||
with mock.patch('os.path.exists', return_value=True): | ||
self.assertEqual(conf.GetUserCfgDir(), '/home/foo/.idlerc') | ||
|
||
# Check os.getcwd should success | ||
with mock.patch('os.path.expanduser', return_value='~'): | ||
with mock.patch('os.getcwd', return_value='/home/foo/cpython'): | ||
with mock.patch('os.mkdir'): | ||
self.assertEqual(conf.GetUserCfgDir(), | ||
'/home/foo/cpython/.idlerc') | ||
|
||
# Check user dir not exists and created failed should raise SystemExit | ||
with mock.patch('os.path.join', return_value='/path/not/exists'): | ||
with self.assertRaises(SystemExit): | ||
with self.assertRaises(FileNotFoundError): | ||
conf.GetUserCfgDir() | ||
|
||
def test_create_config_handlers(self): | ||
conf = self.new_config(_utest=True) | ||
|
||
# Mock out idle_dir | ||
idle_dir = '/home/foo' | ||
with mock.patch.dict({'__name__': '__foo__'}): | ||
with mock.patch('os.path.dirname', return_value=idle_dir): | ||
conf.CreateConfigHandlers() | ||
|
||
# Check keys are equal | ||
self.assertCountEqual(conf.defaultCfg.keys(), conf.config_types) | ||
self.assertCountEqual(conf.userCfg.keys(), conf.config_types) | ||
|
||
# Check conf parser are correct type | ||
for default_parser in conf.defaultCfg.values(): | ||
self.assertIsInstance(default_parser, config.IdleConfParser) | ||
for user_parser in conf.userCfg.values(): | ||
self.assertIsInstance(user_parser, config.IdleUserConfParser) | ||
|
||
# Check config path are correct | ||
for config_type, parser in conf.defaultCfg.items(): | ||
self.assertEqual(parser.file, | ||
os.path.join(idle_dir, 'config-%s.def' % config_type)) | ||
for config_type, parser in conf.userCfg.items(): | ||
self.assertEqual(parser.file, | ||
os.path.join(conf.userdir, 'config-%s.cfg' % config_type)) | ||
|
||
def test_load_cfg_files(self): | ||
conf = self.new_config(_utest=True) | ||
|
||
# Borrow test/cfgparser.1 from test_configparser. | ||
config_path = findfile('cfgparser.1') | ||
conf.defaultCfg['foo'] = config.IdleConfParser(config_path) | ||
conf.userCfg['foo'] = config.IdleUserConfParser(config_path) | ||
|
||
# Load all config from path | ||
conf.LoadCfgFiles() | ||
|
||
eq = self.assertEqual | ||
|
||
# Check defaultCfg is loaded | ||
eq(conf.defaultCfg['foo'].Get('Foo Bar', 'foo'), 'newbar') | ||
eq(conf.defaultCfg['foo'].GetOptionList('Foo Bar'), ['foo']) | ||
|
||
# Check userCfg is loaded | ||
eq(conf.userCfg['foo'].Get('Foo Bar', 'foo'), 'newbar') | ||
eq(conf.userCfg['foo'].GetOptionList('Foo Bar'), ['foo']) | ||
|
||
def test_get_section_list(self): | ||
conf = self.mock_config() | ||
|
||
self.assertCountEqual( | ||
conf.GetSectionList('default', 'main'), | ||
['General', 'EditorWindow', 'Indent', 'Theme', | ||
'Keys', 'History', 'HelpFiles']) | ||
self.assertCountEqual( | ||
conf.GetSectionList('user', 'main'), | ||
['General', 'EditorWindow', 'Indent', 'Theme', | ||
'Keys', 'History', 'HelpFiles']) | ||
|
||
def test_get_highlight(self): | ||
conf = self.mock_config() | ||
|
||
eq = self.assertEqual | ||
eq(conf.GetHighlight('IDLE Classic', 'normal'), {'foreground': '#000000', | ||
'background': '#ffffff'}) | ||
eq(conf.GetHighlight('IDLE Classic', 'normal', 'fg'), '#000000') | ||
eq(conf.GetHighlight('IDLE Classic', 'normal', 'bg'), '#ffffff') | ||
with self.assertRaises(config.InvalidFgBg): | ||
conf.GetHighlight('IDLE Classic', 'normal', 'fb') | ||
|
||
# Test cursor (this background should be normal-background) | ||
eq(conf.GetHighlight('IDLE Classic', 'cursor'), {'foreground': 'black', | ||
'background': '#ffffff'}) | ||
|
||
def test_get_theme_dict(self): | ||
"XXX: NOT YET DONE" | ||
conf = self.mock_config() | ||
|
||
# These two should be the same | ||
self.assertEqual( | ||
conf.GetThemeDict('default', 'IDLE Classic'), | ||
conf.GetThemeDict('user', 'IDLE Classic')) | ||
|
||
with self.assertRaises(config.InvalidTheme): | ||
conf.GetThemeDict('bad', 'IDLE Classic') | ||
|
||
def test_current_colors_and_keys(self): | ||
conf = self.mock_config() | ||
|
||
self.assertEqual(conf.current_colors_and_keys('Theme'), 'IDLE Classic') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm wondering if there needs to be more tested here by using SetOption to change the currently active theme or keys. There's a lot in the docstring and code for just one test. If it helps, on config dialog, there is a separate boolean for both themes and keys and defines what kind is currently selected. Meaning, if the user has selected a custom (user) theme, then the boolean is false for is_builtin_theme. If the user has selected a default keyset, then the boolean is true for are_builtin_keys. The first check for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, just got to the other test class for this. You can probably skip my original comment. Sorry about that. |
||
|
||
def test_default_keys(self): | ||
import sys | ||
current_platform = sys.platform | ||
conf = self.new_config(_utest=True) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had a similar test in help_about and Terry said it wasn't necessary to test each branch based on the OS. Maybe his comments don't apply here, but here's a link to the issue: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that is because #2380 is about bitness, that affect isn't quite huge, but this test do will affect user in the different platform (darwin using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cheryl, #2380 was a peculiar situation in the the correct answer depends on the machine, not the OS, and there is no automated way to get the correct answer. But we do the the correct range -- a set of two items. While the default_keys tests mimic the current implementation, I can think of two other strategies, and ways to introduce bugs that would be detected by this test. |
||
|
||
sys.platform = 'win' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make this 'win32'. This is the current platform for consumer Windows regardless of bitness. If someone were to change the current implementation, 'win32' must still work. |
||
self.assertEqual(conf.default_keys(), 'IDLE Classic Windows') | ||
|
||
sys.platform = 'darwin' | ||
self.assertEqual(conf.default_keys(), 'IDLE Classic OSX') | ||
|
||
sys.platform = 'linux' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make this 'somelinux', for similar reason to above. |
||
self.assertEqual(conf.default_keys(), 'IDLE Modern Unix') | ||
|
||
# Restore platform | ||
sys.platform = current_platform | ||
|
||
def test_get_extensions(self): | ||
conf = self.mock_config() | ||
|
||
eq = self.assertEqual | ||
eq(conf.GetExtensions(), | ||
['AutoComplete', 'AutoExpand', 'CallTips', 'CodeContext', | ||
'FormatParagraph', 'ParenMatch', 'RstripExtension', 'ScriptBinding', | ||
'ZoomHeight']) | ||
eq(conf.GetExtensions(active_only=False), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be worthwhile to set some to enable=False before running some of the tests? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, thanks for pointing out. I'll put some extra test data into it. |
||
['AutoComplete', 'AutoExpand', 'CallTips', 'CodeContext', | ||
'FormatParagraph', 'ParenMatch', 'RstripExtension', 'ScriptBinding', | ||
'ZoomHeight']) | ||
eq(conf.GetExtensions(editor_only=True), | ||
['AutoComplete', 'AutoExpand', 'CallTips', 'CodeContext', | ||
'FormatParagraph', 'ParenMatch', 'RstripExtension', 'ScriptBinding', | ||
'ZoomHeight']) | ||
eq(conf.GetExtensions(shell_only=True), | ||
['AutoComplete', 'AutoExpand', 'CallTips', 'FormatParagraph', | ||
'ParenMatch', 'ZoomHeight']) | ||
|
||
def test_remove_key_bind_names(self): | ||
conf = self.mock_config() | ||
|
||
self.assertEqual( | ||
conf.RemoveKeyBindNames(conf.GetSectionList('default', 'extensions')), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. with the sorted call removed from the function, add .sort() here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it better to add .sort() or to use assertCountEqual()? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I changed this to |
||
['AutoComplete', 'AutoExpand', 'CallTips', 'CodeContext', | ||
'FormatParagraph', 'ParenMatch', 'RstripExtension', 'ScriptBinding', | ||
'ZoomHeight']) | ||
|
||
def test_get_extn_name_for_event(self): | ||
conf = self.mock_config() | ||
|
||
eq = self.assertEqual | ||
eq(conf.GetExtnNameForEvent('force-open-completions'), 'AutoComplete') | ||
eq(conf.GetExtnNameForEvent('expand-word'), 'AutoExpand') | ||
eq(conf.GetExtnNameForEvent('force-open-calltip'), 'CallTips') | ||
eq(conf.GetExtnNameForEvent('zoom-height'), 'ZoomHeight') | ||
|
||
def test_get_extension_keys(self): | ||
conf = self.mock_config() | ||
|
||
eq = self.assertEqual | ||
eq(conf.GetExtensionKeys('AutoComplete'), | ||
{'<<force-open-completions>>': ['<Control-Key-space>']}) | ||
eq(conf.GetExtensionKeys('ParenMatch'), | ||
{'<<flash-paren>>': ['<Control-Key-0>']}) | ||
eq(conf.GetExtensionKeys('ZoomHeight'), | ||
{'<<zoom-height>>': ['<Alt-Key-2>']}) | ||
|
||
def test_get_extension_bindings(self): | ||
conf = self.mock_config() | ||
|
||
self.assertEqual(conf.GetExtensionBindings('NotExists'), {}) | ||
self.assertEqual( | ||
conf.GetExtensionBindings('ZoomHeight'), | ||
{'<<zoom-height>>': ['<Alt-Key-2>']}) | ||
|
||
def test_get_current_keyset(self): | ||
import sys | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be removed since it is imported at the top of the test, I'll leave here until @terryjreedy push his commit to GitHub (there is a conflict here) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I not pushing anything until tomorrow my afternoon, to avoid conflict with you. (Its past midnight here.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this has been removed. |
||
current_platform = sys.platform | ||
conf = self.mock_config() | ||
|
||
# Ensure that platform isn't darwin | ||
sys.platform = 'linux' | ||
self.assertEqual(conf.GetCurrentKeySet(), conf.GetKeySet(conf.CurrentKeys())) | ||
|
||
# This should not be the same, sicne replace <Alt- to <Option- | ||
sys.platform = 'darwin' | ||
self.assertNotEqual(conf.GetCurrentKeySet(), conf.GetKeySet(conf.CurrentKeys())) | ||
|
||
# Restore platform | ||
sys.platform = current_platform | ||
|
||
def test_is_core_binding(self): | ||
# XXX: Should move out the core keys to config file or other place | ||
conf = self.mock_config() | ||
|
||
self.assertTrue(conf.IsCoreBinding('copy')) | ||
self.assertTrue(conf.IsCoreBinding('cut')) | ||
self.assertTrue(conf.IsCoreBinding('del-word-right')) | ||
self.assertFalse(conf.IsCoreBinding('not-exists')) | ||
|
||
def test_get_extra_help_source_list(self): | ||
conf = self.mock_config() | ||
|
||
self.assertEqual(conf.GetExtraHelpSourceList('default'), []) | ||
self.assertEqual(conf.GetExtraHelpSourceList('user'), []) | ||
with self.assertRaises(config.InvalidConfigSet): | ||
self.assertEqual(conf.GetExtraHelpSourceList('bad'), []) | ||
|
||
def test_get_all_extra_help_source_list(self): | ||
conf = self.mock_config() | ||
|
||
self.assertCountEqual( | ||
conf.GetAllExtraHelpSourcesList(), | ||
conf.GetExtraHelpSourceList('default') + conf.GetExtraHelpSourceList('user')) | ||
|
||
def test_get_font(self): | ||
from tkinter import Tk | ||
from tkinter.font import Font | ||
conf = self.mock_config() | ||
|
||
root = Tk() | ||
root.withdraw() | ||
f = Font.actual(Font(name='TkFixedFont', exists=True, root=root)) | ||
|
||
self.assertEqual( | ||
conf.GetFont(root, 'main', 'EditorWindow'), | ||
(f['family'], 10 if f['size'] < 10 else f['size'], f['weight'])) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. End function with root.destroy() so that children are gone before root disappears. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. updated. |
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know if you saw this for TkFixedFont. Basically, the code picks the best Font for the platform instead of always using the default. I think it might be OK to mock TkFixedFont and f, but maybe it's not necessary. |
||
def test_get_core_keys(self): | ||
conf = self.mock_config() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you want to mock out _warn as you did in other tests? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm, do you point the wrong one? I didn't get any stdout in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, I meant that the method for GetCoreKeys has calls to _warn, so I thought you might need to cover them. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mocked _warn last night to prevent printed output. Adding more code to provoke calls to warn, and assert that it is called, is a good idea, and needed for complete coverage. But that can be a follow-on PR by any one of us. Reviewing such new code will be easier with this merged. Requesting a fix for a faulty test that does not test what it claims to test is a different matter. You did well to pick up the number order = alpha order thing, which I missed. |
||
|
||
eq = self.assertEqual | ||
eq(conf.GetCoreKeys()['<<center-insert>>'], ['<Control-l>']) | ||
eq(conf.GetCoreKeys()['<<copy>>'], ['<Control-c>', '<Control-C>']) | ||
eq(conf.GetCoreKeys()['<<history-next>>'], ['<Alt-n>']) | ||
eq(conf.GetCoreKeys('IDLE Classic Windows')['<<center-insert>>'], | ||
['<Control-Key-l>', '<Control-Key-L>']) | ||
eq(conf.GetCoreKeys('IDLE Classic OSX')['<<copy>>'], ['<Command-Key-c>']) | ||
eq(conf.GetCoreKeys('IDLE Classic Unix')['<<history-next>>'], | ||
['<Alt-Key-n>', '<Meta-Key-n>']) | ||
eq(conf.GetCoreKeys('IDLE Modern Unix')['<<history-next>>'], | ||
['<Alt-Key-n>', '<Meta-Key-n>']) | ||
|
||
|
||
class CurrentColorKeysTest(unittest.TestCase): | ||
""" Test colorkeys function with user config [Theme] and [Keys] patterns. | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The original deleted in_place and did not sort. It was a roundabout version of
I checked that in-place and sorted are not needed. In fact, when doing cleanups, this should be a set comprehension. The using code can then use set union in place of 3 current lines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was the
sorted
to be removed?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will if I feel like it.