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
Merged

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 force-pushed the fix-freeze-debug-message branch from ed6d459 to 07af3fb Apr 4, 2019
@cjerdonek cjerdonek marked this pull request as ready for review Apr 7, 2019
@cjerdonek cjerdonek force-pushed the fix-freeze-debug-message branch 3 times, most recently from c4fd256 to 1d50c5a Apr 7, 2019
@cjerdonek
Copy link
Member Author

@cjerdonek cjerdonek commented Apr 13, 2019

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 cjerdonek force-pushed the fix-freeze-debug-message branch from 1d50c5a to 7e10908 May 6, 2019
@cjerdonek
Copy link
Member Author

@cjerdonek cjerdonek commented May 7, 2019

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

@cjerdonek cjerdonek merged commit 5a00ac4 into pypa:master May 7, 2019
29 checks passed
@cjerdonek cjerdonek deleted the fix-freeze-debug-message branch May 7, 2019
@lock lock bot added the S: auto-locked 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.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

1 participant