Skip to content

fix(tests): make doctor tests deterministic across CI environments #997

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

Merged
sergio-sisternes-epam merged 1 commit into from
Apr 27, 2026
Merged

fix(tests): make doctor tests deterministic across CI environments #997

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
62 changes: 31 additions & 31 deletions tests/unit/commands/test_marketplace_doctor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import subprocess
import textwrap
from pathlib import Path
from types import SimpleNamespace
from unittest.mock import MagicMock, patch

import pytest
Expand Down Expand Up @@ -36,6 +37,30 @@
""")


# Token env vars that AuthResolver inspects. Cleared in the autouse
# fixture below so doctor tests are deterministic regardless of CI env.
_TOKEN_ENV_VARS = ("GITHUB_APM_PAT", "GITHUB_TOKEN", "GH_TOKEN")
Comment on lines +40 to +42
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

_TOKEN_ENV_VARS/comment implies these are the token env vars AuthResolver inspects, but AuthResolver also checks per-org vars like GITHUB_APM_PAT_<ORG> before the global ones. If the intent is "clear all auth env vars" for determinism, consider also clearing any env vars with the GITHUB_APM_PAT_ prefix (or reword the comment/constant name to reflect that this only targets the global vars).

Copilot uses AI. Check for mistakes.


@pytest.fixture(autouse=True)
def _mock_auth_resolver(monkeypatch):
"""Make the auth check deterministic by mocking AuthResolver.

Without this, the number of ``subprocess.run`` calls inside
``doctor()`` varies depending on whether an env-var token exists
(AuthResolver skips ``git credential fill`` when one is found),
which causes positional mock side-effects to shift on CI where
``GITHUB_APM_PAT`` is set.
"""
for var in _TOKEN_ENV_VARS:
monkeypatch.delenv(var, raising=False)

auth_ctx = SimpleNamespace(token="mock-doctor-token")
mock_cls = MagicMock()
mock_cls.return_value.resolve.return_value = auth_ctx
monkeypatch.setattr("apm_cli.core.auth.AuthResolver", mock_cls)


@pytest.fixture
def runner():
return CliRunner()
Expand All @@ -49,7 +74,6 @@ def _make_run_result(returncode=0, stdout="", stderr=""):


_GH_OK = _make_run_result(0, stdout="gh version 2.50.0 (2024-06-01)\nhttps://github.com/cli/cli/releases/tag/v2.50.0")
_GIT_CRED_OK = _make_run_result(0, stdout="")


# ---------------------------------------------------------------------------
Expand All @@ -67,7 +91,6 @@ def test_all_pass_exit_0(self, mock_run, runner, tmp_path, monkeypatch):
mock_run.side_effect = [
_make_run_result(0, stdout="git version 2.40.0"),
_make_run_result(0, stdout="abc123\tHEAD"),
_GIT_CRED_OK,
_GH_OK,
]

Expand All @@ -80,7 +103,6 @@ def test_git_version_shown(self, mock_run, runner, tmp_path, monkeypatch):
mock_run.side_effect = [
_make_run_result(0, stdout="git version 2.40.0"),
_make_run_result(0, stdout="abc123\tHEAD"),
_GIT_CRED_OK,
_GH_OK,
]

Expand All @@ -93,7 +115,6 @@ def test_network_reachable_shown(self, mock_run, runner, tmp_path, monkeypatch):
mock_run.side_effect = [
_make_run_result(0, stdout="git version 2.40.0"),
_make_run_result(0),
_GIT_CRED_OK,
_GH_OK,
]

Expand Down Expand Up @@ -131,7 +152,6 @@ def test_git_nonzero_exit(self, mock_run, runner, tmp_path, monkeypatch):
mock_run.side_effect = [
_make_run_result(returncode=1, stderr="error"),
_make_run_result(0), # network check may still run
_GIT_CRED_OK,
_GH_OK,
]

Expand All @@ -151,7 +171,6 @@ def test_network_failure_exits_1(self, mock_run, runner, tmp_path, monkeypatch):
mock_run.side_effect = [
_make_run_result(0, stdout="git version 2.40.0"),
_make_run_result(128, stderr="fatal: could not resolve host"),
_GIT_CRED_OK,
_GH_OK,
]

Expand All @@ -164,7 +183,6 @@ def test_network_timeout(self, mock_run, runner, tmp_path, monkeypatch):
mock_run.side_effect = [
_make_run_result(0, stdout="git version 2.40.0"),
subprocess.TimeoutExpired(cmd="git", timeout=5),
_GIT_CRED_OK,
_GH_OK,
]

Expand All @@ -178,7 +196,6 @@ def test_network_auth_error(self, mock_run, runner, tmp_path, monkeypatch):
mock_run.side_effect = [
_make_run_result(0, stdout="git version 2.40.0"),
_make_run_result(128, stderr="fatal: authentication failed"),
_GIT_CRED_OK,
_GH_OK,
]

Expand All @@ -199,7 +216,6 @@ def test_github_token_detected(self, mock_run, runner, tmp_path, monkeypatch):
mock_run.side_effect = [
_make_run_result(0, stdout="git version 2.40.0"),
_make_run_result(0),
_GIT_CRED_OK,
_GH_OK,
]

Expand All @@ -216,7 +232,6 @@ def test_gh_token_detected(self, mock_run, runner, tmp_path, monkeypatch):
mock_run.side_effect = [
_make_run_result(0, stdout="git version 2.40.0"),
_make_run_result(0),
_GIT_CRED_OK,
_GH_OK,
]

Expand All @@ -227,12 +242,15 @@ def test_gh_token_detected(self, mock_run, runner, tmp_path, monkeypatch):
@patch("apm_cli.commands.marketplace.subprocess.run")
def test_no_token_informational(self, mock_run, runner, tmp_path, monkeypatch):
monkeypatch.chdir(tmp_path)
monkeypatch.delenv("GITHUB_TOKEN", raising=False)
monkeypatch.delenv("GH_TOKEN", raising=False)
# Override the autouse mock so AuthResolver reports no token.
no_token_ctx = SimpleNamespace(token=None)
mock_cls = MagicMock()
mock_cls.return_value.resolve.return_value = no_token_ctx
monkeypatch.setattr("apm_cli.core.auth.AuthResolver", mock_cls)

mock_run.side_effect = [
_make_run_result(0, stdout="git version 2.40.0"),
_make_run_result(0),
_GIT_CRED_OK,
_GH_OK,
]

Expand All @@ -253,7 +271,6 @@ def test_gh_found_shows_version(self, mock_run, runner, tmp_path, monkeypatch):
mock_run.side_effect = [
_make_run_result(0, stdout="git version 2.40.0"),
_make_run_result(0),
_GIT_CRED_OK,
_make_run_result(0, stdout="gh version 2.50.0 (2024-06-01)\nhttps://github.com/cli/cli/releases/tag/v2.50.0"),
]

Expand All @@ -267,7 +284,6 @@ def test_gh_missing_is_warning_not_error(self, mock_run, runner, tmp_path, monke
mock_run.side_effect = [
_make_run_result(0, stdout="git version 2.40.0"),
_make_run_result(0),
_GIT_CRED_OK,
FileNotFoundError("gh not found"),
]

Expand All @@ -282,7 +298,6 @@ def test_gh_nonzero_exit(self, mock_run, runner, tmp_path, monkeypatch):
mock_run.side_effect = [
_make_run_result(0, stdout="git version 2.40.0"),
_make_run_result(0),
_GIT_CRED_OK,
_make_run_result(returncode=1, stderr="error"),
]

Expand All @@ -296,7 +311,6 @@ def test_gh_timeout(self, mock_run, runner, tmp_path, monkeypatch):
mock_run.side_effect = [
_make_run_result(0, stdout="git version 2.40.0"),
_make_run_result(0),
_GIT_CRED_OK,
subprocess.TimeoutExpired(cmd="gh", timeout=10),
]

Expand All @@ -310,7 +324,6 @@ def test_gh_general_exception(self, mock_run, runner, tmp_path, monkeypatch):
mock_run.side_effect = [
_make_run_result(0, stdout="git version 2.40.0"),
_make_run_result(0),
_GIT_CRED_OK,
OSError("Permission denied"),
]

Expand All @@ -324,7 +337,6 @@ def test_gh_shown_in_table(self, mock_run, runner, tmp_path, monkeypatch):
mock_run.side_effect = [
_make_run_result(0, stdout="git version 2.40.0"),
_make_run_result(0),
_GIT_CRED_OK,
_GH_OK,
]

Expand All @@ -345,7 +357,6 @@ def test_yml_present_and_valid(self, mock_run, runner, tmp_path, monkeypatch):
mock_run.side_effect = [
_make_run_result(0, stdout="git version 2.40.0"),
_make_run_result(0),
_GIT_CRED_OK,
_GH_OK,
]

Expand All @@ -360,7 +371,6 @@ def test_yml_present_but_invalid(self, mock_run, runner, tmp_path, monkeypatch):
mock_run.side_effect = [
_make_run_result(0, stdout="git version 2.40.0"),
_make_run_result(0),
_GIT_CRED_OK,
_GH_OK,
]

Expand All @@ -375,7 +385,6 @@ def test_yml_absent(self, mock_run, runner, tmp_path, monkeypatch):
mock_run.side_effect = [
_make_run_result(0, stdout="git version 2.40.0"),
_make_run_result(0),
_GIT_CRED_OK,
_GH_OK,
]

Expand All @@ -398,7 +407,6 @@ def test_yml_invalid_does_not_cause_exit_1(self, mock_run, runner, tmp_path, mon
mock_run.side_effect = [
_make_run_result(0, stdout="git version 2.40.0"),
_make_run_result(0),
_GIT_CRED_OK,
_GH_OK,
]

Expand Down Expand Up @@ -427,7 +435,6 @@ def test_verbose_no_crash(self, mock_run, runner, tmp_path, monkeypatch):
mock_run.side_effect = [
_make_run_result(0, stdout="git version 2.40.0"),
_make_run_result(0),
_GIT_CRED_OK,
_GH_OK,
]

Expand All @@ -447,7 +454,6 @@ def test_table_has_check_column(self, mock_run, runner, tmp_path, monkeypatch):
mock_run.side_effect = [
_make_run_result(0, stdout="git version 2.40.0"),
_make_run_result(0),
_GIT_CRED_OK,
_GH_OK,
]

Expand All @@ -465,7 +471,6 @@ def test_info_icon_for_auth(self, mock_run, runner, tmp_path, monkeypatch):
mock_run.side_effect = [
_make_run_result(0, stdout="git version 2.40.0"),
_make_run_result(0),
_GIT_CRED_OK,
_GH_OK,
]

Expand All @@ -478,7 +483,6 @@ def test_pass_icon_for_git(self, mock_run, runner, tmp_path, monkeypatch):
mock_run.side_effect = [
_make_run_result(0, stdout="git version 2.40.0"),
_make_run_result(0),
_GIT_CRED_OK,
_GH_OK,
]

Expand Down Expand Up @@ -516,7 +520,6 @@ def test_git_ok_network_file_not_found(self, mock_run, runner, tmp_path, monkeyp
mock_run.side_effect = [
_make_run_result(0, stdout="git version 2.40.0"),
FileNotFoundError("git not found"),
_GIT_CRED_OK,
_GH_OK,
]

Expand All @@ -542,7 +545,6 @@ def test_duplicate_names_flagged(
mock_run.side_effect = [
_make_run_result(0, stdout="git version 2.40.0"),
_make_run_result(0),
_GIT_CRED_OK,
_GH_OK,
]
mock_load.return_value = MarketplaceYml(
Expand Down Expand Up @@ -576,7 +578,6 @@ def test_no_duplicate_names_shows_pass(
mock_run.side_effect = [
_make_run_result(0, stdout="git version 2.40.0"),
_make_run_result(0),
_GIT_CRED_OK,
_GH_OK,
]
mock_load.return_value = MarketplaceYml(
Expand Down Expand Up @@ -607,7 +608,6 @@ def test_no_duplicate_check_when_yml_absent(
mock_run.side_effect = [
_make_run_result(0, stdout="git version 2.40.0"),
_make_run_result(0),
_GIT_CRED_OK,
_GH_OK,
]

Expand Down
Loading