Fix check for running inside venv (#8481)

* Import and use the function from pip instead of defining it
  ourselves.
* Fix tests.
This commit is contained in:
Martin Hjelmare 2017-07-15 16:25:02 +02:00 committed by Paulus Schoutsen
parent 6ca828fd14
commit 543e8bb62e
5 changed files with 36 additions and 24 deletions

View File

@ -1,15 +1,17 @@
"""Home Assistant command line scripts.""" """Home Assistant command line scripts."""
import argparse import argparse
import importlib import importlib
import logging
import os import os
import sys import sys
import logging
from typing import List from typing import List
from homeassistant.bootstrap import mount_local_lib_path
from homeassistant.config import get_default_config_dir from homeassistant.config import get_default_config_dir
from homeassistant.const import CONSTRAINT_FILE from homeassistant.const import CONSTRAINT_FILE
from homeassistant.util.package import install_package, is_virtual_env from homeassistant.util.package import (
from homeassistant.bootstrap import mount_local_lib_path install_package, running_under_virtualenv)
def run(args: List) -> int: def run(args: List) -> int:
@ -41,7 +43,7 @@ def run(args: List) -> int:
logging.basicConfig(stream=sys.stdout, level=logging.INFO) logging.basicConfig(stream=sys.stdout, level=logging.INFO)
for req in getattr(script, 'REQUIREMENTS', []): for req in getattr(script, 'REQUIREMENTS', []):
if is_virtual_env(): if running_under_virtualenv():
returncode = install_package(req, constraints=os.path.join( returncode = install_package(req, constraints=os.path.join(
os.path.dirname(__file__), os.pardir, CONSTRAINT_FILE)) os.path.dirname(__file__), os.pardir, CONSTRAINT_FILE))
else: else:

View File

@ -77,7 +77,7 @@ def _async_process_requirements(hass: core.HomeAssistant, name: str,
def pip_install(mod): def pip_install(mod):
"""Install packages.""" """Install packages."""
if pkg_util.is_virtual_env(): if pkg_util.running_under_virtualenv():
return pkg_util.install_package( return pkg_util.install_package(
mod, constraints=os.path.join( mod, constraints=os.path.join(
os.path.dirname(__file__), CONSTRAINT_FILE)) os.path.dirname(__file__), CONSTRAINT_FILE))

View File

@ -2,11 +2,12 @@
import asyncio import asyncio
import logging import logging
import os import os
from subprocess import PIPE, Popen
import sys import sys
import threading import threading
from subprocess import Popen, PIPE
from urllib.parse import urlparse from urllib.parse import urlparse
from pip.locations import running_under_virtualenv
from typing import Optional from typing import Optional
import pkg_resources import pkg_resources
@ -36,7 +37,7 @@ def install_package(package: str, upgrade: bool=True,
if constraints is not None: if constraints is not None:
args += ['--constraint', constraints] args += ['--constraint', constraints]
if target: if target:
assert not is_virtual_env() assert not running_under_virtualenv()
# This only works if not running in venv # This only works if not running in venv
args += ['--user'] args += ['--user']
env['PYTHONUSERBASE'] = os.path.abspath(target) env['PYTHONUSERBASE'] = os.path.abspath(target)
@ -70,11 +71,6 @@ def check_package_exists(package: str) -> bool:
return any(dist in req for dist in env[req.project_name]) return any(dist in req for dist in env[req.project_name])
def is_virtual_env() -> bool:
"""Return true if environment is a virtual environment."""
return hasattr(sys, 'real_prefix')
def _get_user_site(deps_dir: str) -> tuple: def _get_user_site(deps_dir: str) -> tuple:
"""Get arguments and environment for subprocess used in get_user_site.""" """Get arguments and environment for subprocess used in get_user_site."""
env = os.environ.copy() env = os.environ.copy()

View File

@ -204,13 +204,14 @@ class TestSetup:
assert 'comp' not in self.hass.config.components assert 'comp' not in self.hass.config.components
@mock.patch('homeassistant.setup.os.path.dirname') @mock.patch('homeassistant.setup.os.path.dirname')
@mock.patch('homeassistant.util.package.sys') @mock.patch('homeassistant.util.package.running_under_virtualenv',
return_value=True)
@mock.patch('homeassistant.util.package.install_package', @mock.patch('homeassistant.util.package.install_package',
return_value=True) return_value=True)
def test_requirement_installed_in_venv( def test_requirement_installed_in_venv(
self, mock_install, mock_sys, mock_dirname): self, mock_install, mock_venv, mock_dirname):
"""Test requirement installed in virtual environment.""" """Test requirement installed in virtual environment."""
mock_sys.real_prefix = 'pythonpath' mock_venv.return_value = True
mock_dirname.return_value = 'ha_package_path' mock_dirname.return_value = 'ha_package_path'
self.hass.config.skip_pip = False self.hass.config.skip_pip = False
loader.set_component( loader.set_component(
@ -222,11 +223,12 @@ class TestSetup:
constraints=os.path.join('ha_package_path', CONSTRAINT_FILE)) constraints=os.path.join('ha_package_path', CONSTRAINT_FILE))
@mock.patch('homeassistant.setup.os.path.dirname') @mock.patch('homeassistant.setup.os.path.dirname')
@mock.patch('homeassistant.util.package.sys', spec=object()) @mock.patch('homeassistant.util.package.running_under_virtualenv',
return_value=False)
@mock.patch('homeassistant.util.package.install_package', @mock.patch('homeassistant.util.package.install_package',
return_value=True) return_value=True)
def test_requirement_installed_in_deps( def test_requirement_installed_in_deps(
self, mock_install, mock_sys, mock_dirname): self, mock_install, mock_venv, mock_dirname):
"""Test requirement installed in deps directory.""" """Test requirement installed in deps directory."""
mock_dirname.return_value = 'ha_package_path' mock_dirname.return_value = 'ha_package_path'
self.hass.config.skip_pip = False self.hass.config.skip_pip = False

View File

@ -66,6 +66,14 @@ def mock_env_copy():
yield env_copy yield env_copy
@pytest.fixture
def mock_venv():
"""Mock homeassistant.util.package.running_under_virtualenv."""
with patch('homeassistant.util.package.running_under_virtualenv') as mock:
mock.return_value = True
yield mock
@asyncio.coroutine @asyncio.coroutine
def mock_async_subprocess(): def mock_async_subprocess():
"""Return an async Popen mock.""" """Return an async Popen mock."""
@ -90,7 +98,7 @@ def test_install_existing_package(mock_exists, mock_popen):
assert mock_popen.return_value.communicate.call_count == 0 assert mock_popen.return_value.communicate.call_count == 0
def test_install(mock_sys, mock_exists, mock_popen, mock_env_copy): def test_install(mock_sys, mock_exists, mock_popen, mock_env_copy, mock_venv):
"""Test an install attempt on a package that doesn't exist.""" """Test an install attempt on a package that doesn't exist."""
env = mock_env_copy() env = mock_env_copy()
assert package.install_package(TEST_NEW_REQ, False) assert package.install_package(TEST_NEW_REQ, False)
@ -106,7 +114,8 @@ def test_install(mock_sys, mock_exists, mock_popen, mock_env_copy):
assert mock_popen.return_value.communicate.call_count == 1 assert mock_popen.return_value.communicate.call_count == 1
def test_install_upgrade(mock_sys, mock_exists, mock_popen, mock_env_copy): def test_install_upgrade(
mock_sys, mock_exists, mock_popen, mock_env_copy, mock_venv):
"""Test an upgrade attempt on a package.""" """Test an upgrade attempt on a package."""
env = mock_env_copy() env = mock_env_copy()
assert package.install_package(TEST_NEW_REQ) assert package.install_package(TEST_NEW_REQ)
@ -122,11 +131,13 @@ def test_install_upgrade(mock_sys, mock_exists, mock_popen, mock_env_copy):
assert mock_popen.return_value.communicate.call_count == 1 assert mock_popen.return_value.communicate.call_count == 1
def test_install_target(mock_sys, mock_exists, mock_popen, mock_env_copy): def test_install_target(
mock_sys, mock_exists, mock_popen, mock_env_copy, mock_venv):
"""Test an install with a target.""" """Test an install with a target."""
target = 'target_folder' target = 'target_folder'
env = mock_env_copy() env = mock_env_copy()
env['PYTHONUSERBASE'] = os.path.abspath(target) env['PYTHONUSERBASE'] = os.path.abspath(target)
mock_venv.return_value = False
mock_sys.platform = 'linux' mock_sys.platform = 'linux'
args = [ args = [
mock_sys.executable, '-m', 'pip', 'install', '--quiet', mock_sys.executable, '-m', 'pip', 'install', '--quiet',
@ -142,15 +153,15 @@ def test_install_target(mock_sys, mock_exists, mock_popen, mock_env_copy):
assert mock_popen.return_value.communicate.call_count == 1 assert mock_popen.return_value.communicate.call_count == 1
def test_install_target_venv(mock_sys, mock_exists, mock_popen, mock_env_copy): def test_install_target_venv(
mock_sys, mock_exists, mock_popen, mock_env_copy, mock_venv):
"""Test an install with a target in a virtual environment.""" """Test an install with a target in a virtual environment."""
target = 'target_folder' target = 'target_folder'
mock_sys.real_prefix = '/usr'
with pytest.raises(AssertionError): with pytest.raises(AssertionError):
package.install_package(TEST_NEW_REQ, False, target=target) package.install_package(TEST_NEW_REQ, False, target=target)
def test_install_error(caplog, mock_sys, mock_exists, mock_popen): def test_install_error(caplog, mock_sys, mock_exists, mock_popen, mock_venv):
"""Test an install with a target.""" """Test an install with a target."""
caplog.set_level(logging.WARNING) caplog.set_level(logging.WARNING)
mock_popen.return_value.returncode = 1 mock_popen.return_value.returncode = 1
@ -160,7 +171,8 @@ def test_install_error(caplog, mock_sys, mock_exists, mock_popen):
assert record.levelname == 'ERROR' assert record.levelname == 'ERROR'
def test_install_constraint(mock_sys, mock_exists, mock_popen, mock_env_copy): def test_install_constraint(
mock_sys, mock_exists, mock_popen, mock_env_copy, mock_venv):
"""Test install with constraint file on not installed package.""" """Test install with constraint file on not installed package."""
env = mock_env_copy() env = mock_env_copy()
constraints = 'constraints_file.txt' constraints = 'constraints_file.txt'