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

Add tests for require-virtualenv #12843

Open
ichard26 opened this issue Jul 11, 2024 · 1 comment
Open

Add tests for require-virtualenv #12843

ichard26 opened this issue Jul 11, 2024 · 1 comment
Labels
C: tests Testing and related things help wanted For requesting inputs from other members of the community project: virtualenv Related to virtualenv

Comments

@ichard26
Copy link
Member

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() in cli/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)

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)

@ichard26 ichard26 added C: tests Testing and related things project: virtualenv Related to virtualenv help wanted For requesting inputs from other members of the community labels Jul 11, 2024
@matthewhughes934
Copy link
Contributor

For the erroring case, could we avoid the monkey patching and just define a custom command and 'pretend' we're not in a virtualenv, something like (the attribute setting/restoring magic should probably be in a fixture/context manager):

    def test_require_virtualenv_errors_when_not_in_venv(self) -> None:
        class Cmd(Command):
            ignore_require_venv: bool = False

        # edit variable set by PEP-405 compliant virtualenvs
        # to pretend we're not in a venv even if we actually are
        has_base_prefix = hasattr(sys, "base_prefix")
        old_base_prefix = getattr(sys, "base_prefix", None)
        if has_base_prefix:
            delattr(sys, "base_prefix")

        try:
            with pytest.raises(SystemExit) as excinfo:
                Cmd(name="fake", summary="").main(["--require-virtualenv"])

            assert excinfo.value.code == VIRTUALENV_NOT_FOUND
        finally:
            # restore our modifications
            if has_base_prefix:
                sys.base_prefix = old_base_prefix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: tests Testing and related things help wanted For requesting inputs from other members of the community project: virtualenv Related to virtualenv
Projects
None yet
Development

No branches or pull requests

2 participants