This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Title: IDLE: Improve configuration tests with mock Save.
Type: enhancement Stage: needs patch
Components: IDLE Versions: Python 3.7, Python 3.6
Status: open Resolution:
Dependencies: Superseder:
Assigned To: terry.reedy Nosy List: cheryl.sabella, terry.reedy
Priority: normal Keywords:

Created on 2017-07-07 02:56 by terry.reedy, last changed 2022-04-11 14:58 by admin.

Messages (5)
msg297854 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017-07-07 02:56
When writing tests for config and configdialog, I blocked the user configs from hitting the filesystem by passing '' as the file name.  This disables the Save function.  To know that Save is called, a mock is needed.  To know what would have been written to disk, the mock could write to a StringIO.  Since there are 4 files and 4 config parsers, there are 4 save functions.  I would mock each separately so it can be determined that the right one was called.  I plan to create a specialized version of idle_mock.Func with a simplified version of Save's body in the __call__ method.
msg297970 - (view) Author: Cheryl Sabella (cheryl.sabella) * (Python committer) Date: 2017-07-08 20:03
If it's helpful, here's some history on Save and save_as in configdialog.

Item: Code change which added the lines to always save 'highlight' and 'keys'.

November 16, 2004, commit 5acdf9308191b6356fb3ed4ba691ba5cd391f202

commit message:
 Saving a Keyset w/o making changes (by using the "Save as New Custom …

…Key Set"

button) caused IDLE to fail on restart (no new keyset was created in
config-keys.cfg).  Also true for Theme/highlights.  Python Bug 1064535.

Item: Code change to always save 'main' first.  Not much information, but seems to be part of the handling of the help source changes.

March 26, 2002  commit 0c5bc8c9518fd18d41aedede536c4a9aa8dfa619 

Commit message:  further work on new config system;

user defined help items
msg297975 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017-07-09 01:46
Very helpful.  Is there a git equivalent to hg annotate, or did you have to trudge through the commit log?
msg297983 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017-07-09 03:55
This issue began with #27173 which added test_config with CurrentColorKeysTest to test the new IdleConf.current_colors_and_keys.  Those tests never called any user parser.Save, but I knew I had to remove the possibility that any future test would touch anyone's working .idlerc.  I thought of at least some of the following:

1. Directly replace config.IdleUserConfParser.Save.  Since 4 instances are created during import, this would require recreating idleCong.userCfg after import.  Restore the status quo by saving the original userCfg and restoring it after tests.  The same applies to 3. and 4.

2. Give instances a Save function, as Cheryl did in PR2610, 
test_config.ConfigChangesTest.test_save_all, to mask the instance method.
        idleConf.userCfg['main'].Save = Func()
Restore the status quo by removing the instance function.

3. Subclass IdleUserConfigParser with a new Save and recreate userCfg, as Cheryl did in on #30868.  But instead of a dummy function, the replacement should be a mock.
class DummyUserConfParser(config.IdleUserConfParser):
    Save = Func()  # or other Mock.
This is essentially 1. with subclassing being an alternate way to replace the original Save.

4. It turns out the initializing IdleUserConfParser with file name '' works to disable both Load and Save without raising an exception.  (Plain open('') gives me FileNotFoundError.)  So, recreate userCfg with instances initialized with ''.

When I opened this issue, I forgot about the no load part.  But it is in the  comment near the top of test_config, and it is essential.  This suggests combining 3. and 4. by initializing a subclass with ''.

A. Testing ConfigChanges.save_all requires a count of calls.  Func can easily be upgraded to do that.

B. The real save function does not just save a page.  Instead, it removes empty sections and if the result is an empty page, the corresponding file is deleted, not just emptied. Testing the function requires a fairly functional substitute.  I am not sure yet how the solution for this should relate to A and its solution.

In #8231, I tried and failed to get IDLE to run with a temporary directory as the home directory, so people without a writeable home directory can still run IDLE.  Having to run with 2.7 is no more, for me, an issue.  Running in 2 processes, and having config indirectly inported into the run process is.  As I noted there, #27534 could result in this issue going away.

I also want to look into a simulated in-memory 'directory', class IdleRC, contains StringIO instances, with a simulated open and any needed os functions.  (For instance, mock-open('.idlerc/config-main.cfg') returns the StringIO for 'main'.)  This would be fast and instrumented for testing as we please.

C. 'Other tests' at the top of this post include both other tests in test_config and other test files.  If Solutions 1. to 4. have to include restoration, then they should be repeated in every test_file that imports config.  So why bother restoring, only to repatch?  This suggests that when testing, usefCfg should be created in an alternate state.

However, ... the current mechanism is that test.test_idle sets idlelib.__init__.testing to True.  This is fine for buildbots, which only run idle test by running test_idle.  So an IdelConf.set_testing, called from test_xyz would be needed.

D. Other modules write other configuration files into .idlerc. When a file is opened, it is added or moved to the top of recent-files.lst.  When the debugger is on and a file with breakpoints is run, the filename and breakpoint lines are supposed to be saved to breakpoints.lst.  (Withoug tests, this could have stop working without me noticing.)

So disabling parser saves during tests is not enough.  The other .idlerc saves must also be blocked.  Some sort of temporary directory is needed.

E. On the next issue after this, #30869, Victor Stinner requested that IDLE not even create $HOME/.idlerc on buildbots.

F. There systems on which IDLE does not find a place to write .idlerc.  IDLE now quits.  Any temporary directory would be better, at least as a choice.  See #8231 and #15862.
msg297990 - (view) Author: Cheryl Sabella (cheryl.sabella) * (Python committer) Date: 2017-07-09 11:06
`git log` is the magic command.  I won't be able to do it justice because it has too many options, but here are a few.

`git log` shows all commit message history on the current branch.
`git log -p` (p for patch) shows the actual diffs
`git log -p -- path/to/file` shows the diffs for one file
`git log --grep` (with or without file) greps the commit messages
`git log -Sword` (with or without file) greps the file content for word

I don't know how gitgui integrates those commands.

Having said that, I also don't know how to look at branches, so because of the name change on the file, I looked at 3.3 in github and trudged through.  But, it was more of a divide and conquer, so it didn't take long.  I randomly opened a commit and checked the file for the line changes.  I'm sure there would have been a way to do it with git log.
Date User Action Args
2022-04-11 14:58:48adminsetgithub: 75051
2017-07-09 11:06:22cheryl.sabellasetmessages: + msg297990
2017-07-09 03:55:56terry.reedysetmessages: + msg297983
2017-07-09 01:46:58terry.reedysetmessages: + msg297975
2017-07-08 20:03:19cheryl.sabellasetnosy: + cheryl.sabella
messages: + msg297970
2017-07-07 02:56:35terry.reedycreate