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.

Sign up for GitHub

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

PR: Skip conda and pyenv tests if these tools are not present #17095

Merged
merged 3 commits into from
Dec 27, 2021
Merged

PR: Skip conda and pyenv tests if these tools are not present #17095

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ed3564f
Skip conda and pyenv tests if these tools are not present
juliangilbey Dec 26, 2021
df6008e
Make PEP8 compliant
juliangilbey Dec 26, 2021
8cdf8f8
Update patches following @ccordoba's advice
juliangilbey Dec 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions spyder/plugins/maininterpreter/widgets/tests/test_status.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,13 @@
from spyder.config.base import running_in_ci
from spyder.plugins.statusbar.widgets.tests.test_status import status_bar
from spyder.plugins.maininterpreter.widgets.status import InterpreterStatus
from spyder.utils.conda import get_list_conda_envs


CONDA_MISSING = (len(get_list_conda_envs()) == 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a function called is_anaconda in spyder.config.utils. So please use that one instead.

Also, there's no need to declare an extra constant here for that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I didn't know about that function, thanks!
There's a newer comment on the stackoverflow thread, by the way, which suggests that is_anaconda should use sys.base_prefix instead of sys.prefix for greater robustness.



@pytest.mark.skipif(CONDA_MISSING, reason="Requires conda to be installed")
@pytest.mark.skipif(not running_in_ci(), reason="Only meant for CIs")
def test_status_bar_conda_interpreter_status(status_bar, qtbot):
"""Test status bar message with conda interpreter."""
Expand Down
8 changes: 8 additions & 0 deletions spyder/utils/tests/test_conda.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
get_conda_root_prefix, get_list_conda_envs, get_list_conda_envs_cache)


CONDA_MISSING = (len(get_list_conda_envs()) == 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change this line to

if not is_anaconda():
    pytest.skip()

and remove the skipif you added in each test below. That will allow us to skip all tests in this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I didn't know the pytest.skip() function; that's nice!


if os.name == 'nt':
TEST_PYEXEC = 'c:/miniconda/envs/foobar/python.exe'
else:
Expand All @@ -35,6 +37,7 @@ def test_add_quotes():
assert output == '/some-path/with-no-spaces'


@pytest.mark.skipif(CONDA_MISSING, reason="Requires conda to be installed")
def test_get_conda_activation_script():
output = get_conda_activation_script(TEST_PYEXEC)
if os.name == 'nt':
Expand All @@ -43,6 +46,7 @@ def test_get_conda_activation_script():
assert output == '/miniconda/bin/activate'


@pytest.mark.skipif(CONDA_MISSING, reason="Requires conda to be installed")
def test_get_conda_env_path():
output = get_conda_env_path(TEST_PYEXEC)
if os.name == 'nt':
Expand All @@ -51,6 +55,7 @@ def test_get_conda_env_path():
assert output == '/miniconda/envs/foobar'


@pytest.mark.skipif(CONDA_MISSING, reason="Requires conda to be installed")
def test_get_conda_root_prefix():
output = get_conda_root_prefix(TEST_PYEXEC)
if os.name == 'nt':
Expand All @@ -61,11 +66,13 @@ def test_get_conda_root_prefix():
assert 'envs' not in get_conda_root_prefix(sys.executable)


@pytest.mark.skipif(CONDA_MISSING, reason="Requires conda to be installed")
@pytest.mark.skipif(not running_in_ci(), reason="Only meant for CIs")
def test_find_conda():
assert find_conda()


@pytest.mark.skipif(CONDA_MISSING, reason="Requires conda to be installed")
@pytest.mark.skipif(not running_in_ci(), reason="Only meant for CIs")
def test_get_list_conda_envs():
output = get_list_conda_envs()
Expand All @@ -75,6 +82,7 @@ def test_get_list_conda_envs():
assert set(expected_envs) == set(output.keys())


@pytest.mark.skipif(CONDA_MISSING, reason="Requires conda to be installed")
@pytest.mark.skipif(not running_in_ci(), reason="Only meant for CIs")
def test_get_list_conda_envs_cache():
time0 = time.time()
Expand Down
5 changes: 5 additions & 0 deletions spyder/utils/tests/test_pyenv.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@
from spyder.utils.pyenv import get_list_pyenv_envs, get_list_pyenv_envs_cache


PYENV_MISSING = (len(get_list_pyenv_envs()) == 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change this line to

if not find_program('pyenv'):
    pytest.skip()

and remove the skipif you added in each test below.

find_program is available in spyder.utils.programs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All three requests fulfilled!



@pytest.mark.skipif(PYENV_MISSING, reason="Requires pyenv to be installed")
@pytest.mark.skipif(not running_in_ci(), reason="Only meant for CIs")
@pytest.mark.skipif(not sys.platform.startswith('linux'),
reason="Only runs on Linux")
Expand All @@ -24,6 +28,7 @@ def test_get_list_pyenv_envs():
assert set(expected_envs) == set(output.keys())


@pytest.mark.skipif(PYENV_MISSING, reason="Requires pyenv to be installed")
@pytest.mark.skipif(not running_in_ci(), reason="Only meant for CIs")
@pytest.mark.skipif(not sys.platform.startswith('linux'),
reason="Only runs on Linux")
Expand Down