bpo-31003: IDLE - Add more tests for General tab by terryjreedy · Pull Request #2859 · python/cpython

terryjreedy · Jul 25, 2017

https://bugs.python.org/issue31003

Collect general tab functions together.

Adapt tests to new names and test all radiobuttons.

terryjreedy · Jul 25, 2017

WIP: Preparation, now ready to add tests. Each chunk is a coherent diff, and I will try to continue that way.

terryjreedy · Jul 26, 2017

I think this is complete.

mlouielu

I haven't looked deeply to test code, the test pass on Linux.

I have some comment for configdialog.py, some place should be more clarify.

mlouielu · Jul 26, 2017

Lib/idlelib/configdialog.py

-            helplist_item_remove: Command for button_helplist_remove.
-            update_user_help_changed_items: Fill in changes.
+        Enable users to provisionally change general options.
+        Load_general_cfg loads current values.  Radiobuttons


Load_general_cfg -> load_general_cfg

how about "loads current settings" ?

I added 'Function' to sentence in FontTab docstring and copied the form here: "Function load_general_cfg intializes tk variables and helplist using idleConf."

mlouielu · Jul 26, 2017

Lib/idlelib/configdialog.py

-            update_user_help_changed_items: Fill in changes.
+        Enable users to provisionally change general options.
+        Load_general_cfg loads current values.  Radiobuttons
+        startup_shell_on and startup_editor_on set var startup_edit.


IMO the tk_var should add startup_edit (backtick), otherwise I found that is hard to read the comment.

I used 'sets var x_y' in the FontTab docstring. I tried Var instead of var. We could use 'tk variable' to be really explicit, but I was hoping 'var' or 'Var' as an abbreviation (StringVar, IntVar, etc) would be clear enough. I am going to push as is, but we can revisit this in 'extract class' issues where function docstring become class docstring. bpo-31004, bpo-31050

mlouielu · Jul 26, 2017

Lib/idlelib/configdialog.py

+        startup_shell_on and startup_editor_on set var startup_edit.
+        Radiobuttons save_ask_on and save_auto_on set var autosave.
+        Entry boxes win_width_int and win_height_int set var win_width
+        and win_height.  Setting vars invokes var_changed_var_name


var_changed_varname (e.g. startup_edit -> var_changed_startup_edit)

Change to "Setting var_name invokes the var_changed_var_name callback...", which I hope is clearer.

mlouielu · Jul 26, 2017

Lib/idlelib/configdialog.py

+        "Load current configuration settings for the general options."
+        # Set startup state.
+        self.startup_edit.set(idleConf.GetOption(
+                'main', 'General', 'editor-on-startup', default=0, type='bool'))


default=0 was changed (previous is default=1), does this change right?

The default is only needed if config-main.def is corrupted, which should be never. The win size calls do not give a default. config-main.def currently has 0 and default in call, if given, should be same.

mlouielu · Jul 26, 2017

Lib/idlelib/idle_test/mock_idle.py

        self.kwds = kwds
        if isinstance(self.result, BaseException):
            raise self.result
+        elif self.return_self:


Do this need to suppress when isinstance(self.result, BaseException) is True?

I am not sure what 'suppression' you are proposing, but I really want to be able to raise any exception. Calltip tests should include one with eval mocked with Func(BaseException) to raise BaseException. (see code).

csabella · Jul 26, 2017

Lib/idlelib/configdialog.py

-                    (*)radio_startup_edit: Radiobutton - startup_edit
-                    (*)radio_startup_shell: Radiobutton - startup_edit
+                    (*)startup_editor_on: Radiobutton - startup_edit
+                    (*)startup_shell_on: Radiobutton - startup_edit


This is trivial, but with the rename (and since this is moving to its own class soon), I had been thinking about the variable being startup_window and then the two choices just being shell and editor.

If we were starting fresh..., but we are not. We cannot change config-main.def.
[General]
editor-on-startup= 0
This is read as a boolean, and startup_editor should be BooleanVar, not IntVar.

What we show users When tests are merged, we can review code and layout. I intend to remove three top frame and change to:

Window to open at startup: O Shell O Editor
Initial size: Width [_] Height []
When run code in editor: ... (see bpo-19042 for possible change)

csabella · Jul 26, 2017

Lib/idlelib/configdialog.py

-                    (*)radio_save_ask: Radiobutton - autosave
-                    (*)radio_save_auto: Radiobutton - autosave
+                    (*)save_ask_on: Radiobutton - autosave
+                    (*)save_auto_on: Radiobutton - autosave


Same here as above, I was hoping to simplify the names to autosave and the choices to prompt and noprompt.

For this issue, I focused on the names I knew would be most used in tests. We can continue changes in bpo-31050 or bpo-31051.

csabella · Jul 26, 2017

Lib/idlelib/configdialog.py

@@ -735,16 +737,16 @@ def create_page_general(self):
        win_height_title = Label(frame_win_size, text='Height')
        self.entry_win_height = Entry(
                frame_win_size, textvariable=self.win_height, width=3)


You changed entry_win_height and entry_win_width in the comment, but not the code.

With the issue mentioned on the ticket, would this help for validating the entry widget?
https://stackoverflow.com/questions/4140437/interactively-validating-entry-widget-content-in-tkinter#4140988

See new comment on bpo-31051.

csabella · Jul 26, 2017

Lib/idlelib/configdialog.py

-        self.list_help.bind('<ButtonRelease-1>', self.help_source_selected)
+        scroll_helplist = Scrollbar(frame_helplist)
+        scroll_helplist.config(command=self.helplist.yview)
+        self.helplist.config(yscrollcommand=scroll_helplist.set)


Did you want to change config like you did for 'state'?

csabella · Jul 26, 2017

Lib/idlelib/configdialog.py

+        for num in range(1, len(self.user_helplist) + 1):
+            changes.add_option(
+                    'main', 'HelpFiles', str(num),
+                    ';'.join(self.user_helplist[num-1][:2]))


Would this be a good time to change it to for num, item in enumerate(self.user_helplist, 1): and the last line to .join(item[:2])?

in code and text, matching comment. (CSabella comment).

csabella · Jul 26, 2017

Lib/idlelib/idle_test/test_configdialog.py

+
+    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.


Too much text from Font test.

* In configdialog: Document causal pathways in create_page_general. Move related functions to follow this. Simplify some attribute names. * In test_configdialog: Add tests for load and helplist functions. Coverage for the general tab is now complete, and 63% overall. (cherry picked from commit 2bc8f0e)

terryjreedy added 4 commits July 25, 2017 01:27

bpo-31003: IDLE - Add tests for General tab.

3b2b386

Collect general tab functions together.

News blurb.

3e37df2

Describe start, save, and size options. Rename widgets.

976d172

Adapt tests to new names and test all radiobuttons.

Explain help list; rename some widgets and functions.

9736a53

the-knights-who-say-ni added the CLA signed label Jul 25, 2017

terryjreedy changed the title ~~Helper~~ bpo-31003: IDLE - Add more tests for General tab Jul 25, 2017

Add remain general tag tests; use widget mapping instead of config

107524f

mlouielu suggested changes Jul 26, 2017

View reviewed changes

csabella reviewed Jul 26, 2017

View reviewed changes

Change entry_win_width(height) to win_width(height)_int

fd5b9e0

in code and text, matching comment. (CSabella comment).

csabella reviewed Jul 26, 2017

View reviewed changes

terryjreedy added 5 commits July 26, 2017 15:18

Use mapping interface (CSabella comment).

c1bc8f7

Docstring changes in response to Louie.

e7b9aa0

Fix GeneralTest docstring (CSabella)

c43fa46

coverage and whitespace

073b73d

Merge branch 'master' into helper

34ab6e4

terryjreedy merged commit 2bc8f0e into python:master Jul 27, 2017

terryjreedy deleted the helper branch July 27, 2017 00:54

Search code, repositories, users, issues, pull requests...

Uh oh!

Conversation

terryjreedy commented Jul 25, 2017 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

terryjreedy commented Jul 25, 2017

Uh oh!

terryjreedy commented Jul 26, 2017

Uh oh!

mlouielu left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

terryjreedy Jul 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

terryjreedy commented Jul 25, 2017 •

edited by bedevere-bot

Loading

mlouielu left a comment •

edited

Loading

terryjreedy Jul 26, 2017 •

edited

Loading