Skip to content
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

IDLE: Improve configuration tests with mock Save. #75051

Open
terryjreedy opened this issue Jul 7, 2017 · 5 comments
Open

IDLE: Improve configuration tests with mock Save. #75051

terryjreedy opened this issue Jul 7, 2017 · 5 comments
Assignees
Labels
3.7 (EOL) end of life tests Tests in the Lib/test dir topic-IDLE type-feature A feature request or enhancement

Comments

@terryjreedy
Copy link
Member

BPO 30868
Nosy @terryjreedy, @csabella

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:

assignee = 'https://github.com/terryjreedy'
closed_at = None
created_at = <Date 2017-07-07.02:56:35.781>
labels = ['expert-IDLE', 'type-feature', '3.7']
title = 'IDLE: Improve configuration tests with mock Save.'
updated_at = <Date 2017-07-09.11:06:22.147>
user = 'https://github.com/terryjreedy'

bugs.python.org fields:

activity = <Date 2017-07-09.11:06:22.147>
actor = 'cheryl.sabella'
assignee = 'terry.reedy'
closed = False
closed_date = None
closer = None
components = ['IDLE']
creation = <Date 2017-07-07.02:56:35.781>
creator = 'terry.reedy'
dependencies = []
files = []
hgrepos = []
issue_num = 30868
keywords = []
message_count = 5.0
messages = ['297854', '297970', '297975', '297983', '297990']
nosy_count = 2.0
nosy_names = ['terry.reedy', 'cheryl.sabella']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'needs patch'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue30868'
versions = ['Python 3.6', 'Python 3.7']

@terryjreedy
Copy link
Member Author

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.

@terryjreedy terryjreedy added the 3.7 (EOL) end of life label Jul 7, 2017
@terryjreedy terryjreedy self-assigned this Jul 7, 2017
@terryjreedy terryjreedy added topic-IDLE type-feature A feature request or enhancement labels Jul 7, 2017
@csabella
Copy link
Contributor

csabella commented Jul 8, 2017

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 5acdf93

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 0c5bc8c

Commit message: further work on new config system;

user defined help items
--------------

@terryjreedy
Copy link
Member Author

Very helpful. Is there a git equivalent to hg annotate, or did you have to trudge through the commit log?

@terryjreedy
Copy link
Member Author

This issue began with bpo-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 configdialog_tests_v1.py on bpo-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 bpo-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, bpo-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, bpo-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 bpo-8231 and bpo-15862.

@csabella
Copy link
Contributor

csabella commented Jul 9, 2017

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

https://stackoverflow.com/questions/1337320/how-to-grep-git-commit-diffs-or-contents-for-a-certain-word#1340245

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.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
@erlend-aasland erlend-aasland added the tests Tests in the Lib/test dir label Jul 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.7 (EOL) end of life tests Tests in the Lib/test dir topic-IDLE type-feature A feature request or enhancement
Projects
Status: No status
Development

No branches or pull requests

3 participants