classification
Title: IDLE: Don't touch .idlerc when testing.
Type: behavior Stage: needs patch
Components: IDLE Versions: Python 3.8, Python 3.7
process
Status: open Resolution:
Dependencies: 8231 15862 Superseder:
Assigned To: terry.reedy Nosy List: louielu, terry.reedy, vstinner
Priority: normal Keywords:

Created on 2017-07-07 03:21 by louielu, last changed 2019-03-20 19:35 by terry.reedy.

Pull Requests
URL Status Linked Edit
PR 2614 closed louielu, 2017-07-07 03:22
Messages (7)
msg297856 - (view) Author: Louie Lu (louielu) * Date: 2017-07-07 03:21
In bpo #30780, there is a mistake of tearDownModule didn't restore the use rCfg.

To prevent future mistake, Adding .idlerc to regrtest saved_test_environment, so that `--fail-env-changed` option can detect .idlerc been changed in IDLE test.
msg297858 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017-07-07 04:32
I would have liked the mistake in #30780 to have been noticed, but I don't think the patch would have caught it.  What I failed to restore was an in memory date structure that is part of the config module.   The patch seems to be concerned only with the filesystem, which is something different.

On IDLE startup, config tries 1. to find a home directory, 2. within that a .idlerc directory, and 3. within that user config files.  If 1. fails, it used getcwd() as home.  Either way, if 2. fails, config tries to make .idlerc.  Failure here is currently fatal, though I would like to eventually change that.  So I presume creation succeeds at least once on each buildbot.  Creating .idlerc is intentional behavior, not a mistake, and in the absence of buildbot owners complaining, I don't think it should be made a test failure.  Creating non-empty user config files during a test (on user machines) would be a mistake, and I have made sure that tests do not touch them.  Checking for user config changes within .idlerc would be okay,

Victor, it is up to you as regrtest expert to review both the implied policy change and the effect of the code change.
msg297874 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-07-07 09:42
I dislike the idea of opening all $HOME/.idlerc/ directory and store their content in memory.

If a unit test writes into $HOME, we already have an issue. Unit tests must not modify the user configuration. If an unit test is interrupted, there is a high risk of leaving the modified configuration.

If idle tests need a well defined configuration, I suggest to start by modifiying idlelib.config.idleConf to use a temporary directory, or mock the whole object.
msg297875 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-07-07 09:42
test_site has a similar issue: bpo-30227, "test_site must not write outside the build directory: must not write into $HOME/.local/".
msg297917 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017-07-07 23:10
The current tests do not write anything.  But IDLE currently requires the existence of .idlerc to run, and tries to write it if not found.  Changing this requirement without disabling multiple features would not be trivial.  On possibility might be to replace builtin open with a wrapper that returned named StringIO objects when the path began with '.idlerc/'.  The StringIO would have to be cached so that closing and reopening the same path got the same object. But this is only good for one process. See below.

There are two existing issues, #8231 and #15862, about HOME not being writable and not existing.  The first has a patch I wrote 2 years ago trying to do what you suggest above.  It used tempfile.mkdtemp for 2.7 compatibility, by I would now use TemporaryDirectory. The discussion ended at that time with some unresolved issues.

One in particular is that IDLE runs with two processes, with the user execution process being based on run.py.  One of the idlelib modules imported by run eventually imports config. See msg253681 and msg253714.

The is already a mechanism for test_idle to tell idlelib module 'testing'.  If and when #8231 is solved, that mechanism could be used to ignore $HOME.
msg297985 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017-07-09 04:04
#27534 is about reducing run's imports.  I believe that the current patch may remove one path by which config is indirectly imported.  While writing msg297983 of #30868, I remembered that config files are not the only files put in .idlerc.  So a solution to block writing config files while testing is not enough.  An in-memory .idlerc with StringIOs for files, both for testing and for user systems without a place to write it, is looking more inviting to me than it did yesterday.
msg338506 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2019-03-20 19:35
I closed PR 2614 in response to Victor's comments, in particular his preference that tests not even read .idlerc.  Since we do not want tests to depend on the contents, there is no need to even read the contents.  (And the exception of testing write-read round-tripping requires a fresh temporary directory.)

I propose that when testing, config.py should create idleConf.usercfg empty, without reference to .idlerc.  The result would be the same as now after usercfg is replaced with testcfg, as in test_configdialog, without the need to revert the replacement.

test_idle has the equivalent of
  import idlelib
  idlelib.testing = True
The same should be added to any test module that sets options.  Then idlelib.testing can be used to change config.py behavior.

Victor: what changes to a module trigger the 'environment changed' warning/error?  The above unreverted change to idlelib.__init__ does not.  The unreverted change to idleCong in test_configdialog did, until fixed.
History
Date User Action Args
2019-03-20 19:35:54terry.reedysetassignee: terry.reedy
components: + IDLE, - Library (Lib)
title: regrtest: Add .idlerc to saved_test_environment -> IDLE: Don't touch .idlerc when testing.
type: behavior
versions: + Python 3.8
messages: + msg338506
stage: needs patch
2017-07-09 04:04:46terry.reedysetmessages: + msg297985
2017-07-07 23:10:49terry.reedysetdependencies: + Unable to run IDLE without write-access to home directory, IDLE: startup problem when HOME does not exist
messages: + msg297917
2017-07-07 09:42:57vstinnersetmessages: + msg297875
2017-07-07 09:42:13vstinnersetmessages: + msg297874
2017-07-07 04:32:38terry.reedysetmessages: + msg297858
2017-07-07 03:22:06louielusetpull_requests: + pull_request2679
2017-07-07 03:21:06louielucreate