diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index e570994b..edd1ddbe 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -13,7 +13,7 @@ jobs: look-for-outdated-code: runs-on: ubuntu-24.04 steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@v6 - name: Install dependencies run: | sudo apt update && sudo apt install python3-pip diff --git a/pyproject.toml b/pyproject.toml index 26e6e6c7..a3292799 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -31,7 +31,7 @@ markers = ["reconcile", "conflict", "migrate", "discourse"] # Formatting tools configuration [tool.black] line-length = 99 -target-version = ["py310"] +target-version = ["py312"] [tool.isort] line_length = 99 diff --git a/src/gatekeeper/__init__.py b/src/gatekeeper/__init__.py index 99c4a147..69f421a3 100644 --- a/src/gatekeeper/__init__.py +++ b/src/gatekeeper/__init__.py @@ -2,6 +2,7 @@ # See LICENSE file for licensing details. """Library for uploading docs to charmhub.""" + import logging from collections.abc import Iterable, Iterator from itertools import tee diff --git a/src/gatekeeper/constants.py b/src/gatekeeper/constants.py index 27eb6b4c..90f52dfb 100644 --- a/src/gatekeeper/constants.py +++ b/src/gatekeeper/constants.py @@ -6,6 +6,7 @@ The use of this module should be limited to cases where the constant is not better placed in another module or to resolve circular imports. """ + DEFAULT_BRANCH = "main" DOCUMENTATION_TAG = "discourse-gatekeeper/base-content" diff --git a/src/gatekeeper/docs_directory.py b/src/gatekeeper/docs_directory.py index 638d0c17..29ffbaaf 100644 --- a/src/gatekeeper/docs_directory.py +++ b/src/gatekeeper/docs_directory.py @@ -2,6 +2,7 @@ # See LICENSE file for licensing details. """Class for reading the docs directory.""" + import itertools import typing from functools import partial diff --git a/tests/conftest.py b/tests/conftest.py index 565c553a..e25f20d3 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -69,6 +69,11 @@ def fixture_git_repo( """Create repository with mocked upstream.""" repo = Repo.clone_from(url=upstream_git_repo.working_dir, to_path=repository_path) + # Disable GPG signing for tags in test environment + writer = repo.config_writer() + writer.set_value("tag", "gpgSign", "false") + writer.release() + repo.git.checkout("-b", default_branch) # Go into detached head mode to reflect how GitHub performs the checkout @@ -98,6 +103,8 @@ def fixture_upstream_git_repo(upstream_repository_path: Path, default_branch: st writer = upstream_repository.config_writer() writer.set_value("user", "name", "upstream_user") writer.set_value("user", "email", "upstream_email") + # Disable GPG signing for tags in test environment + writer.set_value("tag", "gpgSign", "false") writer.release() upstream_repository.git.checkout("-b", default_branch) @@ -106,6 +113,7 @@ def fixture_upstream_git_repo(upstream_repository_path: Path, default_branch: st upstream_repository.git.commit("-m", "'initial commit'") upstream_repository.git.checkout("-b", BASE_REMOTE_BRANCH) + # Create lightweight tag upstream_repository.git.tag(DOCUMENTATION_TAG) return upstream_repository diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index c12e5dc9..68a413aa 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -56,7 +56,9 @@ async def discourse(model: Model) -> Application: apps=[postgres_charm_name, redis_charm_name], status="active", raise_on_error=False ) - discourse_app: Application = await model.deploy(discourse_charm_name, channel="edge") + discourse_app: Application = await model.deploy( + discourse_charm_name, channel="edge", revision=229 + ) await model.wait_for_idle(apps=[discourse_charm_name], status="waiting", raise_on_error=False) await model.integrate(discourse_charm_name, f"{postgres_charm_name}:database") @@ -344,12 +346,12 @@ def discourse_enable_tags( headers = { "Api-Key": discourse_admin_api_credentials.key, "Api-Username": discourse_admin_api_credentials.username, + "Content-Type": "application/json", } - data = {"tagging_enabled": "true"} response = requests.put( - f"{discourse_address}/admin/site_settings/tagging_enabled", + f"{discourse_address}/admin/site_settings/tagging_enabled.json", headers=headers, - data=data, + json={"tagging_enabled": "true"}, timeout=60, ) assert response.status_code == 200, f"Enabling tagging failed, {response.content=}" @@ -360,6 +362,10 @@ async def discourse_remove_rate_limits( discourse_admin_api_headers: dict[str, str], discourse_address: str ): """Disables rate limits on discourse.""" + headers_with_content_type = { + **discourse_admin_api_headers, + "Content-Type": "application/json", + } settings = { "unique_posts_mins": "0", "rate_limit_create_post": "0", @@ -381,9 +387,9 @@ async def discourse_remove_rate_limits( } for setting, value in settings.items(): response = requests.put( - f"{discourse_address}/admin/site_settings/{setting}", - headers=discourse_admin_api_headers, - data={setting: value}, + f"{discourse_address}/admin/site_settings/{setting}.json", + headers=headers_with_content_type, + json={setting: value}, timeout=60, ) assert ( diff --git a/tests/integration/test___init__run_conflict.py b/tests/integration/test___init__run_conflict.py index d603a296..7591ca1a 100644 --- a/tests/integration/test___init__run_conflict.py +++ b/tests/integration/test___init__run_conflict.py @@ -105,7 +105,7 @@ async def test_run_conflict( urls_with_actions = reconcile_output.topics assert len(urls_with_actions) == 2 - (doc_url, _) = urls_with_actions.keys() + doc_url, _ = urls_with_actions.keys() assert (urls := tuple(urls_with_actions)) == (doc_url, index_url) doc_table_line_1 = f"| 1 | {doc_table_key} | [{doc_title}]({urlparse(doc_url).path}) |" assert_substrings_in_string( @@ -242,7 +242,7 @@ async def test_run_conflict( urls_with_actions = reconcile_output.topics assert len(urls_with_actions) == 3 - (alt_doc_url, _, _) = urls_with_actions.keys() + alt_doc_url, _, _ = urls_with_actions.keys() assert (urls := tuple(urls_with_actions)) == (alt_doc_url, doc_url, index_url) alt_doc_table_line_5 = ( f"| 1 | {alt_doc_table_key} | [{alt_doc_title}]({urlparse(alt_doc_url).path}) |" diff --git a/tests/integration/test___init__run_migrate.py b/tests/integration/test___init__run_migrate.py index acc7eb9b..30f71770 100644 --- a/tests/integration/test___init__run_migrate.py +++ b/tests/integration/test___init__run_migrate.py @@ -140,9 +140,7 @@ async def test_run_migrate( assert output_migrate.pull_request_url == mock_pull_request.html_url assert output_migrate.action == PullRequestAction.OPENED assert (upstream_doc_dir / "index.md").is_file() - assert ( - (upstream_doc_dir / "index.md").read_text(encoding="utf-8") - == f"""Testing index page. + assert (upstream_doc_dir / "index.md").read_text(encoding="utf-8") == f"""Testing index page. Testing index page content. @@ -160,7 +158,6 @@ async def test_run_migrate( 1. [Canonical 3](https://canonical.com/projects) 1. [{content_page_4.content}](group-3/content-4.md) 1. [Group 5](group-5)""" - ) assert (group_1_path := upstream_doc_dir / "group-1").is_dir() assert (group_1_path / migration.GITKEEP_FILENAME).is_file() assert (group_2_path := upstream_doc_dir / "group-2").is_dir() diff --git a/tests/integration/test___init__run_reconcile.py b/tests/integration/test___init__run_reconcile.py index e28ece4e..8f4125d1 100644 --- a/tests/integration/test___init__run_reconcile.py +++ b/tests/integration/test___init__run_reconcile.py @@ -179,7 +179,7 @@ async def test_run( assert output_reconcile is not None assert len(output_reconcile.topics) == 2 - (doc_url, _) = output_reconcile.topics.keys() + doc_url, _ = output_reconcile.topics.keys() assert (urls := tuple(output_reconcile.topics)) == (doc_url, index_url) doc_table_line_1 = f"| 1 | {doc_table_key} | [{doc_content_1}]({urlparse(doc_url).path}) |" assert_substrings_in_string( @@ -292,7 +292,7 @@ async def test_run( assert urls_with_actions is not None assert len(urls_with_actions) == 3 - (_, nested_dir_doc_url, _) = urls_with_actions.keys() + _, nested_dir_doc_url, _ = urls_with_actions.keys() assert (urls := tuple(urls_with_actions)) == (doc_url, nested_dir_doc_url, index_url) nested_dir_doc_table_line_1 = ( f"| 2 | {nested_dir_doc_table_key} |" @@ -560,7 +560,7 @@ async def test_run_hidden( urls_with_actions = output_reconcile.topics assert len(urls_with_actions) == 2 - (doc_url, index_url) = urls_with_actions.keys() + doc_url, index_url = urls_with_actions.keys() assert (urls := tuple(urls_with_actions)) == (doc_url, index_url) doc_table_line_1 = f"| 1 | {doc_table_key} | [{doc_title}]({urlparse(doc_url).path}) |" assert_substrings_in_string( @@ -673,7 +673,7 @@ async def test_run_hidden( urls_with_actions = output_reconcile.topics assert len(urls_with_actions) == 3 - (_, alt_doc_url, _) = urls_with_actions.keys() + _, alt_doc_url, _ = urls_with_actions.keys() assert (urls := tuple(urls_with_actions)) == (doc_url, alt_doc_url, index_url) alt_doc_table_line_4 = ( f"| | {alt_doc_table_key} | [{alt_doc_title}]({urlparse(alt_doc_url).path}) |" @@ -795,7 +795,7 @@ async def test_run_external( urls_with_actions = output_reconcile.topics assert len(urls_with_actions) == 2 - (external_url, index_url) = urls_with_actions.keys() + external_url, index_url = urls_with_actions.keys() assert (urls := tuple(urls_with_actions)) == (external_url, index_url) item_table_line_1 = f"| 1 | https-canonical-com | [{item_title_1}]({item_url_1}) |" assert_substrings_in_string( @@ -843,7 +843,7 @@ async def test_run_external( urls_with_actions = output_reconcile.topics assert len(urls_with_actions) == 2 - (external_url, index_url) = urls_with_actions.keys() + external_url, index_url = urls_with_actions.keys() assert (urls := tuple(urls_with_actions)) == (external_url, index_url) item_table_line_3 = f"| 1 | https-canonical-com | [{item_title_3}]({external_url}) |" assert_substrings_in_string( diff --git a/tests/unit/test___init__.py b/tests/unit/test___init__.py index 3845b346..8616b243 100644 --- a/tests/unit/test___init__.py +++ b/tests/unit/test___init__.py @@ -2,6 +2,7 @@ # See LICENSE file for licensing details. # pylint: disable=too-many-lines """Unit tests for execution.""" + import logging from pathlib import Path from unittest import mock @@ -427,7 +428,8 @@ def test__run_reconcile_invalid_external_item(mocked_clients): "gatekeeper.repository.Client.metadata", types_.Metadata(name="name 1", docs=None), ) -def test__run_reconcile_external_item(mocked_clients): +@mock.patch("requests.head") +def test__run_reconcile_external_item(mock_requests_head, mocked_clients): """ arrange: given metadata with name but not docs and docs folder with an external item on the index @@ -435,6 +437,11 @@ def test__run_reconcile_external_item(mocked_clients): assert: then a documentation page is created and an index page is created with a navigation page with the external item. """ + # Mock HTTP HEAD request to avoid network calls + mock_response = mock.MagicMock() + mock_response.status_code = 200 + mock_requests_head.return_value = mock_response + mocked_clients.discourse.create_topic.side_effect = [index_url := "url 1"] with mocked_clients.repository.with_branch(DEFAULT_BRANCH) as repo: diff --git a/tests/unit/test_check.py b/tests/unit/test_check.py index 73cb4062..52612992 100644 --- a/tests/unit/test_check.py +++ b/tests/unit/test_check.py @@ -5,8 +5,10 @@ import logging from typing import NamedTuple, cast +from unittest import mock import pytest +import requests from gatekeeper import check, types_ @@ -492,6 +494,7 @@ def test_external_refs( index_contents: tuple[types_.IndexContentsListItem, ...], expected_problems: tuple[ExpectedProblem], caplog: pytest.LogCaptureFixture, + monkeypatch: pytest.MonkeyPatch, ): """ arrange: given index_contents @@ -500,6 +503,38 @@ def test_external_refs( """ caplog.set_level(logging.INFO) + # Mock requests.head to avoid actual network calls + def mock_head(url: str, timeout: int = 60): # pylint: disable=unused-argument + """Mock HEAD request. + + Args: + url: The URL to check. + timeout: The timeout for the request. + + Returns: + A mock response object. + + Raises: + ConnectionError: If the URL simulates a connection error. + """ + response = mock.MagicMock(spec=requests.Response) + + # Simulate valid responses for canonical.com URLs + if "canonical.com" in url and "invalid" not in url: + response.status_code = 200 + # Simulate 404 for canonica.com (typo) or invalid paths + elif "canonica.com" in url or "invalid-page" in url: + response.status_code = 404 + # Simulate connection errors for invalid domains + elif "invalid.link.com" in url or "invalid.url.com" in url: + raise requests.ConnectionError(f"Connection error for {url}") + else: + response.status_code = 200 + + return response + + monkeypatch.setattr("requests.head", mock_head) + returned_problems = tuple(check.external_refs(index_contents=index_contents)) assert len(returned_problems) == len(expected_problems) diff --git a/tests/unit/test_discourse.py b/tests/unit/test_discourse.py index 0f651be4..8c389e06 100644 --- a/tests/unit/test_discourse.py +++ b/tests/unit/test_discourse.py @@ -595,22 +595,19 @@ def test_function_discourse_error( [ pytest.param("test content", "test content", id="version 2.6.0 response"), pytest.param( - textwrap.dedent( - """\ + textwrap.dedent("""\ test-username | timestamp | # 23 test content ------------------------- - """ - ), + """), "test content", id="version 2.8.14 response", ), pytest.param( - textwrap.dedent( - """\ + textwrap.dedent("""\ test-username | timestamp | # 23 test content @@ -623,8 +620,7 @@ def test_function_discourse_error( ------------------------- - """ - ), + """), "test content", id="version 2.8.14 response with post replies", ), diff --git a/tests/unit/test_index.py b/tests/unit/test_index.py index f251584e..e5119980 100644 --- a/tests/unit/test_index.py +++ b/tests/unit/test_index.py @@ -221,22 +221,16 @@ def _test_get_content_for_server_parameters(): ), # Can't use f-string due to needing new line pytest.param( - "# contents\n" - + ( - other_content := """# -line 1""" - ), + "# contents\n" + (other_content := """# +line 1"""), other_content, id="contents followed by header with single line", ), # Can't use f-string due to needing new line pytest.param( - "# contents\n" - + ( - other_content := """# + "# contents\n" + (other_content := """# line 1 -line 2""" - ), +line 2"""), other_content, id="contents followed by header with multiple lines", ), @@ -250,11 +244,8 @@ def _test_get_content_for_server_parameters(): ), # Can't use f-string due to needing new line pytest.param( - "# contents\n" - + ( - other_content := """# -# contents""" - ), + "# contents\n" + (other_content := """# +# contents"""), other_content, id="contents followed by header followed by another contents", ), diff --git a/tox.ini b/tox.ini index 204fc8e0..cd185038 100644 --- a/tox.ini +++ b/tox.ini @@ -12,7 +12,7 @@ tst_path = {toxinidir}/tests/ all_path = {[vars]src_path} {[vars]main_path} {[vars]tst_path} [testenv] -basepython = python3.10 +basepython = python3.12 allowlist_externals=python setenv = PYTHONPATH = {toxinidir}:{toxinidir}/lib:{[vars]src_path} @@ -35,14 +35,13 @@ description = Check code against coding style standards deps = -r{toxinidir}/requirements.txt black - flake8<6.0.0 + flake8 flake8-docstrings>=1.6 flake8-copyright>=0.2 flake8-builtins>=2.0 flake8-docstrings-complete>=1.0.3 flake8-test-docs>=1.0 - ; There is an error with version 6.0.0 related to integers and arguments - pyproject-flake8<6.0.0 + pyproject-flake8 pep8-naming isort codespell