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

Fix a freeze debug log message. #6383

Merged
merged 4 commits into from
May 7, 2019

Conversation

cjerdonek
Copy link
Member

@cjerdonek cjerdonek commented Apr 4, 2019

The main purpose of this PR is to fix a non-fatal logging error in a debug log message in freeze.py (first commit out of the four in this PR).

It also does the following related things:

  1. (second commit) Update PipTestEnvironment to also check for logging errors when allow_stderr_error is false, and also add a test to check that this works. This will help to catch and prevent similar errors in the future.
  2. (third commit) Remove a bunch of unneeded expect_stderr=True arguments in VCS tests. (If (1) and (2) had been done before, the logging error would have been detected by the test suite.)
  3. (fourth and final commit of the PR) Also remove some unneeded expect_error=True in some VCS tests. This unveiled a broken test (test_git_with_non_editable_unpacking), which is also fixed in the fourth commit.

Possible future change / improvement: also check for logging errors when our tests are running even if allow_stderr_error is true.

@cjerdonek cjerdonek added C: freeze 'pip freeze' related C: tests Testing and related things type: bugfix labels Apr 4, 2019
@cjerdonek cjerdonek marked this pull request as ready for review April 7, 2019 06:50
@cjerdonek cjerdonek force-pushed the fix-freeze-debug-message branch 3 times, most recently from c4fd256 to 1d50c5a Compare April 7, 2019 07:58
@cjerdonek
Copy link
Member Author

Hi @xavfernandez, it would be great if you could take a look at this. With the exception of a single line (the bug fix), this is a tests-only PR.

@cjerdonek
Copy link
Member Author

Merging now as this is a trivial bug fix with test improvements.

@cjerdonek cjerdonek merged commit 5a00ac4 into pypa:master May 7, 2019
@cjerdonek cjerdonek deleted the fix-freeze-debug-message branch May 7, 2019 02:56
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 6, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation C: freeze 'pip freeze' related C: tests Testing and related things
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant