-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
cache_dir: use $TOX_ENV_DIR/ prefix if set #4271
Conversation
The |
@asottile |
Is this good in general? |
its good in general - i'd like better layering for it, but its not clear how to get that yet |
testing/test_cacheprovider.py
Outdated
@pytest.fixture(scope="function", autouse=True) | ||
def handle_env(monkeypatch): | ||
"""Ensure env is like most of the tests expect it, i.e. not using tox.""" | ||
monkeypatch.delenv("TOX_ENV_DIR", raising=False) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This only gets used in this module/file then, right?
Might be worth requesting it explicitly though, but there are like 12+ files failing due to this.
Codecov Report
@@ Coverage Diff @@
## features #4271 +/- ##
============================================
- Coverage 95.84% 95.63% -0.22%
============================================
Files 111 111
Lines 24898 24909 +11
Branches 2432 2434 +2
============================================
- Hits 23863 23821 -42
- Misses 737 775 +38
- Partials 298 313 +15
Continue to review full report at Codecov.
|
just pondering some thoughts after revisiting im wondering if we ought to support uri style values there |
@RonnyPfannschmidt |
@blueyed true |
@pytest.fixture(scope="module", autouse=True) | ||
def handle_env(): | ||
"""Ensure env is like most of the tests expect it, i.e. not using tox.""" | ||
orig_env = os.environ.pop("TOX_ENV_DIR", None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could also easily be written as:
@pytest.fixture(scope="module", autouse=True)
def handle_env(monkeypatch):
monkeypatch.delenv('TOX_ENV_DIR', raising=False)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, ScopeMismatch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh true, but in that case I would use "function"
scope as the cost of patching for each test is negligible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, and used that initially IIRC, but then figured that it is at least something, if only for when stepping through hook callers via pdb.. ;)
Fixes #4270
"rst" pre-commit hook fails:
I think we should fix it to allow for our own roles etc. /cc @asottile