Skip to content

Conversation

@vstinner
Copy link
Member

@vstinner vstinner commented Jun 26, 2019

Fix test_wsgiref.testEnviron() to no longer depend on the environment
variables (don't fail if "X" variable is set).

testEnviron() now overrides os.environ to get a deterministic
environment. Test full TestHandler.environ content: not only a few
selected variables.

https://bugs.python.org/issue37411

Fix test_wsgiref.testEnviron() to no longer depend on the environment
variables (don't fail if "X" variable is set).

testEnviron() now overrides os.environ to get a deterministic
environment. Test full TestHandler.environ content: not only a few
selected variables.
@vstinner
Copy link
Member Author

@tirkarthi: Would you be interested to review my PR?

# set by BaseHandler.setup_environ()
'wsgi.input': handler.get_stdin(),
'wsgi.errors': handler.get_stderr(),
'wsgi.version': (1, 0),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clarification : Is it okay to let these values to be taken from handler's attributes so that the tests don't have to worry about these default values being changed in future? Since we just need to check handler.wsgi_version is set correctly with

env['wsgi.version'] = self.wsgi_version
in setup_environ. With this change the below assertion checks inside for loops are also not needed. Anything I am missing with setting these values?

Something like below :

'wsgi.input': handler.get_stdin(),
'wsgi.errors': handler.get_stderr(),
'wsgi.version': handler.wsgi_version,
'wsgi.run_once': handler.wsgi_run_once,
'wsgi.url_scheme': handler.get_scheme(),
'wsgi.multithread': handler.wsgi_multithread,
'wsgi.multiprocess': handler.wsgi_multiprocess,
'wsgi.file_wrapper': util.FileWrapper,

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a deliberate choice of copy directly the expected value rather than "reimplement" the logic creating the environ dictionary. I don't expect that default values will change often. If they change, well, the CI will tell you that and it will be trivial to update the tests.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tirkarthi: the test already checks that wsgi_xxx attributes are copied to wsgi.xxx env vars. I exchanged the two tests in testEnviron() so attributes are checked before hardcoded values. Are you fine with hardcoded values? Or do you prefer that I explicitly copy wsgi_xxx attributes?

Copy link
Member

@tirkarthi tirkarthi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks Victor for the details.

@vstinner
Copy link
Member Author

test_asyncio failed on AppVeyor. I manually scheduled a rebuild.

@vstinner vstinner merged commit 5150d32 into python:master Jun 26, 2019
@miss-islington
Copy link
Contributor

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 2.7, 3.7, 3.8.
🐍🍒⛏🤖

@vstinner vstinner deleted the test_wsgiref_environ branch June 26, 2019 16:16
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 26, 2019
Fix test_wsgiref.testEnviron() to no longer depend on the environment
variables (don't fail if "X" variable is set).

testEnviron() now overrides os.environ to get a deterministic
environment. Test full TestHandler.environ content: not only a few
selected variables.
(cherry picked from commit 5150d32)

Co-authored-by: Victor Stinner <[email protected]>
@bedevere-bot
Copy link

GH-14402 is a backport of this pull request to the 3.8 branch.

@bedevere-bot
Copy link

GH-14403 is a backport of this pull request to the 3.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 26, 2019
Fix test_wsgiref.testEnviron() to no longer depend on the environment
variables (don't fail if "X" variable is set).

testEnviron() now overrides os.environ to get a deterministic
environment. Test full TestHandler.environ content: not only a few
selected variables.
(cherry picked from commit 5150d32)

Co-authored-by: Victor Stinner <[email protected]>
@bedevere-bot
Copy link

GH-14404 is a backport of this pull request to the 2.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 26, 2019
Fix test_wsgiref.testEnviron() to no longer depend on the environment
variables (don't fail if "X" variable is set).

testEnviron() now overrides os.environ to get a deterministic
environment. Test full TestHandler.environ content: not only a few
selected variables.
(cherry picked from commit 5150d32)

Co-authored-by: Victor Stinner <[email protected]>
miss-islington added a commit that referenced this pull request Jun 26, 2019
Fix test_wsgiref.testEnviron() to no longer depend on the environment
variables (don't fail if "X" variable is set).

testEnviron() now overrides os.environ to get a deterministic
environment. Test full TestHandler.environ content: not only a few
selected variables.
(cherry picked from commit 5150d32)

Co-authored-by: Victor Stinner <[email protected]>
miss-islington added a commit that referenced this pull request Jun 26, 2019
Fix test_wsgiref.testEnviron() to no longer depend on the environment
variables (don't fail if "X" variable is set).

testEnviron() now overrides os.environ to get a deterministic
environment. Test full TestHandler.environ content: not only a few
selected variables.
(cherry picked from commit 5150d32)

Co-authored-by: Victor Stinner <[email protected]>
vstinner added a commit that referenced this pull request Jun 26, 2019
)

Fix test_wsgiref.testEnviron() to no longer depend on the environment
variables (don't fail if "X" variable is set).

testEnviron() now overrides os.environ to get a deterministic
environment. Test full TestHandler.environ content: not only a few
selected variables.
(cherry picked from commit 5150d32)

Co-authored-by: Victor Stinner <[email protected]>
lisroach pushed a commit to lisroach/cpython that referenced this pull request Sep 10, 2019
Fix test_wsgiref.testEnviron() to no longer depend on the environment
variables (don't fail if "X" variable is set).

testEnviron() now overrides os.environ to get a deterministic
environment. Test full TestHandler.environ content: not only a few
selected variables.
DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
Fix test_wsgiref.testEnviron() to no longer depend on the environment
variables (don't fail if "X" variable is set).

testEnviron() now overrides os.environ to get a deterministic
environment. Test full TestHandler.environ content: not only a few
selected variables.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tests Tests in the Lib/test dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants