From 14a6aaabff8512c95fb7ba4f54b4b51f1257c45a Mon Sep 17 00:00:00 2001 From: Chris Jerdonek Date: Sat, 16 Feb 2019 15:37:48 -0800 Subject: [PATCH 1/2] Include in pip's User-Agent whether it looks like pip is in CI. --- news/5499.feature | 2 ++ src/pip/_internal/download.py | 31 +++++++++++++++++++++++++++++++ tests/unit/test_download.py | 22 +++++++++++++++++++++- 3 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 news/5499.feature diff --git a/news/5499.feature b/news/5499.feature new file mode 100644 index 00000000000..7a786d853a9 --- /dev/null +++ b/news/5499.feature @@ -0,0 +1,2 @@ +Include in pip's User-Agent string whether it looks like pip is running +under CI. diff --git a/src/pip/_internal/download.py b/src/pip/_internal/download.py index 2bbe1762cda..bbcc643797a 100644 --- a/src/pip/_internal/download.py +++ b/src/pip/_internal/download.py @@ -72,6 +72,31 @@ logger = logging.getLogger(__name__) +# These are environment variables present when running under various +# CI systems. For each variable, some CI systems that use the variable +# are indicated. The collection was chosen so that for each of a number +# of popular systems, at least one of the environment variables is used. +# This list is used to provide some indication of and lower bound for +# CI traffic to PyPI. Thus, it is okay if the list is not comprehensive. +# For more background, see: https://github.com/pypa/pip/issues/5499 +CI_ENVIRONMENT_VARIABLES = [ + # Azure Pipelines + 'BUILD_BUILDID', + # Jenkins + 'BUILD_ID', + # AppVeyor, CircleCI, Codeship, Gitlab CI, Shippable, Travis CI + 'CI', +] + + +def looks_like_ci(): + # type: () -> bool + """ + Return whether it looks like pip is running under CI. + """ + return any(name in os.environ for name in CI_ENVIRONMENT_VARIABLES) + + def user_agent(): """ Return a string representing the user agent. @@ -135,6 +160,12 @@ def user_agent(): if setuptools_version is not None: data["setuptools_version"] = setuptools_version + # Use None rather than False so as not to give the impression that + # pip knows it is not being run under CI. Rather, it is a null or + # inconclusive result. Also, we include some value rather than no + # value to make it easier to know that the check has been run. + data["ci"] = True if looks_like_ci() else None + return "{data[installer][name]}/{data[installer][version]} {json}".format( data=data, json=json.dumps(data, separators=(",", ":"), sort_keys=True), diff --git a/tests/unit/test_download.py b/tests/unit/test_download.py index 532eb3fc3eb..a0aee9204b0 100644 --- a/tests/unit/test_download.py +++ b/tests/unit/test_download.py @@ -51,7 +51,27 @@ def _fake_session_get(*args, **kwargs): def test_user_agent(): - PipSession().headers["User-Agent"].startswith("pip/%s" % pip.__version__) + user_agent = PipSession().headers["User-Agent"] + + assert user_agent.startswith("pip/%s" % pip.__version__) + + +@pytest.mark.parametrize('name, expected_like_ci', [ + ('BUILD', False), + ('BUILD_BUILDID', True), + ('BUILD_ID', True), + ('CI', True), +]) +def test_user_agent__ci(monkeypatch, name, expected_like_ci): + # Clear existing names since we can be running under an actual CI. + for ci_name in ('BUILD_BUILDID', 'BUILD_ID', 'CI'): + monkeypatch.delenv(ci_name, raising=False) + + monkeypatch.setenv(name, 'true') + user_agent = PipSession().headers["User-Agent"] + + assert ('"ci":true' in user_agent) == expected_like_ci + assert ('"ci":null' in user_agent) == (not expected_like_ci) class FakeStream(object): From a229f114e6e43860c67c0ff369216b74055b04d2 Mon Sep 17 00:00:00 2001 From: Chris Jerdonek Date: Sun, 17 Feb 2019 23:03:51 -0800 Subject: [PATCH 2/2] Address review comments. --- src/pip/_internal/download.py | 7 +++++-- tests/unit/test_download.py | 29 +++++++++++++++++++++-------- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/pip/_internal/download.py b/src/pip/_internal/download.py index bbcc643797a..9e345419feb 100644 --- a/src/pip/_internal/download.py +++ b/src/pip/_internal/download.py @@ -79,14 +79,14 @@ # This list is used to provide some indication of and lower bound for # CI traffic to PyPI. Thus, it is okay if the list is not comprehensive. # For more background, see: https://github.com/pypa/pip/issues/5499 -CI_ENVIRONMENT_VARIABLES = [ +CI_ENVIRONMENT_VARIABLES = ( # Azure Pipelines 'BUILD_BUILDID', # Jenkins 'BUILD_ID', # AppVeyor, CircleCI, Codeship, Gitlab CI, Shippable, Travis CI 'CI', -] +) def looks_like_ci(): @@ -94,6 +94,9 @@ def looks_like_ci(): """ Return whether it looks like pip is running under CI. """ + # We don't use the method of checking for a tty (e.g. using isatty()) + # because some CI systems mimic a tty (e.g. Travis CI). Thus that + # method doesn't provide definitive information in either direction. return any(name in os.environ for name in CI_ENVIRONMENT_VARIABLES) diff --git a/tests/unit/test_download.py b/tests/unit/test_download.py index a0aee9204b0..a848054e15f 100644 --- a/tests/unit/test_download.py +++ b/tests/unit/test_download.py @@ -10,8 +10,8 @@ import pip from pip._internal.download import ( - PipSession, SafeFileCache, path_to_url, unpack_file_url, unpack_http_url, - url_to_path, + CI_ENVIRONMENT_VARIABLES, PipSession, SafeFileCache, path_to_url, + unpack_file_url, unpack_http_url, url_to_path, ) from pip._internal.exceptions import HashMismatch from pip._internal.models.link import Link @@ -50,26 +50,39 @@ def _fake_session_get(*args, **kwargs): rmtree(temp_dir) +def get_user_agent(): + return PipSession().headers["User-Agent"] + + def test_user_agent(): - user_agent = PipSession().headers["User-Agent"] + user_agent = get_user_agent() assert user_agent.startswith("pip/%s" % pip.__version__) @pytest.mark.parametrize('name, expected_like_ci', [ - ('BUILD', False), ('BUILD_BUILDID', True), ('BUILD_ID', True), ('CI', True), + # Test a prefix substring of one of the variable names we use. + ('BUILD', False), ]) def test_user_agent__ci(monkeypatch, name, expected_like_ci): - # Clear existing names since we can be running under an actual CI. - for ci_name in ('BUILD_BUILDID', 'BUILD_ID', 'CI'): + # Delete the variable names we use to check for CI to prevent the + # detection from always returning True in case the tests are being run + # under actual CI. It is okay to depend on CI_ENVIRONMENT_VARIABLES + # here (part of the code under test) because this setup step can only + # prevent false test failures. It can't cause a false test passage. + for ci_name in CI_ENVIRONMENT_VARIABLES: monkeypatch.delenv(ci_name, raising=False) - monkeypatch.setenv(name, 'true') - user_agent = PipSession().headers["User-Agent"] + # Confirm the baseline before setting the environment variable. + user_agent = get_user_agent() + assert '"ci":null' in user_agent + assert '"ci":true' not in user_agent + monkeypatch.setenv(name, 'true') + user_agent = get_user_agent() assert ('"ci":true' in user_agent) == expected_like_ci assert ('"ci":null' in user_agent) == (not expected_like_ci)