#7280: increase retry wait for rmtree #7361
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…to release files on windows.
chrahunt
reviewed
Nov 15, 2019
| @@ -366,11 +366,11 @@ def test_rmtree_retries(tmpdir, monkeypatch): | |||
| rmtree('foo') | |||
|
|
|||
|
|
|||
| def test_rmtree_retries_for_3sec(tmpdir, monkeypatch): | |||
| def test_rmtree_retries_for_12sec(tmpdir, monkeypatch): | |||
There was a problem hiding this comment.
Does this test actually last 12 seconds?
There was a problem hiding this comment.
yes, sadly.
>tox -e py37 -- tests\unit\test_utils.py
. . .
py37 run-test: commands[0] | pytest --timeout 300 'tests\unit\test_utils.py'
============================= test session starts =============================
platform win32 -- Python 3.7.4, pytest-3.8.2, py-1.8.0, pluggy-0.13.0
. . .
tests\unit\test_utils.py .............................s................. [ 32%]
........................................................................ [ 82%]
.......................... [100%]
=========================== short test summary info ===========================
SKIP [1] tests\unit\test_utils.py:400: condition: sys.platform == 'win32'
=================== 144 passed, 1 skipped in 15.06 seconds ====================
_______________________________________________________________ summary _______________________________________________________________
py37: commands succeeded
congratulations :)
There was a problem hiding this comment.
Ouch. Can we refactor like this:
def rmtree_provider(**kwargs):
@retry(**kwargs)
def rmtree(...):
# as before
...
return rmtree
and just test that it works as expected with 1 or 2 seconds, then set rmtree = rmtree_provider(stop_max_delay=12000, wait_fixed=500) in pip._internal.utils.misc? Then we're free to adjust the time without impacting test duration.
If you want to make that as a separate PR it would help separate this side concern from your overall proposal to move to 12s.
|
Any problem closing this in favor of #7363? |
|
if you prefer the other approach, i am good with closing this one.
…On Sat, Nov 16, 2019 at 11:21 Christopher Hunt ***@***.***> wrote:
Any problem closing this in favor of #7363
<#7363>?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#7361?email_source=notifications&email_token=AAC42ZLFRLO3MMZPI5ZOSUTQUAMZLA5CNFSM4JOCAUL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEHU7DQ#issuecomment-554651534>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAC42ZOCWJGZUZBRGNSKRELQUAMZLANCNFSM4JOCAULQ>
.
|
lock
bot
added
the
auto-locked
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
to address #7280 - increase retry wait for rmtree to allow more time for virus scanners to release files on windows.