Skip to content

bpo-31836: Test_code_module now passes with sys.ps1, ps2 set #4070

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

Merged
merged 4 commits into from
Oct 28, 2017

Conversation

terryjreedy
Copy link
Member

@terryjreedy terryjreedy commented Oct 21, 2017

The code module uses sys.ps1 if present or sets it to '>>> ' if not.
Test_code_module now properly tests both behaviors. Ditto for ps2.

https://bugs.python.org/issue31836

Test_code_module assume sys.ps1 is initially absent; make it so.
Add test that code.interact respects existing sys.ps1. Ditto for ps2.
@serhiy-storchaka
Copy link
Member

Wouldn't be better to restore sys.ps1 in test_idle if it is changed here?

@terryjreedy terryjreedy changed the title bpo-31836: Test_code_module now passes if run after test_idle bpo-31836: Test_code_module now passes with sys.ps1, ps2 set Oct 21, 2017
@terryjreedy
Copy link
Member Author

A program that needs sys.ps1/2 in particular states should set the state it needs. Test_code_module needs them both unset and set to a non-default string. Running test_code_module after test_idle exposed the same bug that was exposed previously by running autotest in a Python shell. The current patch prevents the autotest test_code_module failure that I saw previously. See the issue for more.

@terryjreedy
Copy link
Member Author

The full test suite passes on my Win10 machine.

@serhiy-storchaka
Copy link
Member

I think this issue should be fixed on both sides. Tests that change module globals should restore them after testing. And test_code_module should not depend on the current value of sys.ps1, but test all possible situations. The test.support.swap_attr() context manager can help here.

@terryjreedy
Copy link
Member Author

Tests routine mutate sys.modules and add attributes to other modules without 'restoring' them. For instance, test_tk adds at least .font, .test, and .ttk to tkinter. sys.ps1 is at least as expected as these.

However, on reading the IDLE code, I decided it would be improved by adding a PyShell() attribute set from sys.ps1 when it exists, and never modifying sys. (The code module could do the same.) I made this a separate issue (bpo-31858) and PR (#4143). I plan to push both tomorrow.

@serhiy-storchaka
Copy link
Member

I suggest to not change sys.ps1 permanently, but restore the old values after testing.

Actually test_code_module already contains necessary code for restoring the sys module. It only needs to initialize ps1 and ps2 properly when they already were set.

@terryjreedy
Copy link
Member Author

After the update merge, which included the change to IDLE not setting sys.ps1, test_code_module continues to pass the real test, which is running with import test.autotest.

@terryjreedy terryjreedy merged commit 5a4bbcd into python:master Oct 28, 2017
@miss-islington
Copy link
Contributor

Thanks @terryjreedy for the PR 🌮🎉.. I'm working now to backport this PR to: 3.6.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 28, 2017
@bedevere-bot
Copy link

GH-4156 is a backport of this pull request to the 3.6 branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants