From c6ab89a34b45da6a3f84d97f0a3575beb18e6b41 Mon Sep 17 00:00:00 2001 From: "David A. Ramos" Date: Wed, 8 Nov 2023 19:35:18 -0800 Subject: [PATCH 1/6] Run autopep8 and isort --- setup.cfg | 7 ---- src/pytest_unflakable/__init__.py | 7 ++-- src/pytest_unflakable/_api.py | 8 ++-- src/pytest_unflakable/_git.py | 5 ++- src/pytest_unflakable/_plugin.py | 58 ++++++++++++++--------------- tests/common.py | 7 ++-- tests/test_unflakable.py | 62 +++++++++++++++---------------- tox.ini | 21 ++++++++++- 8 files changed, 92 insertions(+), 83 deletions(-) diff --git a/setup.cfg b/setup.cfg index 6adbab3..4016817 100644 --- a/setup.cfg +++ b/setup.cfg @@ -43,14 +43,7 @@ install_requires = [options.extras_require] dev = - flake8==5.0.4 - flake8-quotes - mypy - py>=1.9.0 requests-mock[fixture] - types-requests - types-setuptools - typing_extensions [options.entry_points] pytest11 = diff --git a/src/pytest_unflakable/__init__.py b/src/pytest_unflakable/__init__.py index 2676605..ef28902 100644 --- a/src/pytest_unflakable/__init__.py +++ b/src/pytest_unflakable/__init__.py @@ -3,18 +3,17 @@ # Copyright (c) 2022-2023 Developer Innovations, LLC import argparse - +import logging import os import pprint import sys from typing import TYPE_CHECKING import pytest -import logging from ._api import get_test_suite_manifest -from ._git import get_current_git_commit, get_current_git_branch -from ._plugin import UnflakablePlugin, QuarantineMode, UnflakableXdistHooks +from ._git import get_current_git_branch, get_current_git_commit +from ._plugin import QuarantineMode, UnflakablePlugin, UnflakableXdistHooks if TYPE_CHECKING: Config = pytest.Config diff --git a/src/pytest_unflakable/_api.py b/src/pytest_unflakable/_api.py index 0077c54..74e3936 100644 --- a/src/pytest_unflakable/_api.py +++ b/src/pytest_unflakable/_api.py @@ -2,13 +2,13 @@ # Copyright (c) 2022-2023 Developer Innovations, LLC -from typing import List, Optional, TYPE_CHECKING - -import sys import logging -import pkg_resources import platform import pprint +import sys +from typing import TYPE_CHECKING, List, Optional + +import pkg_resources import requests if TYPE_CHECKING: diff --git a/src/pytest_unflakable/_git.py b/src/pytest_unflakable/_git.py index 2c54ce2..88ab537 100644 --- a/src/pytest_unflakable/_git.py +++ b/src/pytest_unflakable/_git.py @@ -1,8 +1,9 @@ # Copyright (c) 2022-2023 Developer Innovations, LLC + import logging -from typing import Optional, List -from subprocess import run import sys +from subprocess import run +from typing import List, Optional GIT_ERROR_STR = 'WARNING: Unflakable failed to auto-detect current git branch and commit' GIT_ERROR_HINT = ( diff --git a/src/pytest_unflakable/_plugin.py b/src/pytest_unflakable/_plugin.py index cc89968..867185c 100644 --- a/src/pytest_unflakable/_plugin.py +++ b/src/pytest_unflakable/_plugin.py @@ -1,31 +1,27 @@ """Plugin implementation.""" + # Copyright (c) 2022-2023 Developer Innovations, LLC +import logging +from datetime import datetime, timezone from enum import Enum from pathlib import Path -from typing import ( - Any, Dict, List, Union, Tuple, Mapping, Optional, cast, Generator, Set, TYPE_CHECKING -) - -import logging +from time import time +from typing import (TYPE_CHECKING, Any, Dict, Generator, List, Mapping, + Optional, Set, Tuple, Union, cast) -import pytest import _pytest -from time import time -from datetime import datetime, timezone +import pytest -from ._api import ( - create_test_suite_run, - build_test_suite_run_url, - CreateTestSuiteRunRequest, - TestRunAttemptRecord, - TestRunRecord, TestAttemptResult, TestSuiteManifest, -) +from ._api import (CreateTestSuiteRunRequest, TestAttemptResult, + TestRunAttemptRecord, TestRunRecord, TestSuiteManifest, + build_test_suite_run_url, create_test_suite_run) if TYPE_CHECKING: - import py from typing import Literal + import py + # Most of these types aren't defined or exported in older versions of pytest, but we only need # the type checking to work on newer versions. CallInfo = pytest.CallInfo[None] @@ -311,7 +307,7 @@ def pytest_runtest_makereport( test_filename = relative_to(node_path(item), node_path(item.session)) test_name = item_name(item) is_quarantined = (test_filename, test_name) in self.quarantined_tests and ( - self.quarantine_mode == QuarantineMode.IGNORE_FAILURES) + self.quarantine_mode == QuarantineMode.IGNORE_FAILURES) assert self.session @@ -449,9 +445,9 @@ def pytest_report_teststatus( # empty string here to avoid double-counting. '' if (report.unflakable_is_quarantined and self.quarantine_mode == QuarantineMode.IGNORE_FAILURES) or ( - # If only the `teardown` phase has failed in the past, then don't - # treat the test as flaky just because the `call` phase passed. - not report.unflakable_prior_non_teardown_failures) + # If only the `teardown` phase has failed in the past, then don't + # treat the test as flaky just because the `call` phase passed. + not report.unflakable_prior_non_teardown_failures) else 'flaky' ), 'R', @@ -539,7 +535,7 @@ def _build_test_suite_run_request( attempt_result = ( cast(TestAttemptResult, 'quarantined') if is_quarantined and ( - self.quarantine_mode == QuarantineMode.IGNORE_FAILURES) else cast( + self.quarantine_mode == QuarantineMode.IGNORE_FAILURES) else cast( TestAttemptResult, 'fail') ) else: @@ -553,16 +549,16 @@ def _build_test_suite_run_request( 'end_time': _ts_to_rfc3339(end_time) if end_time is not None else None, 'duration_ms': int( ( - ( - item_attempt_reports['setup'].duration - if 'setup' in item_attempt_reports else 0. - ) + ( - item_attempt_reports['call'].duration - if 'call' in item_attempt_reports else 0. - ) + ( - item_attempt_reports['teardown'].duration - if 'teardown' in item_attempt_reports else 0. - ) + ( + item_attempt_reports['setup'].duration + if 'setup' in item_attempt_reports else 0. + ) + ( + item_attempt_reports['call'].duration + if 'call' in item_attempt_reports else 0. + ) + ( + item_attempt_reports['teardown'].duration + if 'teardown' in item_attempt_reports else 0. + ) ) * 1000. ), 'result': attempt_result, diff --git a/tests/common.py b/tests/common.py index 4a9188f..15af68e 100644 --- a/tests/common.py +++ b/tests/common.py @@ -1,13 +1,14 @@ """Tests for pytest_unflakable plugin.""" # Copyright (c) 2022-2023 Developer Innovations, LLC +import itertools +import re import subprocess from enum import Enum -from typing import List, Optional, Tuple, Dict, Iterable, cast, Callable, Sequence, TYPE_CHECKING +from typing import (TYPE_CHECKING, Callable, Dict, Iterable, List, Optional, + Sequence, Tuple, cast) -import itertools import pytest -import re import requests import requests_mock from _pytest.config import ExitCode diff --git a/tests/test_unflakable.py b/tests/test_unflakable.py index e2f70ef..e0cee5f 100644 --- a/tests/test_unflakable.py +++ b/tests/test_unflakable.py @@ -1,19 +1,19 @@ """Tests for pytest_unflakable plugin.""" -import os # Copyright (c) 2022-2023 Developer Innovations, LLC +import os +import platform +import sys + import pytest import requests_mock from _pytest.config import ExitCode -import sys -import platform from pytest_unflakable import _api -from .common import ( - GitMock, run_test_case, _TestAttemptOutcome, _TestResultCounts, MonkeyPatch -) +from .common import (GitMock, MonkeyPatch, _TestAttemptOutcome, + _TestResultCounts, run_test_case) requests_mock.mock.case_sensitive = True @@ -41,37 +41,37 @@ def _3platform() -> None: TEST_PARAMS_XDIST_ARG_NAMES = ['xdist'] TEST_PARAMS_XDIST_ARG_VALUES = ( - [ - pytest.param(False, id='not_xdist'), - ] + ([ - pytest.param(True, id='xdist'), - ] if os.environ.get('TEST_XDIST') == '1' else []) + [ + pytest.param(False, id='not_xdist'), + ] + ([ + pytest.param(True, id='xdist'), + ] if os.environ.get('TEST_XDIST') == '1' else []) ) TEST_PARAMS_VERBOSE_XDIST_ARG_NAMES = ['verbose', 'xdist'] TEST_PARAMS_VERBOSE_XDIST_ARG_VALUES = ( - [ - pytest.param(False, False, id='not_verbose-not_xdist'), - pytest.param(True, False, id='verbose-not_xdist'), - ] + ([ - pytest.param(False, True, id='not_verbose-xdist'), - pytest.param(True, True, id='verbose-xdist'), - ] if os.environ.get('TEST_XDIST') == '1' else []) + [ + pytest.param(False, False, id='not_verbose-not_xdist'), + pytest.param(True, False, id='verbose-not_xdist'), + ] + ([ + pytest.param(False, True, id='not_verbose-xdist'), + pytest.param(True, True, id='verbose-xdist'), + ] if os.environ.get('TEST_XDIST') == '1' else []) ) TEST_PARAMS_VERBOSE_QUARANTINED_XDIST_ARG_NAMES = ['verbose', 'quarantined', 'xdist'] TEST_PARAMS_VERBOSE_QUARANTINED_XDIST_ARG_VALUES = ( - [ - pytest.param(False, False, False, id='not_verbose-not_quarantined-not_xdist'), - pytest.param(False, True, False, id='not_verbose-quarantined-not_xdist'), - pytest.param(True, False, False, id='verbose-not_quarantined-not_xdist'), - pytest.param(True, True, False, id='verbose-quarantined-not_xdist'), - ] + ([ - pytest.param(False, False, True, id='not_verbose-not_quarantined-xdist'), - pytest.param(False, True, True, id='not_verbose-quarantined-xdist'), - pytest.param(True, False, True, id='verbose-not_quarantined-xdist'), - pytest.param(True, True, True, id='verbose-quarantined-xdist'), - ] if os.environ.get('TEST_XDIST') == '1' else []) + [ + pytest.param(False, False, False, id='not_verbose-not_quarantined-not_xdist'), + pytest.param(False, True, False, id='not_verbose-quarantined-not_xdist'), + pytest.param(True, False, False, id='verbose-not_quarantined-not_xdist'), + pytest.param(True, True, False, id='verbose-quarantined-not_xdist'), + ] + ([ + pytest.param(False, False, True, id='not_verbose-not_quarantined-xdist'), + pytest.param(False, True, True, id='not_verbose-quarantined-xdist'), + pytest.param(True, False, True, id='verbose-not_quarantined-xdist'), + pytest.param(True, True, True, id='verbose-quarantined-xdist'), + ] if os.environ.get('TEST_XDIST') == '1' else []) ) @@ -1126,8 +1126,8 @@ def test_pass(): expected_commit='CLI_COMMIT', expect_xdist=xdist, extra_args=[ - '--branch', 'CLI_BRANCH', '--commit', 'CLI_COMMIT' - ] + (XDIST_ARGS if xdist else []), + '--branch', 'CLI_BRANCH', '--commit', 'CLI_COMMIT' + ] + (XDIST_ARGS if xdist else []), ) diff --git a/tox.ini b/tox.ini index 5980e13..ac02eaf 100644 --- a/tox.ini +++ b/tox.ini @@ -1,6 +1,6 @@ # For more information about tox, see https://tox.readthedocs.io/en/latest/ [tox] -envlist = pytest{62,70,71,72,73,74},pytest{62,70,71,72,73,74}-xdist,flake8,mypy,pycodestyle +envlist = pytest{62,70,71,72,73,74},pytest{62,70,71,72,73,74}-xdist,autopep8,flake8,mypy,pycodestyle [testenv] extras = dev @@ -72,13 +72,32 @@ deps = setenv = TEST_XDIST = 1 +[testenv:autopep8] +deps = + autopep8==2.0.4 + isort==5.12.0 +commands = + isort src tests + autopep8 -i -r src tests + [testenv:flake8] +deps = + flake8==6.1.0 + flake8-quotes commands = flake8 src tests [testenv:mypy] +deps = + mypy==1.6.1 + py>=1.9.0 + types-requests + types-setuptools + typing_extensions commands = mypy --strict --allow-untyped-decorators src tests [testenv:pycodestyle] +deps = + pycodestyle==2.11.1 commands = pycodestyle src tests # See https://flake8.pycqa.org/en/latest/user/configuration.html. From 138c66ab24b4d234d7ad6196e2ae4a1eaa6a510c Mon Sep 17 00:00:00 2001 From: "David A. Ramos" Date: Thu, 9 Nov 2023 19:09:38 -0800 Subject: [PATCH 2/6] Avoid fetching manifest in xdist workers on failure xdist workers don't have access to the API key and cannot fetch the manifest. Prior to this change, the xdist worker would attempt to fetch the manifest if the fetch attempt in the parent process failed. This change fixes the bug so that xdist workers will never attempt to fetch the manifest, even if the fetch in the parent process failed. --- src/pytest_unflakable/__init__.py | 4 ++-- src/pytest_unflakable/_plugin.py | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/pytest_unflakable/__init__.py b/src/pytest_unflakable/__init__.py index ef28902..7aac282 100644 --- a/src/pytest_unflakable/__init__.py +++ b/src/pytest_unflakable/__init__.py @@ -159,8 +159,8 @@ def pytest_configure(config: Config) -> None: insecure_disable_tls_validation = config.getoption( 'unflakable_insecure_disable_tls_validation', False) manifest = None - if is_xdist_worker and 'unflakable_manifest' in config.workerinput: # type: ignore - manifest = config.workerinput['unflakable_manifest'] # type: ignore + if is_xdist_worker: + manifest = config.workerinput.get('unflakable_manifest') # type: ignore logger.debug( f'xdist worker received manifest for test suite {test_suite_id}: ' f'{pprint.pformat(manifest)}' diff --git a/src/pytest_unflakable/_plugin.py b/src/pytest_unflakable/_plugin.py index 867185c..7411ad8 100644 --- a/src/pytest_unflakable/_plugin.py +++ b/src/pytest_unflakable/_plugin.py @@ -165,8 +165,7 @@ def pytest_configure_node(self, node: Any) -> None: """ nodeid = node.workerinput['workerid'] self.logger.debug(f'called hook pytest_configure_node: {nodeid}') - if self.manifest is not None: - node.workerinput['unflakable_manifest'] = self.manifest + node.workerinput['unflakable_manifest'] = self.manifest class UnflakablePlugin: From df3b14ac885946a7a3b01dd0ecc149fa4c11262f Mon Sep 17 00:00:00 2001 From: "David A. Ramos" Date: Thu, 9 Nov 2023 19:30:59 -0800 Subject: [PATCH 3/6] Add test for failure to fetch manifest --- tests/common.py | 13 ++-- tests/test_unflakable.py | 158 ++++++++++++++++----------------------- 2 files changed, 70 insertions(+), 101 deletions(-) diff --git a/tests/common.py b/tests/common.py index 15af68e..fd7dfe2 100644 --- a/tests/common.py +++ b/tests/common.py @@ -244,10 +244,11 @@ def assert_regex(regex: str, string: str) -> None: assert re.match(regex, string) is not None, f'`{string}` does not match regex {regex}' +@requests_mock.Mocker(case_sensitive=True, kw='requests_mocker') def run_test_case( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, manifest: _api.TestSuiteManifest, + requests_mocker: requests_mock.Mocker, expected_test_file_outcomes: List[ Tuple[str, List[Tuple[Tuple[str, ...], List[_TestAttemptOutcome]]]]], expected_test_result_counts: _TestResultCounts, @@ -266,7 +267,7 @@ def run_test_case( expect_xdist: bool = False, ) -> None: api_key_path = pytester.makefile('', expected_api_key) if use_api_key_path else None - requests_mock.get( + requests_mocker.get( url='https://app.unflakable.com/api/v1/test-suites/MOCK_SUITE_ID/manifest', request_headers={'Authorization': f'Bearer {expected_api_key}'}, complete_qs=True, @@ -274,7 +275,7 @@ def run_test_case( json=manifest, ) - requests_mock.post( + requests_mocker.post( url='https://app.unflakable.com/api/v1/test-suites/MOCK_SUITE_ID/runs', request_headers={ 'Authorization': f'Bearer {expected_api_key}', @@ -482,7 +483,7 @@ def run_test_case( expected_test_result_counts.non_skipped_tests > 0) else []) ) - assert requests_mock.call_count == ( + assert requests_mocker.call_count == ( ( 2 if expected_uploaded_test_runs is not None and ( expected_test_result_counts.non_skipped_tests > 0) else 1 @@ -491,7 +492,7 @@ def run_test_case( # Checked expected User-Agent. We do this here instead of using an `additional_matcher` to make # errors easier to diagnose. - for request in requests_mock.request_history: + for request in requests_mocker.request_history: assert_regex( r'^unflakable-pytest-plugin/.* \(PyTest .*; Python .*; Platform .*\)$', request.headers.get('User-Agent', '') @@ -500,7 +501,7 @@ def run_test_case( if plugin_enabled and ( expected_uploaded_test_runs is not None and expected_test_result_counts.non_skipped_tests > 0): - create_test_suite_run_request = requests_mock.request_history[1] + create_test_suite_run_request = requests_mocker.request_history[1] assert create_test_suite_run_request.url == ( 'https://app.unflakable.com/api/v1/test-suites/MOCK_SUITE_ID/runs') assert create_test_suite_run_request.method == 'POST' diff --git a/tests/test_unflakable.py b/tests/test_unflakable.py index e0cee5f..6f030ec 100644 --- a/tests/test_unflakable.py +++ b/tests/test_unflakable.py @@ -4,10 +4,11 @@ import os import platform +import re import sys +import pkg_resources import pytest -import requests_mock from _pytest.config import ExitCode from pytest_unflakable import _api @@ -15,8 +16,6 @@ from .common import (GitMock, MonkeyPatch, _TestAttemptOutcome, _TestResultCounts, run_test_case) -requests_mock.mock.case_sensitive = True - # Run on 2 CPUs. XDIST_ARGS = ['-n', '2'] @@ -29,7 +28,15 @@ def _1python_version() -> None: pass -@pytest.fixture(params=[f'pytest{_api.PYTEST_VERSION}'], autouse=True) +__PYTEST_MINOR_VERSION_MATCH = re.match( + r'^([0-9]+\.[0-9]+)\..*$', + pkg_resources.get_distribution('pytest').version, +) +assert __PYTEST_MINOR_VERSION_MATCH is not None +__PYTEST_MINOR_VERSION = __PYTEST_MINOR_VERSION_MATCH.group(1) + + +@pytest.fixture(params=[f'pytest{__PYTEST_MINOR_VERSION}'], autouse=True) def _2pytest_version() -> None: pass @@ -39,38 +46,46 @@ def _3platform() -> None: pass +@pytest.fixture( + params=['xdist_installed' if os.environ.get('TEST_XDIST') == '1' else 'xdist_not_installed'], + autouse=True, +) +def _4xdist_installed() -> None: + pass + + TEST_PARAMS_XDIST_ARG_NAMES = ['xdist'] TEST_PARAMS_XDIST_ARG_VALUES = ( [ - pytest.param(False, id='not_xdist'), + pytest.param(False, id='xdist_disabled'), ] + ([ - pytest.param(True, id='xdist'), + pytest.param(True, id='xdist_enabled'), ] if os.environ.get('TEST_XDIST') == '1' else []) ) TEST_PARAMS_VERBOSE_XDIST_ARG_NAMES = ['verbose', 'xdist'] TEST_PARAMS_VERBOSE_XDIST_ARG_VALUES = ( [ - pytest.param(False, False, id='not_verbose-not_xdist'), - pytest.param(True, False, id='verbose-not_xdist'), + pytest.param(False, False, id='not_verbose-xdist_disabled'), + pytest.param(True, False, id='verbose-xdist_disabled'), ] + ([ - pytest.param(False, True, id='not_verbose-xdist'), - pytest.param(True, True, id='verbose-xdist'), + pytest.param(False, True, id='not_verbose-xdist_enabled'), + pytest.param(True, True, id='verbose-xdist_enabled'), ] if os.environ.get('TEST_XDIST') == '1' else []) ) TEST_PARAMS_VERBOSE_QUARANTINED_XDIST_ARG_NAMES = ['verbose', 'quarantined', 'xdist'] TEST_PARAMS_VERBOSE_QUARANTINED_XDIST_ARG_VALUES = ( [ - pytest.param(False, False, False, id='not_verbose-not_quarantined-not_xdist'), - pytest.param(False, True, False, id='not_verbose-quarantined-not_xdist'), - pytest.param(True, False, False, id='verbose-not_quarantined-not_xdist'), - pytest.param(True, True, False, id='verbose-quarantined-not_xdist'), + pytest.param(False, False, False, id='not_verbose-not_quarantined-xdist_disabled'), + pytest.param(False, True, False, id='not_verbose-quarantined-xdist_disabled'), + pytest.param(True, False, False, id='verbose-not_quarantined-xdist_disabled'), + pytest.param(True, True, False, id='verbose-quarantined-xdist_disabled'), ] + ([ - pytest.param(False, False, True, id='not_verbose-not_quarantined-xdist'), - pytest.param(False, True, True, id='not_verbose-quarantined-xdist'), - pytest.param(True, False, True, id='verbose-not_quarantined-xdist'), - pytest.param(True, True, True, id='verbose-quarantined-xdist'), + pytest.param(False, False, True, id='not_verbose-not_quarantined-xdist_enabled'), + pytest.param(False, True, True, id='not_verbose-quarantined-xdist_enabled'), + pytest.param(True, False, True, id='verbose-not_quarantined-xdist_enabled'), + pytest.param(True, True, True, id='verbose-quarantined-xdist_enabled'), ] if os.environ.get('TEST_XDIST') == '1' else []) ) @@ -78,7 +93,6 @@ def _3platform() -> None: @pytest.mark.parametrize(TEST_PARAMS_VERBOSE_XDIST_ARG_NAMES, TEST_PARAMS_VERBOSE_XDIST_ARG_VALUES) def test_flaky( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, verbose: bool, xdist: bool, @@ -98,7 +112,6 @@ def test_flaky(): run_test_case( pytester, - requests_mock, manifest={'quarantined_tests': []}, expected_test_file_outcomes=[ ( @@ -125,7 +138,6 @@ def test_flaky(): @pytest.mark.parametrize(TEST_PARAMS_VERBOSE_XDIST_ARG_NAMES, TEST_PARAMS_VERBOSE_XDIST_ARG_VALUES) def test_quarantine_flaky( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, verbose: bool, xdist: bool, @@ -145,7 +157,6 @@ def test_flaky(): run_test_case( pytester, - requests_mock, manifest={'quarantined_tests': [ { 'test_id': 'MOCK_TEST_ID', @@ -178,7 +189,6 @@ def test_flaky(): @pytest.mark.parametrize(TEST_PARAMS_VERBOSE_XDIST_ARG_NAMES, TEST_PARAMS_VERBOSE_XDIST_ARG_VALUES) def test_flaky_until_last_attempt( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, verbose: bool, xdist: bool, @@ -198,7 +208,6 @@ def test_flaky(): run_test_case( pytester, - requests_mock, manifest={'quarantined_tests': []}, expected_test_file_outcomes=[ ( @@ -226,7 +235,6 @@ def test_flaky(): @pytest.mark.parametrize(TEST_PARAMS_VERBOSE_XDIST_ARG_NAMES, TEST_PARAMS_VERBOSE_XDIST_ARG_VALUES) def test_all_statuses( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, verbose: bool, xdist: bool, @@ -276,7 +284,6 @@ def test_skipped(): run_test_case( pytester, - requests_mock, manifest, expected_test_file_outcomes=[ ( @@ -324,7 +331,6 @@ def test_skipped(): @pytest.mark.parametrize(TEST_PARAMS_VERBOSE_XDIST_ARG_NAMES, TEST_PARAMS_VERBOSE_XDIST_ARG_VALUES) def test_class_all_statuses( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, verbose: bool, xdist: bool, @@ -374,7 +380,6 @@ def test_skipped(self): run_test_case( pytester, - requests_mock, manifest, expected_test_file_outcomes=[ ( @@ -423,7 +428,6 @@ def test_skipped(self): @pytest.mark.parametrize(TEST_PARAMS_VERBOSE_XDIST_ARG_NAMES, TEST_PARAMS_VERBOSE_XDIST_ARG_VALUES) def test_nested_classes( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, verbose: bool, xdist: bool, @@ -478,7 +482,6 @@ def test_flaky(self): run_test_case( pytester, - requests_mock, manifest, expected_test_file_outcomes=[ ( @@ -518,7 +521,6 @@ def test_flaky(self): @pytest.mark.parametrize(TEST_PARAMS_VERBOSE_XDIST_ARG_NAMES, TEST_PARAMS_VERBOSE_XDIST_ARG_VALUES) def test_unittest_all_statuses( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, verbose: bool, xdist: bool, @@ -574,7 +576,6 @@ def test_skipped(self): run_test_case( pytester, - requests_mock, manifest, expected_test_file_outcomes=[ ( @@ -623,7 +624,6 @@ def test_skipped(self): @pytest.mark.parametrize(TEST_PARAMS_VERBOSE_XDIST_ARG_NAMES, TEST_PARAMS_VERBOSE_XDIST_ARG_VALUES) def test_multiple_files( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, verbose: bool, xdist: bool, @@ -653,7 +653,6 @@ def test_quarantined(): run_test_case( pytester, - requests_mock, manifest, expected_test_file_outcomes=[ ( @@ -689,7 +688,6 @@ def test_quarantined(): @pytest.mark.parametrize(TEST_PARAMS_VERBOSE_XDIST_ARG_NAMES, TEST_PARAMS_VERBOSE_XDIST_ARG_VALUES) def test_quarantine_mode_ignore_failures( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, verbose: bool, xdist: bool, @@ -717,7 +715,6 @@ def test_quarantined(): run_test_case( pytester, - requests_mock, manifest, expected_test_file_outcomes=[ ( @@ -747,7 +744,6 @@ def test_quarantined(): @pytest.mark.parametrize(TEST_PARAMS_VERBOSE_XDIST_ARG_NAMES, TEST_PARAMS_VERBOSE_XDIST_ARG_VALUES) def test_quarantine_mode_no_quarantine( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, verbose: bool, xdist: bool, @@ -775,7 +771,6 @@ def test_quarantined(): run_test_case( pytester, - requests_mock, manifest, expected_test_file_outcomes=[ ( @@ -805,7 +800,6 @@ def test_quarantined(): @pytest.mark.parametrize(TEST_PARAMS_VERBOSE_XDIST_ARG_NAMES, TEST_PARAMS_VERBOSE_XDIST_ARG_VALUES) def test_quarantine_mode_skip_tests( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, verbose: bool, xdist: bool, @@ -833,7 +827,6 @@ def test_quarantined(): run_test_case( pytester, - requests_mock, manifest, expected_test_file_outcomes=[ ( @@ -858,7 +851,6 @@ def test_quarantined(): @pytest.mark.parametrize(TEST_PARAMS_VERBOSE_XDIST_ARG_NAMES, TEST_PARAMS_VERBOSE_XDIST_ARG_VALUES) def test_parameterized( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, verbose: bool, xdist: bool, @@ -876,7 +868,6 @@ def test_with_param(p): run_test_case( pytester, - requests_mock, manifest={'quarantined_tests': [ { 'test_id': 'MOCK_TEST_ID', @@ -920,14 +911,12 @@ def test_with_param(p): @pytest.mark.parametrize(TEST_PARAMS_XDIST_ARG_NAMES, TEST_PARAMS_XDIST_ARG_VALUES) def test_empty_collection( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, xdist: bool, ) -> None: pytester.makepyfile(test_input='') run_test_case( pytester, - requests_mock, manifest={'quarantined_tests': []}, expected_test_file_outcomes=[], expected_test_result_counts=_TestResultCounts(), @@ -941,7 +930,6 @@ def test_empty_collection( @pytest.mark.parametrize(TEST_PARAMS_VERBOSE_XDIST_ARG_NAMES, TEST_PARAMS_VERBOSE_XDIST_ARG_VALUES) def test_all_skipped( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, verbose: bool, xdist: bool, @@ -956,7 +944,6 @@ def test_skipped(): """) run_test_case( pytester, - requests_mock, manifest={'quarantined_tests': []}, expected_test_file_outcomes=[], expected_test_result_counts=_TestResultCounts(num_skipped=1), @@ -971,7 +958,6 @@ def test_skipped(): @pytest.mark.parametrize(TEST_PARAMS_VERBOSE_XDIST_ARG_NAMES, TEST_PARAMS_VERBOSE_XDIST_ARG_VALUES) def test_skipped_and_pass( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, verbose: bool, xdist: bool, @@ -991,7 +977,6 @@ def test_skipped(): subprocess_mock.update(branch='MOCK_BRANCH', commit='MOCK_COMMIT') run_test_case( pytester, - requests_mock, manifest={'quarantined_tests': []}, expected_test_file_outcomes=[ ( @@ -1014,7 +999,6 @@ def test_skipped(): @pytest.mark.parametrize(TEST_PARAMS_XDIST_ARG_NAMES, TEST_PARAMS_XDIST_ARG_VALUES) def test_git_detached_head( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, xdist: bool, ) -> None: @@ -1027,7 +1011,6 @@ def test_pass(): run_test_case( pytester, - requests_mock, manifest={'quarantined_tests': []}, expected_test_file_outcomes=[ ('test_input.py', [(('test_pass',), [_TestAttemptOutcome.PASSED])])], @@ -1042,7 +1025,6 @@ def test_pass(): @pytest.mark.parametrize(TEST_PARAMS_XDIST_ARG_NAMES, TEST_PARAMS_XDIST_ARG_VALUES) def test_no_git_repo( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, xdist: bool, ) -> None: @@ -1055,7 +1037,6 @@ def test_pass(): run_test_case( pytester, - requests_mock, manifest={'quarantined_tests': []}, expected_test_file_outcomes=[ ('test_input.py', [(('test_pass',), [_TestAttemptOutcome.PASSED])])], @@ -1072,7 +1053,6 @@ def test_pass(): @pytest.mark.parametrize(TEST_PARAMS_XDIST_ARG_NAMES, TEST_PARAMS_XDIST_ARG_VALUES) def test_no_git_auto_detect( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, xdist: bool, ) -> None: @@ -1085,7 +1065,6 @@ def test_pass(): run_test_case( pytester, - requests_mock, manifest={'quarantined_tests': []}, expected_test_file_outcomes=[ ('test_input.py', [(('test_pass',), [_TestAttemptOutcome.PASSED])])], @@ -1102,7 +1081,6 @@ def test_pass(): @pytest.mark.parametrize(TEST_PARAMS_XDIST_ARG_NAMES, TEST_PARAMS_XDIST_ARG_VALUES) def test_git_cli_args( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, xdist: bool, ) -> None: @@ -1115,7 +1093,6 @@ def test_pass(): run_test_case( pytester, - requests_mock, manifest={'quarantined_tests': []}, expected_test_file_outcomes=[ ('test_input.py', [(('test_pass',), [_TestAttemptOutcome.PASSED])])], @@ -1134,7 +1111,6 @@ def test_pass(): @pytest.mark.parametrize(TEST_PARAMS_VERBOSE_XDIST_ARG_NAMES, TEST_PARAMS_VERBOSE_XDIST_ARG_VALUES) def test_no_retries( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, verbose: bool, xdist: bool, @@ -1154,7 +1130,6 @@ def test_flaky(): run_test_case( pytester, - requests_mock, manifest={'quarantined_tests': []}, expected_test_file_outcomes=[ ('test_input.py', [(('test_flaky',), [_TestAttemptOutcome.FAILED])]), @@ -1173,7 +1148,6 @@ def test_flaky(): @pytest.mark.parametrize(TEST_PARAMS_XDIST_ARG_NAMES, TEST_PARAMS_XDIST_ARG_VALUES) def test_api_key_environ( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, monkeypatch: MonkeyPatch, xdist: bool, @@ -1187,7 +1161,6 @@ def test_pass(): run_test_case( pytester, - requests_mock, manifest={'quarantined_tests': []}, expected_api_key='API_KEY_FROM_ENVIRON', expected_test_file_outcomes=[ @@ -1205,7 +1178,6 @@ def test_pass(): @pytest.mark.parametrize(TEST_PARAMS_XDIST_ARG_NAMES, TEST_PARAMS_XDIST_ARG_VALUES) def test_plugin_disabled( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, xdist: bool, ) -> None: @@ -1226,7 +1198,6 @@ def test_pass(): run_test_case( pytester, - requests_mock, manifest={'quarantined_tests': []}, expected_test_file_outcomes=[ ('test_input.py', [ @@ -1246,7 +1217,6 @@ def test_pass(): @pytest.mark.parametrize(TEST_PARAMS_XDIST_ARG_NAMES, TEST_PARAMS_XDIST_ARG_VALUES) def test_no_upload_results( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, xdist: bool, ) -> None: @@ -1273,7 +1243,6 @@ def test_quarantined(): run_test_case( pytester, - requests_mock, manifest, expected_test_file_outcomes=[ ( @@ -1299,7 +1268,6 @@ def test_quarantined(): @pytest.mark.parametrize(TEST_PARAMS_VERBOSE_XDIST_ARG_NAMES, TEST_PARAMS_VERBOSE_XDIST_ARG_VALUES) def test_select_subset( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, verbose: bool, xdist: bool, @@ -1320,7 +1288,6 @@ def test_skipped(): run_test_case( pytester, - requests_mock, manifest={'quarantined_tests': []}, expected_test_file_outcomes=[ ( @@ -1347,7 +1314,6 @@ def test_skipped(): ) def test_collect_only( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, verbose: bool, ) -> None: @@ -1363,7 +1329,6 @@ def test_pass(): run_test_case( pytester, - requests_mock, manifest={'quarantined_tests': []}, expected_test_file_outcomes=[], expected_test_result_counts=_TestResultCounts(num_collected=1), @@ -1378,7 +1343,6 @@ def test_pass(): TEST_PARAMS_VERBOSE_QUARANTINED_XDIST_ARG_VALUES) def test_setup_failure( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, verbose: bool, quarantined: bool, @@ -1405,7 +1369,6 @@ def test_setup_fail(setup_fail): run_test_case( pytester, - requests_mock, manifest={'quarantined_tests': [ { 'test_id': 'MOCK_TEST_ID', @@ -1451,7 +1414,6 @@ def test_setup_fail(setup_fail): TEST_PARAMS_VERBOSE_QUARANTINED_XDIST_ARG_VALUES) def test_setup_flaky( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, verbose: bool, quarantined: bool, @@ -1484,7 +1446,6 @@ def test_setup_flaky(setup_flaky): run_test_case( pytester, - requests_mock, manifest={'quarantined_tests': [ { 'test_id': 'MOCK_TEST_ID', @@ -1528,7 +1489,6 @@ def test_setup_flaky(setup_flaky): TEST_PARAMS_VERBOSE_QUARANTINED_XDIST_ARG_VALUES) def test_teardown_failure( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, verbose: bool, quarantined: bool, @@ -1556,7 +1516,6 @@ def test_teardown_fail(teardown_fail): run_test_case( pytester, - requests_mock, manifest={'quarantined_tests': [ { 'test_id': 'MOCK_TEST_ID', @@ -1611,7 +1570,6 @@ def test_teardown_fail(teardown_fail): TEST_PARAMS_VERBOSE_QUARANTINED_XDIST_ARG_VALUES) def test_teardown_flaky( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, verbose: bool, quarantined: bool, @@ -1645,7 +1603,6 @@ def test_teardown_flaky(teardown_flaky): run_test_case( pytester, - requests_mock, manifest={'quarantined_tests': [ { 'test_id': 'MOCK_TEST_ID', @@ -1691,7 +1648,6 @@ def test_teardown_flaky(teardown_flaky): @pytest.mark.parametrize(TEST_PARAMS_VERBOSE_XDIST_ARG_NAMES, TEST_PARAMS_VERBOSE_XDIST_ARG_VALUES) def test_xfail_pass( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, verbose: bool, xdist: bool, @@ -1709,7 +1665,6 @@ def test_xfail(): run_test_case( pytester, - requests_mock, manifest={'quarantined_tests': []}, expected_test_file_outcomes=[ ('test_input.py', [(('test_xfail',), [_TestAttemptOutcome.XFAILED])]), @@ -1728,7 +1683,6 @@ def test_xfail(): @pytest.mark.parametrize(TEST_PARAMS_VERBOSE_XDIST_ARG_NAMES, TEST_PARAMS_VERBOSE_XDIST_ARG_VALUES) def test_xfail_fail( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, verbose: bool, xdist: bool, @@ -1747,7 +1701,6 @@ def test_xfail(): run_test_case( pytester, - requests_mock, manifest={'quarantined_tests': []}, expected_test_file_outcomes=[ ('test_input.py', [ @@ -1771,7 +1724,6 @@ def test_xfail(): TEST_PARAMS_VERBOSE_QUARANTINED_XDIST_ARG_VALUES) def test_xfail_fail_strict( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, verbose: bool, quarantined: bool, @@ -1791,7 +1743,6 @@ def test_xfail(): run_test_case( pytester, - requests_mock, manifest={'quarantined_tests': [ { 'test_id': 'MOCK_TEST_ID', @@ -1831,7 +1782,6 @@ def test_xfail(): TEST_PARAMS_VERBOSE_QUARANTINED_XDIST_ARG_VALUES) def test_xfail_flaky_strict( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, verbose: bool, quarantined: bool, @@ -1858,7 +1808,6 @@ def test_xfail(): run_test_case( pytester, - requests_mock, manifest={'quarantined_tests': [ { 'test_id': 'MOCK_TEST_ID', @@ -1895,7 +1844,6 @@ def test_xfail(): @pytest.mark.parametrize(TEST_PARAMS_XDIST_ARG_NAMES, TEST_PARAMS_XDIST_ARG_VALUES) def test_warnings( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, xdist: bool, ) -> None: @@ -1914,7 +1862,6 @@ def test_pass(): run_test_case( pytester, - requests_mock, manifest, expected_test_file_outcomes=[ ('test_input.py', [(('test_pass',), [_TestAttemptOutcome.PASSED])]), @@ -1940,7 +1887,6 @@ def test_pass(): ) def test_stepwise( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, verbose: bool, quarantined: bool, @@ -1973,7 +1919,6 @@ def test_pass2(): run_test_case( pytester, - requests_mock, manifest, expected_test_file_outcomes=[ ('test_input.py', [ @@ -2004,11 +1949,8 @@ def test_pass2(): expect_progress=False, ) - requests_mock.reset_mock() - run_test_case( pytester, - requests_mock, manifest, expected_test_file_outcomes=[ ('test_input.py', [ @@ -2056,7 +1998,6 @@ def test_pass2(): ) def test_xdist( pytester: pytest.Pytester, - requests_mock: requests_mock.Mocker, subprocess_mock: GitMock, verbose: bool, quarantined: bool, @@ -2107,7 +2048,6 @@ def test_skipped(): run_test_case( pytester, - requests_mock, manifest, expected_test_file_outcomes=[ ('test_input1.py', [ @@ -2165,7 +2105,35 @@ def test_skipped(): }, expected_exit_code=ExitCode.TESTS_FAILED, verbose=verbose, - # Run on 2 CPUs. - extra_args=['-n', '2'], # , '--debug'], + extra_args=XDIST_ARGS, expect_xdist=True, ) + + +# Run should pass even if we fail to fetch the manifest. +@pytest.mark.parametrize(TEST_PARAMS_XDIST_ARG_NAMES, TEST_PARAMS_XDIST_ARG_VALUES) +def test_fetch_failure( + pytester: pytest.Pytester, + subprocess_mock: GitMock, + xdist: bool, +) -> None: + pytester.makepyfile(test_input=""" + def test_pass(): + pass + """) + + subprocess_mock.update(branch=None, commit=None) + + run_test_case( + pytester, + manifest=None, + expected_test_file_outcomes=[ + ('test_input.py', [(('test_pass',), [_TestAttemptOutcome.PASSED])])], + expected_test_result_counts=_TestResultCounts(num_passed=1), + expected_uploaded_test_runs={('test_input.py', ('test_pass',)): ['pass']}, + expected_exit_code=ExitCode.OK, + expected_branch=None, + expected_commit=None, + expect_xdist=xdist, + extra_args=XDIST_ARGS if xdist else [], + ) From 1f6aca3b9cce222d697b81671d20bd1e8e0fb2a8 Mon Sep 17 00:00:00 2001 From: "David A. Ramos" Date: Sat, 11 Nov 2023 17:21:12 -0800 Subject: [PATCH 4/6] Add retries to API requests --- src/pytest_unflakable/_api.py | 68 ++++++++++++++--- src/pytest_unflakable/_plugin.py | 8 +- tests/__init__.py | 3 + tests/common.py | 124 ++++++++++++++++++++++--------- tests/test_unflakable.py | 107 ++++++++++++++++++++++++++ 5 files changed, 262 insertions(+), 48 deletions(-) diff --git a/src/pytest_unflakable/_api.py b/src/pytest_unflakable/_api.py index 74e3936..635ee2c 100644 --- a/src/pytest_unflakable/_api.py +++ b/src/pytest_unflakable/_api.py @@ -2,14 +2,18 @@ # Copyright (c) 2022-2023 Developer Innovations, LLC +from __future__ import annotations + import logging import platform import pprint import sys -from typing import TYPE_CHECKING, List, Optional +import time +from typing import TYPE_CHECKING, Any, List, Mapping, Optional import pkg_resources import requests +from requests import Response, Session if TYPE_CHECKING: from typing_extensions import NotRequired, TypedDict @@ -29,6 +33,7 @@ f'unflakable-pytest-plugin/{PACKAGE_VERSION} (PyTest {PYTEST_VERSION}; ' f'Python {PYTHON_VERSION}; Platform {PLATFORM_VERSION})' ) +NUM_REQUEST_TRIES = 3 class TestRef(TypedDict): @@ -77,6 +82,47 @@ class TestSuiteRunPendingSummary(TypedDict): commit: NotRequired[Optional[str]] +def send_api_request( + api_key: str, + method: Literal['GET', 'POST'], + url: str, + logger: logging.Logger, + headers: Optional[Mapping[str, str | bytes | None]] = None, + json: Optional[Any] = None, + verify: Optional[bool | str] = None, +) -> Response: + session = Session() + session.headers.update({ + 'Authorization': f'Bearer {api_key}', + 'User-Agent': USER_AGENT, + }) + + for idx in range(NUM_REQUEST_TRIES): + try: + response = session.request(method, url, headers=headers, json=json, verify=verify) + if response.status_code not in [429, 500, 502, 503, 504]: + return response + elif idx + 1 != NUM_REQUEST_TRIES: + logger.warning( + 'Retrying request to `%s` due to unexpected response with status code %d' % ( + url, + response.status_code, + ) + ) + except (requests.exceptions.ConnectionError, requests.exceptions.Timeout) as e: + if idx + 1 != NUM_REQUEST_TRIES: + logger.warning('Retrying %s request to `%s` due to error: %s' % + (method, url, repr(e))) + else: + raise + + sleep_sec = (2 ** idx) + logger.debug('Sleeping for %f second(s) before retry' % sleep_sec) + time.sleep(sleep_sec) + + return response + + def create_test_suite_run( request: CreateTestSuiteRunRequest, test_suite_id: str, @@ -87,16 +133,15 @@ def create_test_suite_run( ) -> TestSuiteRunPendingSummary: logger.debug(f'creating test suite run {pprint.pformat(request)}') - run_response = requests.post( + run_response = send_api_request( + api_key=api_key, + method='POST', url=( f'{base_url if base_url is not None else BASE_URL}/api/v1/test-suites/{test_suite_id}' '/runs' ), - headers={ - 'Authorization': f'Bearer {api_key}', - 'Content-Type': 'application/json', - 'User-Agent': USER_AGENT, - }, + logger=logger, + headers={'Content-Type': 'application/json'}, json=request, verify=not insecure_disable_tls_validation, ) @@ -117,15 +162,14 @@ def get_test_suite_manifest( ) -> TestSuiteManifest: logger.debug(f'fetching manifest for test suite {test_suite_id}') - manifest_response = requests.get( + manifest_response = send_api_request( + api_key=api_key, + method='GET', url=( f'{base_url if base_url is not None else BASE_URL}/api/v1/test-suites/{test_suite_id}' '/manifest' ), - headers={ - 'Authorization': f'Bearer {api_key}', - 'User-Agent': USER_AGENT, - }, + logger=logger, verify=not insecure_disable_tls_validation, ) manifest_response.raise_for_status() diff --git a/src/pytest_unflakable/_plugin.py b/src/pytest_unflakable/_plugin.py index 7411ad8..b6ab3ea 100644 --- a/src/pytest_unflakable/_plugin.py +++ b/src/pytest_unflakable/_plugin.py @@ -12,6 +12,7 @@ import _pytest import pytest +from _pytest.config import ExitCode from ._api import (CreateTestSuiteRunRequest, TestAttemptResult, TestRunAttemptRecord, TestRunRecord, TestSuiteManifest, @@ -609,7 +610,10 @@ def pytest_sessionfinish( logger=self.logger, ) except Exception as e: - pytest.exit('ERROR: Failed to report results to Unflakable: %s\n' % (repr(e)), 1) + pytest.exit( + 'ERROR: Failed to report results to Unflakable: %s\n' % (repr(e)), + ExitCode.INTERNAL_ERROR, + ) else: print( 'Unflakable report: %s' % ( @@ -623,4 +627,4 @@ def pytest_sessionfinish( # We multiply by 2 here because each quarantined test is double-counted by the Session: once # for the quarantined report, and once for the fake report that's used for logging errors. if session.testsfailed > 0 and session.testsfailed == self.num_tests_quarantined * 2: - pytest.exit('All failed tests are quarantined', 0) + pytest.exit('All failed tests are quarantined', ExitCode.OK) diff --git a/tests/__init__.py b/tests/__init__.py index e69de29..76094f9 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -0,0 +1,3 @@ +import pytest + +pytest.register_assert_rewrite('tests.common') diff --git a/tests/common.py b/tests/common.py index fd7dfe2..d54dd8f 100644 --- a/tests/common.py +++ b/tests/common.py @@ -7,6 +7,8 @@ from enum import Enum from typing import (TYPE_CHECKING, Callable, Dict, Iterable, List, Optional, Sequence, Tuple, cast) +from unittest import mock +from unittest.mock import Mock, call, patch import pytest import requests @@ -244,11 +246,13 @@ def assert_regex(regex: str, string: str) -> None: assert re.match(regex, string) is not None, f'`{string}` does not match regex {regex}' +@patch.multiple('time', sleep=mock.DEFAULT) @requests_mock.Mocker(case_sensitive=True, kw='requests_mocker') def run_test_case( pytester: pytest.Pytester, - manifest: _api.TestSuiteManifest, + manifest: Optional[_api.TestSuiteManifest], requests_mocker: requests_mock.Mocker, + sleep: Mock, expected_test_file_outcomes: List[ Tuple[str, List[Tuple[Tuple[str, ...], List[_TestAttemptOutcome]]]]], expected_test_result_counts: _TestResultCounts, @@ -265,14 +269,21 @@ def run_test_case( env_vars: Optional[Dict[str, str]] = None, expect_progress: bool = True, expect_xdist: bool = False, + failed_manifest_requests: int = 0, + failed_upload_requests: int = 0, ) -> None: api_key_path = pytester.makefile('', expected_api_key) if use_api_key_path else None requests_mocker.get( url='https://app.unflakable.com/api/v1/test-suites/MOCK_SUITE_ID/manifest', request_headers={'Authorization': f'Bearer {expected_api_key}'}, complete_qs=True, - status_code=200, - json=manifest, + response_list=[ + {'exc': requests.exceptions.ConnectTimeout} + for _ in range(failed_manifest_requests) + ] + ([{ + 'status_code': 200, + 'json': manifest, + }] if manifest is not None else []) ) requests_mocker.post( @@ -282,8 +293,13 @@ def run_test_case( 'Content-Type': 'application/json', }, complete_qs=True, - status_code=201, - json=mock_create_test_suite_run_response, + response_list=[ + {'exc': requests.exceptions.ConnectTimeout} + for _ in range(failed_upload_requests) + ] + [{ + 'status_code': 201, + 'json': mock_create_test_suite_run_response, + }] ) pytest_args: List[str] = ( @@ -483,42 +499,82 @@ def run_test_case( expected_test_result_counts.non_skipped_tests > 0) else []) ) - assert requests_mocker.call_count == ( - ( - 2 if expected_uploaded_test_runs is not None and ( - expected_test_result_counts.non_skipped_tests > 0) else 1 - ) if plugin_enabled else 0 - ) + if plugin_enabled: + expected_get_test_suite_manifest_attempts = ( + failed_manifest_requests + (1 if failed_manifest_requests < + _api.NUM_REQUEST_TRIES and manifest is not None else 0) + ) + for manifest_attempt in range(expected_get_test_suite_manifest_attempts): + request = requests_mocker.request_history[manifest_attempt] - # Checked expected User-Agent. We do this here instead of using an `additional_matcher` to make - # errors easier to diagnose. - for request in requests_mocker.request_history: - assert_regex( - r'^unflakable-pytest-plugin/.* \(PyTest .*; Python .*; Platform .*\)$', - request.headers.get('User-Agent', '') + assert request.url == ( + 'https://app.unflakable.com/api/v1/test-suites/MOCK_SUITE_ID/manifest' + ) + assert request.method == 'GET' + assert request.body is None + + if manifest_attempt > 0: + assert ( + sleep.call_args_list[manifest_attempt - 1] == call(2 ** (manifest_attempt - 1)) + ) + + expected_upload_attempts = ( + failed_upload_requests + (1 if ( + failed_upload_requests < _api.NUM_REQUEST_TRIES + and expected_uploaded_test_runs is not None + and expected_test_result_counts.non_skipped_tests != 0 + ) else 0) ) - if plugin_enabled and ( - expected_uploaded_test_runs is not None and - expected_test_result_counts.non_skipped_tests > 0): - create_test_suite_run_request = requests_mocker.request_history[1] - assert create_test_suite_run_request.url == ( - 'https://app.unflakable.com/api/v1/test-suites/MOCK_SUITE_ID/runs') - assert create_test_suite_run_request.method == 'POST' + for upload_attempt in range(expected_upload_attempts): + create_test_suite_run_request = requests_mocker.request_history[ + expected_get_test_suite_manifest_attempts + upload_attempt + ] + assert create_test_suite_run_request.url == ( + 'https://app.unflakable.com/api/v1/test-suites/MOCK_SUITE_ID/runs') + assert create_test_suite_run_request.method == 'POST' + + create_test_suite_run_body: _api.CreateTestSuiteRunRequest = ( + create_test_suite_run_request.json() + ) - create_test_suite_run_body: _api.CreateTestSuiteRunRequest = ( - create_test_suite_run_request.json() + if expected_commit is not None: + assert create_test_suite_run_body['commit'] == expected_commit + else: + assert 'commit' not in create_test_suite_run_body + + if expected_branch is not None: + assert create_test_suite_run_body['branch'] == expected_branch + else: + assert 'branch' not in create_test_suite_run_body + + if upload_attempt > 0: + assert ( + sleep.call_args_list[ + max(expected_get_test_suite_manifest_attempts - 1, 0) + + upload_attempt - 1 + ] == call(2 ** (upload_attempt - 1)) + ) + + assert requests_mocker.call_count == ( + expected_get_test_suite_manifest_attempts + expected_upload_attempts + ), 'Expected %d total API requests, but received %d' % ( + expected_get_test_suite_manifest_attempts + expected_upload_attempts, + requests_mocker.call_count, ) - if expected_commit is not None: - assert create_test_suite_run_body['commit'] == expected_commit - else: - assert 'commit' not in create_test_suite_run_body + # Checked expected User-Agent. We do this here instead of using an `additional_matcher` to + # make errors easier to diagnose. + for request in requests_mocker.request_history: + assert request.headers.get('Authorization', '') == f'Bearer {expected_api_key}' - if expected_branch is not None: - assert create_test_suite_run_body['branch'] == expected_branch - else: - assert 'branch' not in create_test_suite_run_body + assert_regex( + r'^unflakable-pytest-plugin/.* \(PyTest .*; Python .*; Platform .*\)$', + request.headers.get('User-Agent', '') + ) + else: + assert requests_mocker.call_count == 0 + assert sleep.call_count == 0 assert result.ret == expected_exit_code, ( f'expected exit code {expected_exit_code}, but got {result.ret}') diff --git a/tests/test_unflakable.py b/tests/test_unflakable.py index 6f030ec..6def7c7 100644 --- a/tests/test_unflakable.py +++ b/tests/test_unflakable.py @@ -2136,4 +2136,111 @@ def test_pass(): expected_commit=None, expect_xdist=xdist, extra_args=XDIST_ARGS if xdist else [], + failed_manifest_requests=_api.NUM_REQUEST_TRIES, + ) + + +# The manifest should be followed even if the initial fetch request fails. +@pytest.mark.parametrize(TEST_PARAMS_XDIST_ARG_NAMES, TEST_PARAMS_XDIST_ARG_VALUES) +def test_fetch_retry( + pytester: pytest.Pytester, + subprocess_mock: GitMock, + xdist: bool, +) -> None: + pytester.makepyfile(test_input=""" + def test_quarantined(): + assert False + """) + + subprocess_mock.update(branch=None, commit=None) + + run_test_case( + pytester, + manifest={ + 'quarantined_tests': [ + { + 'test_id': 'MOCK_TEST_ID', + 'filename': 'test_input.py', + 'name': ['test_quarantined'] + } + ] + }, + expected_test_file_outcomes=[ + ('test_input.py', [ + (('test_quarantined',), [ + _TestAttemptOutcome.QUARANTINED, + _TestAttemptOutcome.RETRY_QUARANTINED, + _TestAttemptOutcome.RETRY_QUARANTINED, + ]), + ]), + ], + expected_test_result_counts=_TestResultCounts(num_quarantined=1), + expected_uploaded_test_runs={('test_input.py', ('test_quarantined',)): [ + 'quarantined', 'quarantined', 'quarantined']}, + expected_exit_code=ExitCode.OK, + expected_branch=None, + expected_commit=None, + expect_xdist=xdist, + extra_args=XDIST_ARGS if xdist else [], + failed_manifest_requests=1, + ) + + +# The run should fail if we fail to upload the results. +@pytest.mark.parametrize(TEST_PARAMS_XDIST_ARG_NAMES, TEST_PARAMS_XDIST_ARG_VALUES) +def test_upload_failure( + pytester: pytest.Pytester, + subprocess_mock: GitMock, + xdist: bool, +) -> None: + pytester.makepyfile(test_input=""" + def test_pass(): + pass + """) + + subprocess_mock.update(branch=None, commit=None) + + run_test_case( + pytester, + manifest={'quarantined_tests': []}, + expected_test_file_outcomes=[ + ('test_input.py', [(('test_pass',), [_TestAttemptOutcome.PASSED])])], + expected_test_result_counts=_TestResultCounts(num_passed=1), + expected_uploaded_test_runs=None, + expected_exit_code=ExitCode.INTERNAL_ERROR, + expected_branch=None, + expected_commit=None, + expect_xdist=xdist, + extra_args=XDIST_ARGS if xdist else [], + failed_upload_requests=_api.NUM_REQUEST_TRIES, + ) + + +# The run should succeed even if the first upload attempt fails. +@pytest.mark.parametrize(TEST_PARAMS_XDIST_ARG_NAMES, TEST_PARAMS_XDIST_ARG_VALUES) +def test_upload_retry( + pytester: pytest.Pytester, + subprocess_mock: GitMock, + xdist: bool, +) -> None: + pytester.makepyfile(test_input=""" + def test_pass(): + pass + """) + + subprocess_mock.update(branch=None, commit=None) + + run_test_case( + pytester, + manifest={'quarantined_tests': []}, + expected_test_file_outcomes=[ + ('test_input.py', [(('test_pass',), [_TestAttemptOutcome.PASSED])])], + expected_test_result_counts=_TestResultCounts(num_passed=1), + expected_uploaded_test_runs={('test_input.py', ('test_pass',)): ['pass']}, + expected_exit_code=ExitCode.OK, + expected_branch=None, + expected_commit=None, + expect_xdist=xdist, + extra_args=XDIST_ARGS if xdist else [], + failed_upload_requests=1, ) From abe3fecaaee76abc0bb6ecae8ffc80d2470b38df Mon Sep 17 00:00:00 2001 From: "David A. Ramos" Date: Sat, 11 Nov 2023 21:50:18 -0800 Subject: [PATCH 5/6] Support test run payloads over 6MB AWS Lambda limits payloads to 6MB, which puts an upper bound on the size of the Unflakable backend API's request bodies. This change leverages a newer backend API endpoint to upload test runs to presigned S3 URLs, which are not subject to AWS Lambda limits. The backend then retrieves the upload directly from S3. --- src/pytest_unflakable/_api.py | 106 +++++++++++++++---- src/pytest_unflakable/_plugin.py | 6 +- tests/common.py | 173 ++++++++++++++++++++++++++----- tests/test_unflakable.py | 3 + 4 files changed, 242 insertions(+), 46 deletions(-) diff --git a/src/pytest_unflakable/_api.py b/src/pytest_unflakable/_api.py index 635ee2c..bce77a5 100644 --- a/src/pytest_unflakable/_api.py +++ b/src/pytest_unflakable/_api.py @@ -4,16 +4,18 @@ from __future__ import annotations +import gzip +import json import logging import platform import pprint import sys import time -from typing import TYPE_CHECKING, Any, List, Mapping, Optional +from typing import TYPE_CHECKING, List, Mapping, Optional import pkg_resources import requests -from requests import Response, Session +from requests import HTTPError, Response, Session if TYPE_CHECKING: from typing_extensions import NotRequired, TypedDict @@ -67,7 +69,7 @@ class TestRunRecord(TypedDict): attempts: List[TestRunAttemptRecord] -class CreateTestSuiteRunRequest(TypedDict): +class CreateTestSuiteRunInlineRequest(TypedDict): branch: NotRequired[Optional[str]] commit: NotRequired[Optional[str]] start_time: str @@ -75,6 +77,14 @@ class CreateTestSuiteRunRequest(TypedDict): test_runs: List[TestRunRecord] +class CreateTestSuiteRunUploadRequest(TypedDict): + upload_id: str + + +class CreateTestSuiteRunUploadUrlResponse(TypedDict): + upload_id: str + + class TestSuiteRunPendingSummary(TypedDict): run_id: str suite_id: str @@ -82,24 +92,35 @@ class TestSuiteRunPendingSummary(TypedDict): commit: NotRequired[Optional[str]] -def send_api_request( - api_key: str, - method: Literal['GET', 'POST'], +def __new_requests_session() -> Session: + session = Session() + session.headers['User-Agent'] = USER_AGENT + + return session + + +def __send_api_request( + session: Session, + api_key: Optional[str], + method: Literal['GET', 'POST', 'PUT'], url: str, logger: logging.Logger, headers: Optional[Mapping[str, str | bytes | None]] = None, - json: Optional[Any] = None, + body: Optional[str | bytes] = None, verify: Optional[bool | str] = None, ) -> Response: - session = Session() - session.headers.update({ - 'Authorization': f'Bearer {api_key}', - 'User-Agent': USER_AGENT, - }) - for idx in range(NUM_REQUEST_TRIES): try: - response = session.request(method, url, headers=headers, json=json, verify=verify) + response = session.request( + method, + url, + headers={ + **({'Authorization': f'Bearer {api_key}'} if api_key is not None else {}), + **(headers if headers is not None else {}) + }, + data=body, + verify=verify, + ) if response.status_code not in [429, 500, 502, 503, 504]: return response elif idx + 1 != NUM_REQUEST_TRIES: @@ -124,7 +145,7 @@ def send_api_request( def create_test_suite_run( - request: CreateTestSuiteRunRequest, + request: CreateTestSuiteRunInlineRequest, test_suite_id: str, api_key: str, base_url: Optional[str], @@ -133,7 +154,53 @@ def create_test_suite_run( ) -> TestSuiteRunPendingSummary: logger.debug(f'creating test suite run {pprint.pformat(request)}') - run_response = send_api_request( + session = __new_requests_session() + + create_upload_url_response = __send_api_request( + session=session, + api_key=api_key, + method='POST', + url=( + f'{base_url if base_url is not None else BASE_URL}/api/v1/test-suites/{test_suite_id}' + '/runs/upload' + ), + logger=logger, + verify=not insecure_disable_tls_validation, + ) + + create_upload_url_response.raise_for_status() + if create_upload_url_response.status_code != 201: + raise HTTPError( + f'Expected 201 response but received {create_upload_url_response.status_code}') + + upload_presigned_url = create_upload_url_response.headers.get('Location', None) + if upload_presigned_url is None: + raise HTTPError('Location response header not found') + + create_upload_url_response_body: CreateTestSuiteRunUploadUrlResponse = ( + create_upload_url_response.json() + ) + upload_id = create_upload_url_response_body['upload_id'] + + gzipped_request = gzip.compress(json.dumps(request).encode('utf8')) + upload_response = __send_api_request( + session=session, + api_key=None, + method='PUT', + url=upload_presigned_url, + logger=logger, + headers={ + 'Content-Encoding': 'gzip', + 'Content-Type': 'application/json', + }, + body=gzipped_request, + verify=not insecure_disable_tls_validation, + ) + upload_response.raise_for_status() + + request_body: CreateTestSuiteRunUploadRequest = {'upload_id': upload_id} + run_response = __send_api_request( + session=session, api_key=api_key, method='POST', url=( @@ -142,7 +209,7 @@ def create_test_suite_run( ), logger=logger, headers={'Content-Type': 'application/json'}, - json=request, + body=json.dumps(request_body).encode('utf8'), verify=not insecure_disable_tls_validation, ) run_response.raise_for_status() @@ -162,7 +229,10 @@ def get_test_suite_manifest( ) -> TestSuiteManifest: logger.debug(f'fetching manifest for test suite {test_suite_id}') - manifest_response = send_api_request( + session = __new_requests_session() + + manifest_response = __send_api_request( + session=session, api_key=api_key, method='GET', url=( diff --git a/src/pytest_unflakable/_plugin.py b/src/pytest_unflakable/_plugin.py index b6ab3ea..6bea264 100644 --- a/src/pytest_unflakable/_plugin.py +++ b/src/pytest_unflakable/_plugin.py @@ -14,7 +14,7 @@ import pytest from _pytest.config import ExitCode -from ._api import (CreateTestSuiteRunRequest, TestAttemptResult, +from ._api import (CreateTestSuiteRunInlineRequest, TestAttemptResult, TestRunAttemptRecord, TestRunRecord, TestSuiteManifest, build_test_suite_run_url, create_test_suite_run) @@ -496,7 +496,7 @@ def pytest_sessionstart(self, session: pytest.Session) -> None: def _build_test_suite_run_request( self, session: pytest.Session, - ) -> CreateTestSuiteRunRequest: + ) -> CreateTestSuiteRunInlineRequest: test_runs: List[TestRunRecord] = [] for (test_filename, test_name), item_reports in self.item_reports.items(): is_quarantined = (test_filename, test_name) in self.quarantined_tests @@ -581,7 +581,7 @@ def _build_test_suite_run_request( request.update(**({'branch': self.branch} if self.branch is not None else {})) request.update(**({'commit': self.commit} if self.commit is not None else {})) - return cast(CreateTestSuiteRunRequest, request) + return cast(CreateTestSuiteRunInlineRequest, request) # Allows us to override the exit code if all the failures are quarantined. We need this to be a # wrapper so that the default hook still gets invoked and prints the summary line with the test diff --git a/tests/common.py b/tests/common.py index d54dd8f..88dc8e2 100644 --- a/tests/common.py +++ b/tests/common.py @@ -1,7 +1,10 @@ """Tests for pytest_unflakable plugin.""" - +import gzip +import hashlib # Copyright (c) 2022-2023 Developer Innovations, LLC import itertools +import json +import os import re import subprocess from enum import Enum @@ -27,6 +30,7 @@ MOCK_RUN_ID = 'MOCK_RUN_ID' MOCK_SUITE_ID = 'MOCK_SUITE_ID' +MOCK_TEAM_ID = 'MOCK_TEAM_ID' # e.g., 2022-01-23T04:05:06.000000+00:00 TIMESTAMP_REGEX = ( @@ -229,16 +233,72 @@ def mock_run( raise RuntimeError(f'unexpected git call with args: {repr(args)}') -def mock_create_test_suite_run_response( - request: requests.Request, +__uploads: Dict[str, Optional[_api.CreateTestSuiteRunInlineRequest]] = {} + + +def __upload_id_for_current_test() -> str: + return hashlib.sha1(os.environ['PYTEST_CURRENT_TEST'].encode('utf8')).hexdigest() + + +def __upload_url(upload_id: str) -> str: + return ( + f'https://s3.mock.amazonaws.com/unflakable-backend-mock-test-uploads/teams/{MOCK_TEAM_ID}' + f'/suites/{MOCK_SUITE_ID}/runs/upload/{upload_id}?X-Amz-Signature=MOCK_SIGNATURE' + ) + + +def __mock_create_test_suite_run_upload_url_response( + upload_id: str, + request: requests_mock.request._RequestObjectProxy, + context: requests_mock.response._Context, +) -> _api.CreateTestSuiteRunUploadUrlResponse: + upload_url = __upload_url(upload_id) + assert upload_url not in __uploads + __uploads[upload_url] = None + + context.headers['Location'] = upload_url + return { + 'upload_id': upload_id, + } + + +def __match_upload(request: requests_mock.request._RequestObjectProxy) -> bool: + return re.match( + r'^%s[0-9a-f]{40}%s$' % ( + re.escape( + 'https://s3.mock.amazonaws.com/unflakable-backend-mock-test-uploads/teams/' + f'{MOCK_TEAM_ID}/suites/{MOCK_SUITE_ID}/runs/upload/' + ), + re.escape('?X-Amz-Signature=MOCK_SIGNATURE') + ), + request.url + ) is not None + + +def __mock_upload_response( + request: requests_mock.request._RequestObjectProxy, + context: requests_mock.response._Context, +) -> bytes: + assert request.url in __uploads + assert __uploads[request.url] is None, 'duplicate upload' + __uploads[request.url] = json.loads(gzip.decompress(request.body)) + return b'' + + +def __mock_create_test_suite_run_response( + request: requests_mock.request._RequestObjectProxy, context: requests_mock.response._Context, ) -> _api.TestSuiteRunPendingSummary: - request_body: _api.CreateTestSuiteRunRequest = request.json() + request_body: _api.CreateTestSuiteRunUploadRequest = request.json() + upload_url = __upload_url(request_body['upload_id']) + upload = __uploads[upload_url] + assert upload is not None, 'missing upload' + return { 'run_id': MOCK_RUN_ID, 'suite_id': MOCK_SUITE_ID, - 'branch': request_body.get('branch'), - 'commit': request_body.get('commit'), + 'branch': upload.get('branch'), + 'commit': upload.get('commit'), } @@ -274,7 +334,7 @@ def run_test_case( ) -> None: api_key_path = pytester.makefile('', expected_api_key) if use_api_key_path else None requests_mocker.get( - url='https://app.unflakable.com/api/v1/test-suites/MOCK_SUITE_ID/manifest', + url=f'https://app.unflakable.com/api/v1/test-suites/{MOCK_SUITE_ID}/manifest', request_headers={'Authorization': f'Bearer {expected_api_key}'}, complete_qs=True, response_list=[ @@ -286,11 +346,11 @@ def run_test_case( }] if manifest is not None else []) ) + upload_id = __upload_id_for_current_test() requests_mocker.post( - url='https://app.unflakable.com/api/v1/test-suites/MOCK_SUITE_ID/runs', + url=f'https://app.unflakable.com/api/v1/test-suites/{MOCK_SUITE_ID}/runs/upload', request_headers={ - 'Authorization': f'Bearer {expected_api_key}', - 'Content-Type': 'application/json', + 'Authorization': f'Bearer {expected_api_key}' }, complete_qs=True, response_list=[ @@ -298,10 +358,38 @@ def run_test_case( for _ in range(failed_upload_requests) ] + [{ 'status_code': 201, - 'json': mock_create_test_suite_run_response, + 'json': lambda request, context: __mock_create_test_suite_run_upload_url_response( + upload_id, + request, + context, + ), }] ) + requests_mocker.put( + # The __match_upload() function matches the URL. + requests_mock.ANY, + request_headers={ + 'Content-Encoding': 'gzip', + 'Content-Type': 'application/json', + }, + complete_qs=True, + status_code=200, + additional_matcher=__match_upload, + content=__mock_upload_response, + ) + + requests_mocker.post( + url=f'https://app.unflakable.com/api/v1/test-suites/{MOCK_SUITE_ID}/runs', + request_headers={ + 'Authorization': f'Bearer {expected_api_key}', + 'Content-Type': 'application/json', + }, + complete_qs=True, + status_code=201, + json=__mock_create_test_suite_run_response, + ) + pytest_args: List[str] = ( (['--enable-unflakable'] if plugin_enabled else []) + (['--api-key-path', str(api_key_path)] if api_key_path is not None else []) + @@ -314,6 +402,7 @@ def run_test_case( ) + list(extra_args) ) + __pytest_current_test = os.environ['PYTEST_CURRENT_TEST'] if monkeypatch is not None: with monkeypatch.context() as mp: for key, val in (env_vars if env_vars is not None else {}).items(): @@ -323,6 +412,9 @@ def run_test_case( else: result = pytester.runpytest(*pytest_args) + # pytester clears PYTEST_CURRENT_TEST for some reason. + os.environ['PYTEST_CURRENT_TEST'] = __pytest_current_test + if verbose: test_outcomes_output = [ # Per-file test outcomes (one line for each test, color-coded). @@ -508,9 +600,10 @@ def run_test_case( request = requests_mocker.request_history[manifest_attempt] assert request.url == ( - 'https://app.unflakable.com/api/v1/test-suites/MOCK_SUITE_ID/manifest' + f'https://app.unflakable.com/api/v1/test-suites/{MOCK_SUITE_ID}/manifest' ) assert request.method == 'GET' + assert request.headers.get('Authorization', '') == f'Bearer {expected_api_key}' assert request.body is None if manifest_attempt > 0: @@ -527,26 +620,57 @@ def run_test_case( ) for upload_attempt in range(expected_upload_attempts): - create_test_suite_run_request = requests_mocker.request_history[ + create_upload_url_request = requests_mocker.request_history[ expected_get_test_suite_manifest_attempts + upload_attempt ] - assert create_test_suite_run_request.url == ( - 'https://app.unflakable.com/api/v1/test-suites/MOCK_SUITE_ID/runs') - assert create_test_suite_run_request.method == 'POST' + assert create_upload_url_request.url == ( + f'https://app.unflakable.com/api/v1/test-suites/{MOCK_SUITE_ID}/runs/upload') + assert create_upload_url_request.method == 'POST' + assert ( + create_upload_url_request.headers.get('Authorization') + == f'Bearer {expected_api_key}' + ) - create_test_suite_run_body: _api.CreateTestSuiteRunRequest = ( - create_test_suite_run_request.json() + # Failed attempts only include the initial request. + if upload_attempt < failed_upload_requests: + continue + + upload_request = requests_mocker.request_history[ + expected_get_test_suite_manifest_attempts + upload_attempt + 1 + ] + assert upload_request.url == __upload_url(upload_id) + assert upload_request.method == 'PUT' + assert upload_request.headers.get('Content-Encoding') == 'gzip' + assert upload_request.headers.get('Content-Type') == 'application/json' + upload_body: _api.CreateTestSuiteRunInlineRequest = ( + json.loads(gzip.decompress(upload_request.body)) ) if expected_commit is not None: - assert create_test_suite_run_body['commit'] == expected_commit + assert upload_body['commit'] == expected_commit else: - assert 'commit' not in create_test_suite_run_body + assert 'commit' not in upload_body if expected_branch is not None: - assert create_test_suite_run_body['branch'] == expected_branch + assert upload_body['branch'] == expected_branch else: - assert 'branch' not in create_test_suite_run_body + assert 'branch' not in upload_body + + create_test_suite_run_request = requests_mocker.request_history[ + expected_get_test_suite_manifest_attempts + upload_attempt + 2 + ] + assert create_test_suite_run_request.url == ( + f'https://app.unflakable.com/api/v1/test-suites/{MOCK_SUITE_ID}/runs') + assert create_test_suite_run_request.method == 'POST' + assert ( + create_test_suite_run_request.headers.get('Authorization') + == f'Bearer {expected_api_key}' + ) + assert create_test_suite_run_request.headers.get('Content-Type') == 'application/json' + create_test_suite_run_body: _api.CreateTestSuiteRunUploadRequest = ( + create_test_suite_run_request.json() + ) + assert create_test_suite_run_body['upload_id'] == upload_id if upload_attempt > 0: assert ( @@ -557,7 +681,8 @@ def run_test_case( ) assert requests_mocker.call_count == ( - expected_get_test_suite_manifest_attempts + expected_upload_attempts + expected_get_test_suite_manifest_attempts + + failed_upload_requests + 3 * (expected_upload_attempts - failed_upload_requests) ), 'Expected %d total API requests, but received %d' % ( expected_get_test_suite_manifest_attempts + expected_upload_attempts, requests_mocker.call_count, @@ -566,8 +691,6 @@ def run_test_case( # Checked expected User-Agent. We do this here instead of using an `additional_matcher` to # make errors easier to diagnose. for request in requests_mocker.request_history: - assert request.headers.get('Authorization', '') == f'Bearer {expected_api_key}' - assert_regex( r'^unflakable-pytest-plugin/.* \(PyTest .*; Python .*; Platform .*\)$', request.headers.get('User-Agent', '') diff --git a/tests/test_unflakable.py b/tests/test_unflakable.py index 6def7c7..f39f4bd 100644 --- a/tests/test_unflakable.py +++ b/tests/test_unflakable.py @@ -1949,6 +1949,9 @@ def test_pass2(): expect_progress=False, ) + # Prevent duplicate upload error. + os.environ['PYTEST_CURRENT_TEST'] += '-step2' + run_test_case( pytester, manifest, From be0feda997a3e2cf5098f7fa32acc2f7c63b0131 Mon Sep 17 00:00:00 2001 From: "David A. Ramos" Date: Sat, 11 Nov 2023 22:07:58 -0800 Subject: [PATCH 6/6] Bump version to 0.1.6 --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index 4016817..9c287a7 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,6 +1,6 @@ [metadata] name = pytest-unflakable -version = 0.1.5 +version = 0.1.6 author = Unflakable author_email = support@unflakable.com maintainer = Unflakable