Description
As far as I know, the require-virtualenv functionality is essentially entirely untested. This is the main reason why it is not documented (see also #2429).
It would be good to see tests for --require-virtualenv
- Functional tests that verify it actually works (who knows, perhaps our virtual environment detection logic will break in some weird edge case down the road)
- Unit/functional tests that force us to explicitly decide whether a new command should opt-in or opt-out of the
--require-virtualenv
feature
Below, I've attached some old discussion on this topic.
It would be nice to have a test for this - but I note that #8603 didn't include a test, so I'm inclined to accept this without a test and add tests for both cases in a follow-up PR.
I started taking a look at the tests- as far as I can tell require_venv
isn't actually tested anywhere and it's not clear to me that there would be any benefit to testing the behavior of ignore_require_venv
for this particular command.
This is the only reference I can find (tests/unit/test_options.py
) and it only seems to be asserting that the order (command, option) doesn't matter.
480 def test_require_virtualenv(self) -> None:
481 # FakeCommand intentionally returns the wrong type.
482 options1, args1 = cast(
483 Tuple[Values, List[str]], main(["--require-virtualenv", "fake"])
484 )
485 options2, args2 = cast(
486 Tuple[Values, List[str]], main(["fake", "--require-virtualenv"])
487 )
488 assert options1.require_venv
489 assert options2.require_venv
my first thought is to implement something like:
- mock virtualenv by monkey patching
running_under_virtualenv()
incli/base_command.py
(with return value of has_venv column in table below) - mock ignore_require_venv by monkeypatching FakeCommand
- test condition:
- if require_venv and not ignore_require_venv and not has_venv:
assert_option_error(msg)
- if require_venv and not ignore_require_venv and not has_venv:
however... that test condition looks an awful lot like a reimplementation of the original implementation...
# cli/base_command.py
if options.require_venv and not self.ignore_require_venv:
# If a venv is required check if it can really be found
if not running_under_virtualenv():
logger.critical("Could not find an activated virtualenv (required).")
sys.exit(VIRTUALENV_NOT_FOUND)
error condition truth matrix:
"has_venv" | "require_venv" | "ignore_require_venv" | "should_error" |
---|---|---|---|
T | T | T | F |
T | T | F | F |
T | F | T | F |
T | F | F | F |
F | T | T | F |
F | T | F | T |
F | F | T | F |
so I guess I'm a little lost on what a test for this should look like...
Originally posted by @justin-f-perez in #10658 (comment)