diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index cdbe6d3be..f425ecae2 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -131,15 +131,20 @@ jobs: - name: Run Tests run: make stop ${{ matrix.test-case }} - # disable due to confusing and unhelpful messages created by codecov - # (see )https://github.com/codecov/feedback/issues/304#issuecomment-2492675117) - # - name: Upload test results to Codecov - # if: ${{ !cancelled() && success() && matrix.test-case == 'test-coverage-only' }} - # uses: codecov/test-results-action@v1 - # with: - # files: reports/coverage-junit.xml,!./cache - # flags: ${{ matrix.python-version }} - # token: ${{ secrets.CODECOV_TOKEN }} + # manually invoke reporting in case of test failure to still generate them + # otherwise, they would have been generated automatically following the successful coverage run + - name: Handle Failed Coverage Report + if: ${{ failure() && matrix.test-case == 'test-coverage-only' }} + run: make coverage-reports + # flaky test analysis, which includes failed tests if applicable + - name: Upload test results to Codecov + if: ${{ !cancelled() && matrix.test-case == 'test-coverage-only' }} + uses: codecov/test-results-action@v1 + with: + files: reports/coverage-junit.xml,!./cache + flags: ${{ matrix.python-version }} + token: ${{ secrets.CODECOV_TOKEN }} + # coverage test analysis - name: Upload coverage report uses: codecov/codecov-action@v2 if: ${{ success() && matrix.test-case == 'test-coverage-only' }} diff --git a/CHANGES.rst b/CHANGES.rst index 18710d029..dd76df4d8 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -16,7 +16,8 @@ Changes: Fixes: ------ -- No change. +- Fix ``weaver.cli.RequestAuthHandler`` and its derived classes erroneously invoking ``request_auth`` method when + both the ``url`` and ``token`` are omitted, leading to invalid ``requests`` call under ``weaver.utils.request_extra``. .. _changes_6.1.1: diff --git a/Makefile b/Makefile index 6a29bfdca..e7ee8cbf1 100644 --- a/Makefile +++ b/Makefile @@ -381,6 +381,7 @@ ifeq ($(filter $(TEST_VERBOSITY),"--capture"),) TEST_VERBOSITY := $(TEST_VERBOSITY) --capture tee-sys endif endif +TEST_XARGS ?= # autogen tests variants with pre-install of dependencies using the '-only' target references TESTS := unit func cli workflow online offline no-tb14 spec coverage @@ -397,56 +398,56 @@ test-all: install-dev test-only ## run all tests (including long running tests) .PHONY: test-only test-only: mkdir-reports ## run all tests but without prior validation of installed dependencies @echo "Running all tests (including slow and online tests)..." - @bash -c '$(CONDA_CMD) pytest tests $(TEST_VERBOSITY) \ + @bash -c '$(CONDA_CMD) pytest tests $(TEST_VERBOSITY) $(TEST_XARGS) \ --junitxml "$(REPORTS_DIR)/test-results.xml"' .PHONY: test-unit-only test-unit-only: mkdir-reports ## run unit tests (skip long running and online tests) @echo "Running unit tests (skip slow and online tests)..." - @bash -c '$(CONDA_CMD) pytest tests $(TEST_VERBOSITY) \ + @bash -c '$(CONDA_CMD) pytest tests $(TEST_VERBOSITY) $(TEST_XARGS) \ -m "not slow and not online and not functional" --junitxml "$(REPORTS_DIR)/test-results.xml"' .PHONY: test-func-only test-func-only: mkdir-reports ## run functional tests (online and usage specific) @echo "Running functional tests..." - @bash -c '$(CONDA_CMD) pytest tests $(TEST_VERBOSITY) \ + @bash -c '$(CONDA_CMD) pytest tests $(TEST_VERBOSITY) $(TEST_XARGS) \ -m "functional" --junitxml "$(REPORTS_DIR)/test-results.xml"' .PHONY: test-cli-only test-cli-only: mkdir-reports ## run WeaverClient and CLI tests @echo "Running CLI tests..." - @bash -c '$(CONDA_CMD) pytest tests $(TEST_VERBOSITY) \ + @bash -c '$(CONDA_CMD) pytest tests $(TEST_VERBOSITY) $(TEST_XARGS) \ -m "cli" --junitxml "$(REPORTS_DIR)/test-results.xml"' .PHONY: test-workflow-only test-workflow-only: mkdir-reports ## run EMS workflow End-2-End tests @echo "Running workflow tests..." - @bash -c '$(CONDA_CMD) pytest tests $(TEST_VERBOSITY) \ + @bash -c '$(CONDA_CMD) pytest tests $(TEST_VERBOSITY) $(TEST_XARGS) \ -m "workflow" --junitxml "$(REPORTS_DIR)/test-results.xml"' .PHONY: test-online-only test-online-only: mkdir-reports ## run online tests (running instance required) @echo "Running online tests (running instance required)..." - @bash -c '$(CONDA_CMD) pytest tests $(TEST_VERBOSITY) \ + @bash -c '$(CONDA_CMD) pytest tests $(TEST_VERBOSITY) $(TEST_XARGS) \ -m "online" --junitxml "$(REPORTS_DIR)/test-results.xml"' .PHONY: test-offline-only test-offline-only: mkdir-reports ## run offline tests (not marked as online) @echo "Running offline tests (not marked as online)..." - @bash -c '$(CONDA_CMD) pytest tests $(TEST_VERBOSITY) \ + @bash -c '$(CONDA_CMD) pytest tests $(TEST_VERBOSITY) $(TEST_XARGS) \ -m "not online" --junitxml "$(REPORTS_DIR)/test-results.xml"' .PHONY: test-no-tb14-only test-no-tb14-only: mkdir-reports ## run all tests except ones marked for 'Testbed-14' @echo "Running all tests except ones marked for 'Testbed-14'..." - @bash -c '$(CONDA_CMD) pytest tests $(TEST_VERBOSITY) \ + @bash -c '$(CONDA_CMD) pytest tests $(TEST_VERBOSITY) $(TEST_XARGS) \ -m "not testbed14" --junitxml "$(REPORTS_DIR)/test-results.xml"' .PHONY: test-spec-only test-spec-only: mkdir-reports ## run tests with custom specification (pytest format) [make SPEC='' test-spec] @echo "Running custom tests from input specification..." @[ "${SPEC}" ] || ( echo ">> 'SPEC' is not set"; exit 1 ) - @bash -c '$(CONDA_CMD) pytest tests $(TEST_VERBOSITY) \ + @bash -c '$(CONDA_CMD) pytest tests $(TEST_VERBOSITY) $(TEST_XARGS) \ -k "${SPEC}" --junitxml "$(REPORTS_DIR)/test-results.xml"' .PHONY: test-smoke @@ -455,11 +456,22 @@ test-smoke: docker-test ## alias to 'docker-test' executing smoke test of bu .PHONY: test-docker test-docker: docker-test ## alias to 'docker-test' execution smoke test of built docker images +# NOTE: +# if any test fails during coverage run, pytest exit code will be propagated to allow reporting of the failure +# this will cause coverage analysis reporting to be skipped from early exit from the failure +# if coverage reporting is still needed although failed tests occurred, call 'coverage-reports' target separately .PHONY: test-coverage-only -test-coverage-only: mkdir-reports ## run all tests using coverage analysis +test-coverage-only: mkdir-reports coverage-run coverage-reports ## run all tests with coverage analysis and reports + +.PHONY: coverage-run +coverage-run: mkdir-reports ## run all tests using coverage analysis @echo "Running coverage analysis..." @bash -c '$(CONDA_CMD) coverage run --rcfile="$(APP_ROOT)/setup.cfg" \ - "$$(which pytest)" "$(APP_ROOT)/tests" --junitxml="$(REPORTS_DIR)/coverage-junit.xml" || true' + "$$(which pytest)" "$(APP_ROOT)/tests" $(TEST_XARGS) --junitxml="$(REPORTS_DIR)/coverage-junit.xml"' + +.PHONY: coverage-reports +coverage-reports: mkdir-reports ## generate coverage reports + @echo "Generate coverage reports..." @bash -c '$(CONDA_CMD) coverage xml --rcfile="$(APP_ROOT)/setup.cfg" -i -o "$(REPORTS_DIR)/coverage.xml"' @bash -c '$(CONDA_CMD) coverage report --rcfile="$(APP_ROOT)/setup.cfg" -i -m' @bash -c '$(CONDA_CMD) coverage html --rcfile="$(APP_ROOT)/setup.cfg" -d "$(REPORTS_DIR)/coverage"' diff --git a/docs/source/conf.py b/docs/source/conf.py index 361181356..ad7fa058d 100644 --- a/docs/source/conf.py +++ b/docs/source/conf.py @@ -460,6 +460,7 @@ def doc_redirect_include(file_path): "data-types", # https://spec.openapis.org/oas/v3.1.0 "defusedxmllxml", # https://github.com/tiran/defusedxml/tree/main "ncml-to-stac", # https://github.com/crim-ca/ncml2stac/tree/main#ncml-to-stac + "issuecomment-[0-9]+", # links to specific GitHub comments ] linkcheck_request_headers = { "https://github.com/": { diff --git a/requirements-dev.txt b/requirements-dev.txt index 5125204be..778515da3 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -4,7 +4,10 @@ # pylint>=2.5 requires astroid>=2.4 # install/update sometime fails randomly, so enforce it astroid -bandit +# pin bandit, many issues in latest versions +# - https://github.com/PyCQA/bandit/issues/1226 +# - https://github.com/PyCQA/bandit/issues/1227 +bandit<1.8 bump2version codacy-coverage coverage @@ -21,11 +24,11 @@ mypy parameterized path!=16.12.0,!=17.0.0 # patch pytest-shutil (https://github.com/man-group/pytest-plugins/issues/224) pluggy>=0.7 -# FIXME: bad interpolation of 'setup.cfg' for pytest 'log_format' (https://github.com/pytest-dev/pytest/issues/10019) -pytest<7 +pytest pytest-httpserver>=1.0.7 # support werkzeug>=3 pytest-server-fixtures -pytest-rerunfailures +#pytest-rerunfailures +pytest-retry pydocstyle # FIXME: pylint-quotes failing with pylint==3 (https://github.com/edaniszewski/pylint-quotes/issues/29) # FIXME: use temporary unofficial version working with pylint>3 (https://github.com/edaniszewski/pylint-quotes/pull/30) diff --git a/tests/functional/test_cli.py b/tests/functional/test_cli.py index 74b2abde4..8c58e1616 100644 --- a/tests/functional/test_cli.py +++ b/tests/functional/test_cli.py @@ -15,6 +15,7 @@ import mock import pytest +import responses import yaml from owslib.ows import DEFAULT_OWS_NAMESPACE from owslib.wps import WPSException @@ -562,7 +563,7 @@ def test_execute_inputs_invalid(self): ]: self.run_execute_inputs_schema_variant(invalid_inputs_schema, expect_success=False) - @pytest.mark.flaky(reruns=2, reruns_delay=1) + @pytest.mark.flaky(retries=2, delay=1) def test_execute_manual_monitor_status_and_download_results(self): """ Test a typical case of :term:`Job` execution, result retrieval and download, but with manual monitoring. @@ -1097,7 +1098,7 @@ def add_docker_pull_ref(cwl, ref): cwl["requirements"][CWL_REQUIREMENT_APP_DOCKER] = {"dockerPull": ref} return cwl - @pytest.mark.flaky(reruns=2, reruns_delay=1) + @pytest.mark.flaky(retries=2, delay=1) def test_deploy_docker_auth_username_password_valid(self): """ Test that username and password arguments can be provided simultaneously for docker login. @@ -1869,6 +1870,7 @@ def test_execute_subscriber_options(self): """ proc = self.test_process["Echo"] with contextlib.ExitStack() as stack_exec: + req_mock = stack_exec.enter_context(responses.RequestsMock()) for mock_exec_proc in mocked_execute_celery(): stack_exec.enter_context(mock_exec_proc) @@ -1876,6 +1878,10 @@ def test_execute_subscriber_options(self): test_email_failed = "failed-job@email.com" test_callback_started = "https://server.com/started" test_callback_success = "https://server.com/success" + + req_mock.add_callback(responses.POST, test_callback_started, callback=lambda _: (200, {}, "")) + req_mock.add_callback(responses.POST, test_callback_success, callback=lambda _: (200, {}, "")) + lines = mocked_sub_requests( self.app, run_command, [ diff --git a/tests/functional/test_wps_package.py b/tests/functional/test_wps_package.py index 1ccd21505..0e9022e45 100644 --- a/tests/functional/test_wps_package.py +++ b/tests/functional/test_wps_package.py @@ -2671,7 +2671,7 @@ def test_execute_with_browsable_directory(self): assert all(file.startswith(cwl_stage_dir) for file in output_listing) assert all(any(file.endswith(dir_file) for file in output_listing) for dir_file in expect_http_files) - @pytest.mark.flaky(reruns=2, reruns_delay=1) + @pytest.mark.flaky(retries=2, delay=1) def test_execute_with_json_listing_directory(self): """ Test that HTTP returning JSON list of directory contents retrieves children files for the process. diff --git a/tests/processes/test_wps_package.py b/tests/processes/test_wps_package.py index b6fff68a0..b59bddc22 100644 --- a/tests/processes/test_wps_package.py +++ b/tests/processes/test_wps_package.py @@ -162,7 +162,7 @@ def __init__(self, shell_command, arguments=None, with_message_input=True): super(MockProcess, self).__init__(body) -@pytest.mark.flaky(reruns=2, reruns_delay=1) +@pytest.mark.flaky(retries=2, delay=1) def test_stdout_stderr_logging_for_commandline_tool_success(caplog): """ Execute a process and assert that stdout is correctly logged to log file upon successful process execution. diff --git a/tests/test_cli.py b/tests/test_cli.py index c6f8464a9..411f9bca3 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -27,6 +27,7 @@ WeaverClient, main as weaver_cli ) +from weaver.exceptions import AuthenticationError from weaver.formats import ContentEncoding, ContentType @@ -408,7 +409,7 @@ def test_auth_handler_basic(): def test_auth_handler_bearer(): req = WebTestRequest({}) - auth = BearerAuthHandler(identity=str(uuid.uuid4())) + auth = BearerAuthHandler(identity=str(uuid.uuid4()), url="https://example.com") token = str(uuid.uuid4()) with mock.patch( "requests.Session.request", @@ -435,7 +436,7 @@ def test_auth_handler_bearer_explicit_token_matches_request_token(): req_request_token = WebTestRequest({}) token = str(uuid.uuid4()) auth_explicit_token = BearerAuthHandler(token=token) - auth_request_token = BearerAuthHandler(identity=str(uuid.uuid4())) + auth_request_token = BearerAuthHandler(identity=str(uuid.uuid4()), url="https://example.com") with mock.patch( "requests.Session.request", side_effect=lambda *_, **__: mocked_auth_response("access_token", token) @@ -450,7 +451,7 @@ def test_auth_handler_bearer_explicit_token_matches_request_token(): def test_auth_handler_cookie(): req = WebTestRequest({}) - auth = CookieAuthHandler(identity=str(uuid.uuid4())) + auth = CookieAuthHandler(identity=str(uuid.uuid4()), url="https://example.com") token = str(uuid.uuid4()) with mock.patch( "requests.Session.request", @@ -506,7 +507,7 @@ def test_auth_handler_cookie_explicit_token_matches_request_token(): req_request_token = WebTestRequest({}) token = str(uuid.uuid4()) auth_explicit_token = CookieAuthHandler(token=token) - auth_request_token = CookieAuthHandler(identity=str(uuid.uuid4())) + auth_request_token = CookieAuthHandler(identity=str(uuid.uuid4()), url="https://example.com") with mock.patch( "requests.Session.request", side_effect=lambda *_, **__: mocked_auth_response("access_token", token) @@ -519,6 +520,30 @@ def test_auth_handler_cookie_explicit_token_matches_request_token(): assert resp_explicit_token.headers["Cookie"] == resp_request_token.headers["Cookie"] +def test_auth_request_handler_no_url_or_token_init(): + with pytest.raises(AuthenticationError): + BearerAuthHandler(identity=str(uuid.uuid4())) + + try: + BearerAuthHandler(token=str(uuid.uuid4())) # OK + BearerAuthHandler(url="https://example.com") # OK + except Exception as exc: + pytest.fail(msg=f"Expected no init error from valid combinations. Got [{exc}]") + + +def test_auth_request_handler_no_url_ignored_request(): + req = WebTestRequest({}) + auth = BearerAuthHandler( + identity=str(uuid.uuid4()), + url="https://example.com", # URL must be passed to avoid error + ) + auth.url = None # reset after init check + with mock.patch("requests.Session.request") as mock_request: + resp = auth(req) # type: ignore + mock_request.assert_not_called() + assert not resp.headers, "No headers should have been added since URL could not be resolved." + + def test_upload_file_not_found(): with tempfile.NamedTemporaryFile() as tmp_file_deleted: pass # delete on close diff --git a/tests/test_utils.py b/tests/test_utils.py index fac191bb5..df268c615 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -655,7 +655,7 @@ def mock_sleep(delay): assert all(called == expect for called, expect in zip(sleep_counter["called_with"], intervals)) -@pytest.mark.flaky(reruns=2, reruns_delay=1) +@pytest.mark.flaky(retries=2, delay=1) def test_request_extra_zero_values(): """ Test that zero-value ``retries`` and ``backoff`` are not ignored. diff --git a/tests/wps_restapi/test_status_codes.py b/tests/wps_restapi/test_status_codes.py index 7c9a8c366..32044c4b9 100644 --- a/tests/wps_restapi/test_status_codes.py +++ b/tests/wps_restapi/test_status_codes.py @@ -2,6 +2,7 @@ import uuid import pytest +from parameterized import parameterized from tests.utils import get_test_weaver_app, setup_config_with_mongodb from weaver.formats import ContentType @@ -45,23 +46,24 @@ class StatusCodeTestCase(unittest.TestCase): headers = {"Accept": ContentType.APP_JSON} - def setUp(self): + @classmethod + def setUpClass(cls): config = setup_config_with_mongodb() - self.testapp = get_test_weaver_app(config) + cls.testapp = get_test_weaver_app(config) - def test_200(self): - for uri in TEST_PUBLIC_ROUTES: - resp = self.testapp.get(uri, expect_errors=True, headers=self.headers) - self.assertEqual(200, resp.status_code, f"route {uri} did not return 200") + @parameterized.expand(TEST_PUBLIC_ROUTES) + def test_200(self, uri): + resp = self.testapp.get(uri, expect_errors=True, headers=self.headers) + self.assertEqual(200, resp.status_code, f"route {uri} did not return 200") @pytest.mark.xfail(reason="Not working if not behind proxy. Protected implementation to be done.") + @parameterized.expand(TEST_FORBIDDEN_ROUTES) @unittest.expectedFailure - def test_401(self): - for uri in TEST_FORBIDDEN_ROUTES: - resp = self.testapp.get(uri, expect_errors=True, headers=self.headers) - self.assertEqual(401, resp.status_code, f"route {uri} did not return 401") + def test_401(self, uri): + resp = self.testapp.get(uri, expect_errors=True, headers=self.headers) + self.assertEqual(401, resp.status_code, f"route {uri} did not return 401") - def test_404(self): - for uri in TEST_NOTFOUND_ROUTES: - resp = self.testapp.get(uri, expect_errors=True, headers=self.headers) - self.assertEqual(404, resp.status_code, f"route {uri} did not return 404") + @parameterized.expand(TEST_NOTFOUND_ROUTES) + def test_404(self, uri): + resp = self.testapp.get(uri, expect_errors=True, headers=self.headers) + self.assertEqual(404, resp.status_code, f"route {uri} did not return 404") diff --git a/weaver/cli.py b/weaver/cli.py index dbfcec97f..7ad33715a 100644 --- a/weaver/cli.py +++ b/weaver/cli.py @@ -23,7 +23,7 @@ from weaver import __meta__ from weaver.datatype import AutoBase -from weaver.exceptions import PackageRegistrationError +from weaver.exceptions import AuthenticationError, PackageRegistrationError from weaver.execute import ExecuteMode, ExecuteResponse, ExecuteReturnPreference, ExecuteTransmissionMode from weaver.formats import ContentEncoding, ContentType, OutputFormat, get_content_type, get_format, repr_json from weaver.processes.constants import ProcessSchema @@ -278,6 +278,9 @@ def __init__( HTTPBasicAuth.__init__(self, username=identity, password=password) self.token = token + if not self.token and not self.url: + raise AuthenticationError("Either the token or the URL to retrieve it must be provided to the handler.") + @property def auth_token_name(self): # type: () -> str @@ -334,12 +337,12 @@ def response_parser(self, response): def __call__(self, request): # type: (AnyRequestType) -> AnyRequestType - if self.token is None: + if self.token is None and self.url: auth_token = self.request_auth() else: auth_token = self.token if not auth_token: - LOGGER.warning("Expected authorization token could not be retrieved from: [%s] in [%s]", + LOGGER.warning("Expected authorization token could not be retrieved from URL: [%s] in [%s]", self.url, fully_qualified_name(self)) else: auth_token = self.parse_token(auth_token) diff --git a/weaver/exceptions.py b/weaver/exceptions.py index 9140e1c9c..6e683ad41 100644 --- a/weaver/exceptions.py +++ b/weaver/exceptions.py @@ -55,6 +55,12 @@ class WeaverExecutionError(WeaverException): """ +class AuthenticationError(WeaverException): + """ + Generic exception related to an issue about authentication configuration or resolution. + """ + + class ListingInvalidParameter(WeaverException, OWSInvalidParameterValue, ValueError): """ Error related to an invalid parameter for listing queries. @@ -265,7 +271,7 @@ class PackageRegistrationError(HTTPInternalServerError, OWSNoApplicableCode, Pac """ -class PackageAuthenticationError(HTTPForbidden, OWSAccessForbidden, PackageException): +class PackageAuthenticationError(HTTPForbidden, OWSAccessForbidden, PackageException, AuthenticationError): """ Error related to a runtime failure caused by failing authentication prerequisite.