bpo-30981: IDLE: Add more configdialog font page tests.

[csabella]

https://bugs.python.org/issue30981

I can edit your comment.

[terryjreedy]

Cheryl, thank you for fixing the main flaw in what I did (binding Func() to the wrong reference). I will explain the remaining touchups after I test them.

[terryjreedy]

Commit message, 2nd line:
Test bold_toggle along with its command, set_samples.

Cheryl, after I pushed c4cf031, github will no longer let me push changes, which means no web edit either. It is like github unchecked the box that gives core devs permission to change a pr. So please push a patch that removes the now unneeded import of Boolean Var and the trailing whitespace that I left in test_configdialog. And recheck the box if you can find it. Hold off on a patch. I am going to try to make a new PR.

[terryjreedy]

There were two problems with your patch. First, deleting local variables at the end of the function is either useless or redundant; they are deleted anyway when the function exits. Doing so will not reverse mutations of persistent objects, like change the command of bold_toggle.

More important, the purpose of testing bold_toggle test is to confirm that clicking the button changes the sample displays (as well as the 'variable'). The is the 'user benefit'. Your test only confirms that invoke() (and presumably a click) really calls the command, as documented. Failure would be a tkinter or tk bug, not an IDLE bug. A separate test was needed to test that the command had originally been set_samples before being mocked.

When a 'command' option is set to a Python function, it is wrapped in a tk tcl function and option 'command' is set to the Python string name of the tcl function. Currently, print(d.bold_toggle['command']) prints nnnnnnnnset_samples, where each n is a digit. It appears that assertTrue(bold_toggle['command'].endswith('set_samples')) would have worked, with the clean-up fixed. But I decided for bold_toggle to fiddling with the option and test the widget and its command together. (This is similar to testing fontlist and its selection handler together.)

[csabella]

Thank you again for explaining that. I"m trying so hard to understand the reasons for doing certain unit tests in certain ways and I still don't have a good grasp of things, like when to delete variables or even what to test. :-(

I had mocked out font_bold because of the side effect of the trace causing the font_bold changes to be added to changes and thus affecting the result of test_font_set. Currently, if test_set_samples_bold_toggle runs before test_font_set, then test_font_set fails. It appears that the test methods run alphabetically. Is that how you make sure they will run in the correct order?

This is what I get in test_font_set if it's not run first:

======================================================================
FAIL: test_font_set (main.FontTabTest)

Traceback (most recent call last):
File "Lib/idlelib/idle_test/test_configdialog.py", line 66, in test_font_set
self.assertEqual(mainpage, expected)
AssertionError: {'EditorWindow': {'font': 'Test Font', 'font-size': '5', 'font-bold': 'True'}} != {'EditorWindow': {'font': 'Test Font', 'font-size': '10', 'font-bold': 'False'}}

• {'EditorWindow': {'font': 'Test Font', 'font-bold': 'True', 'font-size': '5'}}
  ? ^^^ ^

• {'EditorWindow': {'font': 'Test Font', 'font-bold': 'False', 'font-size': '10'}}
  ? ^^^^ ^^

Because changes records the .set() and invoke() calls in test_set_samples_bold_toggle, mainpage no longer matches GetFont in test_font_set.

---

Additional Info:
Realized that changes.clear() should be clearing the changes and thus one test shouldn't affect another. The issue is that var_changed_font sets the value of changes on all three font variables and it has font_size and font_bold as carryover from test_set_samples_bold_toggle. So, if the tests are run out of order, changes doesn't match GetFont because of the dialog.font_name.set() in test_font_set. Is this a testing issue where font_size and font_bold need to be initialized in test_font_set? Or is it an issue with clearing changes that the actual Tk variables aren't changed back to some default when changes is cleared?

[terryjreedy]

Testing should test that code fulfills it purpose. This is why I added the docstring paragraph about the purpose and strategy of the font page. In this case, there are two causal pathways (cause effect chains) leading from each user action to two results. The result is a directed acyclic graph of actions: user action, event generation, Var.set() call, method call, tk wrapper call, or widget['option'] setting). It constitutes the tactics of the page. Each test function tests a portion of the graph. The should collectively test the entire graph. Given that we cannot put a png into the docstring ;-), I will try to rework the lists of Var and methods into a description of the graph.

I noticed the temporary failure of test_font_set and should have investigated, but let myself be distracted by github first adding past commits to my PRs and then messing up this one. Thanks for the reminder. Test functions passing should preferably not depend on their arbitrary names and consequent order. I had though of calling the combined test 'test_bold_toggle_and_set_samples'. Doing so reproduces the failure because bold is toggled 3 times. The issue is that size and bold need to be initialized in test_font_set because they are not initialized in changes.clear (and should and cannot be).

This is related to the fact that 1 font var change write 3 changes, because maybe at one time all 3 were written to user config-main.cfg, but I don't think that is true now, so I am not sure that the triple add_option is really needed. If changes were initialized with default font options, test_font_set would be okay. But that might cause other problems. Another issue.

[terryjreedy]

bpo-30993 is followup for this