-
-
Notifications
You must be signed in to change notification settings - Fork 31.6k
IDLE: Add tests for configdialog keys tab #75185
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
Comments
I'm working on the tests for this. |
Great. It is non-trivial, so I don't expect perfection first try. Please revise docstring with explanation of how page works, like with Font and General pages. I finished tests using those explanations as a guide. They are also a guide for review. |
I'm making good progress on this. I should be able to open the PR tomorrow. It's definitely non-trivial, but using your font and general tests as a guide has been invaluable. |
I tried to minimize the changes to configdialog. There were more I wanted to make, like reorganizing the order of the functions, but I didn't want to make the diff too crazy. I changed some variable names because all of them had 'keys' in it and I couldn't keep track of them. It was almost as descriptive as single letter variables! ;-) I think I made the new names inline with what had been done on the font and general tabs. Again, there were more renaming changes I wanted to make (such as key_set to keyset), but I didn't want every line to be a diff. Also, I used the name 'keyset' to refer to the section names and the keybindings to refer to the option:value pairs. Some old vars in configdialog weren't changed to reflect this 100%, but the comment should be correct. I did not include deactivate_current_config and activate_config__changes since those are not really specific to keys. There are also invoked from themes and from apply. I have notes for some of the other coding changes I'd like to make once this is in its own class. |
I am not sure I agree with all your suggested name changes. But I am looking as more substantive issues first. |
I have thought about factoring out common test code into helper functions either within test_configdialog or in a new module. One example: simulate a click on a listbox, with common before and after code. Another: Testing the group of selection widgets on both theme and keys pages. (If I were not planning to replace this group, we could factor out a SelectionFrame and put an instance on each page.) We might do some such refactoring after this, before HighPageTest. Keep notes on redundancies you notice that might be candidates. |
Lines not yet covered in keys group, other than deactivate_current_config and activate_config_changes functions: def save_new_key_set def delete_custom_keys |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: