From 046a6c0e380abf720cd19082fa3fa6ad3adf9de9 Mon Sep 17 00:00:00 2001 From: Yanks Yoon <37652070+yanksyoon@users.noreply.github.com> Date: Fri, 24 Mar 2023 10:25:32 +0800 Subject: [PATCH] fix: integration test (#97) * fix: create admin api key * fix: revert to discourse from charmhub * fix: typo * fix: user & api key creation flow * fix: upgrade discourse charm * fix: remove unlist parameter(user level api failure) * fix: update assert to pass returned content * fix: set hostname for successful redirect * fix: static test dependency update to bandit w/ toml * fix: lint fixes * test: wait for idle for longer * test: give enough timeout for asset compile * fix: deploy and wait for idle before getting status * debug * fix: parse raw content * fix: lint fixes * fix: wait for idle not active status * fix: content in paragraph tag * fix: backwards compatibility fix for e2e * fix: infinte wait fix * fix: set config after relations * fix: reorder deploy flow * fix: wait for ip * fix: revert * chore: revert back to main operator workflow * chore: pin juju version & move to dev-requirements.txt * chore: remove pre_run * chore: add timeouts to requests * test: separate out response parsing * chore: revert content assertions * fix: doc imperative mood fix * test: fixturize admin API headers * test: consistency fixes * chore: remove unnecessary fstring * fix: fixture reference --- .github/workflows/integration_test.yaml | 1 - dev-requirements.txt | 1 + src/discourse.py | 24 +- src/metadata.py | 4 +- src/migration.py | 2 +- tests/integration/conftest.py | 377 +++++++++++------- tests/integration/pre_run.sh | 3 - tests/integration/test___init__run_migrate.py | 4 +- tests/integration/test_discourse.py | 25 +- tests/integration/types.py | 12 + tests/unit/helpers.py | 14 + tests/unit/test_discourse.py | 54 ++- tests/unit/test_migration.py | 6 +- tox.ini | 4 +- 14 files changed, 353 insertions(+), 178 deletions(-) delete mode 100755 tests/integration/pre_run.sh diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index 77132b57..5ccafd56 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -8,5 +8,4 @@ jobs: uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main secrets: inherit with: - pre-run-script: tests/integration/pre_run.sh modules: '["discourse", "reconcile", "migrate"]' diff --git a/dev-requirements.txt b/dev-requirements.txt index 7f4f9cef..a85126c0 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -1 +1,2 @@ factory_boy>=3.2,<3.3 +juju==3.0.4 diff --git a/src/discourse.py b/src/discourse.py index 2f69d19e..6baa4a52 100644 --- a/src/discourse.py +++ b/src/discourse.py @@ -15,6 +15,7 @@ from .exceptions import DiscourseError, InputError _URL_PATH_PREFIX = "/t/" +_POST_SPLIT_LINE = "\n\n-------------------------\n\n" class _DiscourseTopicInfo(typing.NamedTuple): @@ -345,6 +346,25 @@ def _get_requests_session() -> requests.Session: # pragma: no cover session.mount("https://", adapter) return session + @staticmethod + def _parse_raw_content(content: str) -> str: + """Parse raw topic content returned from discourse /raw/{topic_id} API endpoint. + + Args: + content: Raw content returned by discourse API. + + Returns: + Original topic content. + """ + # Discourse version 2.6.0, the content of a topc is returned as raw string. + if not content.endswith(_POST_SPLIT_LINE): + return content + + # Discourse version 2.8.14, the posts are split by _POST_SPLIT_LINE. + posts = content.split(_POST_SPLIT_LINE) + post_metadata_removed = posts[0].splitlines(keepends=True)[2:] + return "".join(post_metadata_removed) + def retrieve_topic(self, url: str) -> str: """Retrieve the topic content. @@ -379,7 +399,8 @@ def retrieve_topic(self, url: str) -> str: ) as exc: raise DiscourseError(f"Error retrieving the topic, {url=!r}") from exc - return response.content.decode("utf-8") + content = response.content.decode("utf-8") + return self._parse_raw_content(content) def create_topic(self, title: str, content: str) -> str: """Create a new topic. @@ -401,7 +422,6 @@ def create_topic(self, title: str, content: str) -> str: category_id=self._category_id, tags=self._tags, content=content, - unlist_topic=True, ) except pydiscourse.exceptions.DiscourseError as discourse_error: raise DiscourseError( diff --git a/src/metadata.py b/src/metadata.py index 414c786d..8ae4ed54 100644 --- a/src/metadata.py +++ b/src/metadata.py @@ -56,7 +56,9 @@ def get(path: Path) -> types_.Metadata: raise InputError(f"Invalid value for name key: {name}, expected a string value") docs = metadata.get(METADATA_DOCS_KEY) - if not isinstance(docs, str | None) or (METADATA_DOCS_KEY in metadata and docs is None): + if not (isinstance(docs, str) or docs is None) or ( + METADATA_DOCS_KEY in metadata and docs is None + ): raise InputError(f"Invalid value for docs key: {docs}, expected a string value") return types_.Metadata(name=name, docs=docs) diff --git a/src/migration.py b/src/migration.py index c8dfc174..3c3ff17b 100644 --- a/src/migration.py +++ b/src/migration.py @@ -133,7 +133,7 @@ def _create_document_meta(row: types_.TableRow, path: Path) -> types_.DocumentMe path: Relative path to where the document should reside. Raises: - MigrationError: if the table row that was passed in does not cantain a link to document. + MigrationError: if the table row that was passed in does not contain a link to document. Returns: Information required to migrate document. diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 9e886bed..7e8968a1 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -6,14 +6,18 @@ # pylint: disable=redefined-outer-name import asyncio -import re import secrets +import typing import pydiscourse import pytest import pytest_asyncio import requests -from ops.model import ActiveStatus, Application +from juju.action import Action +from juju.application import Application +from juju.client._definitions import ApplicationStatus, FullStatus, UnitStatus +from juju.model import Model +from juju.unit import Unit from pytest_operator.plugin import OpsTest from src.discourse import Discourse @@ -21,218 +25,290 @@ from . import types +@pytest.fixture(scope="module", name="model") +def model_fixture(ops_test: OpsTest) -> Model: + """Get current valid model created for integraion testing with module scope.""" + assert ops_test.model + return ops_test.model + + +@pytest_asyncio.fixture(scope="module") +async def discourse(model: Model) -> Application: + """Deploy discourse.""" + postgres_charm_name = "postgresql-k8s" + redis_charm_name = "redis-k8s" + discourse_charm_name = "discourse-k8s" + await asyncio.gather( + model.deploy(postgres_charm_name), + model.deploy(redis_charm_name), + ) + await model.wait_for_idle(apps=[postgres_charm_name, redis_charm_name], raise_on_error=False) + + discourse_app: Application = await model.deploy(discourse_charm_name, channel="edge") + await model.relate(discourse_charm_name, f"{postgres_charm_name}:db") + await model.relate(discourse_charm_name, redis_charm_name) + + # Need to wait for the waiting status to be resolved + + async def get_discourse_status(): + """Get the status of discourse. + + Returns: + The status of discourse. + """ + return (await model.get_status())["applications"]["discourse-k8s"].status["status"] + + for _ in range(120): + if await get_discourse_status() != "waiting": + break + await asyncio.sleep(10) + assert await get_discourse_status() != "waiting", "discourse never stopped waiting" + + status: FullStatus = await model.get_status() + app_status = typing.cast(ApplicationStatus, status.applications[discourse_app.name]) + unit_status = typing.cast(UnitStatus, app_status.units[f"{discourse_app.name}/0"]) + unit_ip = typing.cast(str, unit_status.address) + # the redirects will be towards default external_hostname value of application name which + # the client cannot reach. Hence we need to override it with accessible address. + await discourse_app.set_config({"external_hostname": f"{unit_ip}:3000"}) + + await model.wait_for_idle() + + return discourse_app + + @pytest.fixture(scope="module") -def discourse_hostname(): - """Get the hostname for discourse.""" - return "discourse" +def discourse_unit_name(discourse: Application): + """Get the discourse charm's unit name.""" + return f"{discourse.name}/0" -async def create_discourse_account( - ops_test: OpsTest, unit: str, email: str, admin: bool -) -> types.Credentials: - """ - Create an account on discourse. +@pytest_asyncio.fixture(scope="module") +async def discourse_address(model: Model, discourse: Application, discourse_unit_name: str): + """Get discourse web address.""" + status: FullStatus = await model.get_status() + app_status = typing.cast(ApplicationStatus, status.applications[discourse.name]) + unit_status = typing.cast(UnitStatus, app_status.units[discourse_unit_name]) + unit_ip = typing.cast(str, unit_status.address) + return f"http://{unit_ip}:3000" + - Assume that the model already contains a discourse unit. +async def create_discourse_admin_account(discourse: Application, email: str): + """Create an admin account on discourse. Args: - ops_test: Used for interactions with the model. - unit: The unit running discourse. - email: The email address to use on the account. - admin: Whether to make the account an admin account. + discourse: The Discourse charm application. + email: The email address to use to create the account. Returns: - The credentials for the discourse account. - + The credentials of the admin user. """ password = secrets.token_urlsafe(16) - # For some reason discourse.units[0] is None so can't use the discourse.units[0].ssh function - return_code, stdout, stderr = await ops_test.juju( - "exec", - "--unit", - unit, - "--", - "cd /srv/discourse/app && ./bin/bundle exec rake admin:create RAILS_ENV=production << " - f"ANSWERS\n{email}\n{password}\n{password}\n{'Y' if admin else 'n'}\nANSWERS", + discourse_unit: Unit = discourse.units[0] + action: Action = await discourse_unit.run_action( + "add-admin-user", email=email, password=password ) - assert ( - return_code == 0 - ), f"discourse account creation failed for email {email}, {stdout=}, {stderr=}" + await action.wait() + return types.Credentials(email=email, username=email.split("@")[0], password=password) + - username_match = re.search(r"Account created successfully with username (.*?)\n", stdout) - assert ( - username_match is not None - ), f"failed to get username for account with email {email}, {stdout=}, {stderr=}" - username = username_match.group(1) - assert isinstance(username, str) +async def create_discourse_admin_api_key( + discourse_address: str, admin_credentials: types.Credentials +) -> types.APICredentials: + """Create a discourse admin API key for admin user account. + + Args: + discourse_address: The discourse web address. + admin_credentials: The discourse admin user credentials. + + Returns: + The admin API credentials. + """ + with requests.session() as sess: + # Get CSRF token + res = sess.get( + f"{discourse_address}/session/csrf", headers={"Accept": "application/json"}, timeout=60 + ) + csrf = res.json()["csrf"] + # Create session & login + res = sess.post( + f"{discourse_address}/session", + headers={ + "Content-Type": "application/x-www-form-urlencoded; charset=UTF-8", + "X-CSRF-Token": csrf, + "X-Requested-With": "XMLHttpRequest", + }, + data={ + "login": admin_credentials.email, + "password": admin_credentials.password, + "second_factor_method": "1", + "timezone": "UTC", + }, + timeout=60, + ) + # Create global key + res = sess.post( + f"{discourse_address}/admin/api/keys", + headers={ + "Content-Type": "application/json", + "X-CSRF-Token": csrf, + "X-Requested-With": "XMLHttpRequest", + }, + json={"key": {"description": "admin-api-key", "username": None}}, + timeout=60, + ) + + return types.APICredentials(username=admin_credentials.username, key=res.json()["key"]["key"]) + + +async def create_discourse_account( + discourse_address: str, email: str, username: str, admin_api_headers: dict[str, str] +) -> types.Credentials: + """Create an user account on discourse. + + Args: + discourse_address: The Discourse web address. + email: The email address to use to create the account. + username: The username to use to create the account. + admin_api_headers: Headers with admin API key. + + Returns: + A newly created discourse user credential. + """ + password = secrets.token_urlsafe(16) + # Register user + requests.post( + f"{discourse_address}/users.json", + headers=admin_api_headers, + json={ + "name": username, + "email": email, + "password": password, + "username": username, + "active": True, + "approved": True, + }, + timeout=60, + ).raise_for_status() return types.Credentials(email=email, username=username, password=password) def create_user_api_key( - discourse_hostname: str, main_api_key: str, user_credentials: types.Credentials + discourse_address: str, + admin_api_headers: dict[str, str], + user_credentials: types.Credentials, ) -> str: """ Create an API key for a user. Args: - discourse_hostname: The hostname that discourse is running under. - main_api_key: The system main API key for the discourse server. + discourse_address: The web address discourse is running under. + admin_api_headers: Headers with admin API key. user_credentials: The crednetials of the user to create an API key for. Returns: The API key for the user. """ - headers = {"Api-Key": main_api_key, "Api-Username": "system"} - data = {"key[description]": "Test key", "key[username]": user_credentials.username} + data = {"key": {"description": "Test key", "username": user_credentials.username}} response = requests.post( - f"http://{discourse_hostname}/admin/api/keys", headers=headers, data=data, timeout=60 + f"{discourse_address}/admin/api/keys", headers=admin_api_headers, json=data, timeout=60 ) - assert ( - response.status_code == 200 - ), f"API creation failed, {user_credentials.username=}, {response.content=}" return response.json()["key"]["key"] @pytest_asyncio.fixture(scope="module") -async def discourse(ops_test: OpsTest, discourse_hostname: str): - """Deploy discourse.""" - postgres_charm_name = "postgresql-k8s" - redis_charm_name = "redis-k8s" - discourse_charm_name = "discourse-k8s" - assert ops_test.model is not None - await asyncio.gather( - ops_test.model.deploy(postgres_charm_name), - ops_test.model.deploy(redis_charm_name), - ) - # Using permissive throttle level to speed up tests - discourse_app = await ops_test.model.deploy( - discourse_charm_name, - config={"external_hostname": discourse_hostname, "throttle_level": "permissive"}, - ) - - await ops_test.model.wait_for_idle() - - await ops_test.model.relate(discourse_charm_name, f"{postgres_charm_name}:db-admin") - await ops_test.model.relate(discourse_charm_name, redis_charm_name) - - # mypy seems to have trouble with this line; - # "error: Cannot determine type of "name" [has-type]" - await ops_test.model.wait_for_idle(status=ActiveStatus.name) # type: ignore - - # Need to wait for the waiting status to be resolved - - async def get_discourse_status(): - """Get the status of discourse. - - Returns: - The status of discourse. - """ - # to help mypy understand model is not None. - assert ops_test.model # nosec - return (await ops_test.model.get_status())["applications"]["discourse-k8s"].status[ - "status" - ] - - for _ in range(120): - if await get_discourse_status() != "waiting": - break - await asyncio.sleep(10) - assert await get_discourse_status() != "waiting", "discourse never stopped waiting" - - return discourse_app +async def discourse_admin_credentials(discourse: Application) -> types.Credentials: + """Get the admin credentials for discourse.""" + return await create_discourse_admin_account(discourse=discourse, email="test@admin.internal") @pytest_asyncio.fixture(scope="module") -async def discourse_unit_name(discourse: Application): - """Get the admin credentials for discourse.""" - return f"{discourse.name}/0" +async def discourse_admin_api_credentials( + discourse_address: str, discourse_admin_credentials: types.Credentials +) -> types.APICredentials: + """Get the admin API credentials for discourse.""" + return await create_discourse_admin_api_key( + discourse_address=discourse_address, admin_credentials=discourse_admin_credentials + ) @pytest_asyncio.fixture(scope="module") -async def discourse_admin_credentials(ops_test: OpsTest, discourse_unit_name: str): - """Get the admin credentials for discourse.""" - return await create_discourse_account( - ops_test=ops_test, unit=discourse_unit_name, email="admin@foo.internal", admin=True - ) +async def discourse_admin_api_headers( + discourse_admin_api_credentials: types.APICredentials, +) -> dict[str, str]: + """Headers with admin api key to access API requiring admin privileges.""" + return { + "Api-Key": discourse_admin_api_credentials.key, + "Api-Username": discourse_admin_api_credentials.username, + } -# Making this depend on discourse_admin_credentials to ensure an admin user gets created -@pytest.mark.usefixtures("discourse_admin_credentials") @pytest_asyncio.fixture(scope="module") -async def discourse_user_credentials(ops_test: OpsTest, discourse_unit_name: str): +async def discourse_user_credentials( + discourse_address: str, discourse_admin_api_headers: dict[str, str] +): """Get the user credentials for discourse.""" return await create_discourse_account( - ops_test=ops_test, unit=discourse_unit_name, email="user@foo.internal", admin=False + discourse_address=discourse_address, + email="user@test.internal", + username="test_user", + admin_api_headers=discourse_admin_api_headers, ) -# Making this depend on discourse_admin_credentials to ensure an admin user gets created -@pytest.mark.usefixtures("discourse_admin_credentials") @pytest_asyncio.fixture(scope="module") -async def discourse_alternate_user_credentials(ops_test: OpsTest, discourse_unit_name: str): +async def discourse_alternate_user_credentials( + discourse_address: str, discourse_admin_api_headers: dict[str, str] +): """Get the alternate user credentials for discourse.""" return await create_discourse_account( - ops_test=ops_test, - unit=discourse_unit_name, - email="alternate_user@foo.internal", - admin=False, + discourse_address=discourse_address, + email="alternate_user@test.internal", + username="alternate_user", + admin_api_headers=discourse_admin_api_headers, ) -@pytest_asyncio.fixture(scope="module") -async def discourse_main_api_key(ops_test: OpsTest, discourse_unit_name: str): - """Get the user api key for discourse.""" - return_code, stdout, stderr = await ops_test.juju( - "exec", - "--unit", - discourse_unit_name, - "--", - "cd /srv/discourse/app && ./bin/bundle exec rake " - "api_key:create_master['main API key for testing'] RAILS_ENV=production", - ) - assert return_code == 0, f"discourse main API key creation failed, {stderr=}" - - return stdout.strip() - - @pytest_asyncio.fixture(scope="module") async def discourse_user_api_key( - discourse_main_api_key: str, + discourse_admin_api_headers: dict[str, str], discourse_user_credentials: types.Credentials, - discourse_hostname: str, + discourse_address: str, ): """Get the user api key for discourse.""" return create_user_api_key( - discourse_hostname=discourse_hostname, - main_api_key=discourse_main_api_key, + discourse_address=discourse_address, + admin_api_headers=discourse_admin_api_headers, user_credentials=discourse_user_credentials, ) @pytest_asyncio.fixture(scope="module") async def discourse_alternate_user_api_key( - discourse_main_api_key: str, + discourse_admin_api_headers, discourse_alternate_user_credentials: types.Credentials, - discourse_hostname: str, + discourse_address: str, ): """Get the alternate user api key for discourse.""" return create_user_api_key( - discourse_hostname=discourse_hostname, - main_api_key=discourse_main_api_key, + discourse_address=discourse_address, + admin_api_headers=discourse_admin_api_headers, user_credentials=discourse_alternate_user_credentials, ) @pytest_asyncio.fixture(scope="module") -async def discourse_client(discourse_main_api_key, discourse_hostname: str): +async def discourse_client( + discourse_admin_api_credentials: types.APICredentials, discourse_address: str +): """Create the category for topics.""" return pydiscourse.DiscourseClient( - host=f"http://{discourse_hostname}", - api_username="system", - api_key=discourse_main_api_key, + host=discourse_address, + api_username=discourse_admin_api_credentials.username, + api_key=discourse_admin_api_credentials.key, ) @@ -246,29 +322,32 @@ async def discourse_category_id(discourse_client: pydiscourse.DiscourseClient): @pytest_asyncio.fixture(scope="module") async def discourse_api( discourse_user_credentials: types.Credentials, - discourse_hostname: str, + discourse_address: str, discourse_user_api_key: str, discourse_category_id: int, ): """Create discourse instance.""" return Discourse( - base_path=f"http://{discourse_hostname}", + base_path=discourse_address, api_username=discourse_user_credentials.username, api_key=discourse_user_api_key, category_id=discourse_category_id, ) -@pytest_asyncio.fixture(scope="module", autouse=True) -async def discourse_enable_tags( - discourse_main_api_key, - discourse_hostname: str, +@pytest.fixture(scope="module", autouse=True) +def discourse_enable_tags( + discourse_admin_api_credentials: types.APICredentials, + discourse_address: str, ): """Enable tags on discourse.""" - headers = {"Api-Key": discourse_main_api_key, "Api-Username": "system"} + headers = { + "Api-Key": discourse_admin_api_credentials.key, + "Api-Username": discourse_admin_api_credentials.username, + } data = {"tagging_enabled": "true"} response = requests.put( - f"http://{discourse_hostname}/admin/site_settings/tagging_enabled", + f"{discourse_address}/admin/site_settings/tagging_enabled", headers=headers, data=data, timeout=60, @@ -277,10 +356,10 @@ async def discourse_enable_tags( @pytest_asyncio.fixture(scope="module", autouse=True) -async def discourse_remove_rate_limits(discourse_main_api_key, discourse_hostname: str): +async def discourse_remove_rate_limits( + discourse_admin_api_headers: dict[str, str], discourse_address: str +): """Disables rate limits on discourse.""" - headers = {"Api-Key": discourse_main_api_key, "Api-Username": "system"} - settings = { "unique_posts_mins": "0", "rate_limit_create_post": "0", @@ -303,8 +382,8 @@ async def discourse_remove_rate_limits(discourse_main_api_key, discourse_hostnam } for setting, value in settings.items(): response = requests.put( - f"http://{discourse_hostname}/admin/site_settings/{setting}", - headers=headers, + f"{discourse_address}/admin/site_settings/{setting}", + headers=discourse_admin_api_headers, data={setting: value}, timeout=60, ) diff --git a/tests/integration/pre_run.sh b/tests/integration/pre_run.sh deleted file mode 100755 index bad31f98..00000000 --- a/tests/integration/pre_run.sh +++ /dev/null @@ -1,3 +0,0 @@ -#!/bin/bash - -echo "127.0.0.1 discourse" | sudo tee -a /etc/hosts diff --git a/tests/integration/test___init__run_migrate.py b/tests/integration/test___init__run_migrate.py index 79ac6d12..ffbf2d9f 100644 --- a/tests/integration/test___init__run_migrate.py +++ b/tests/integration/test___init__run_migrate.py @@ -26,7 +26,7 @@ @pytest.mark.asyncio @pytest.mark.usefixtures("patch_create_repository_client") async def test_run_migrate( - discourse_hostname: str, + discourse_address: str, discourse_api: Discourse, caplog: pytest.LogCaptureFixture, repository: Repo, @@ -48,7 +48,7 @@ async def test_run_migrate( """ caplog.set_level(logging.INFO) document_name = "migration name 1" - discourse_prefix = f"http://{discourse_hostname}" + discourse_prefix = discourse_address content_page_1 = factories.ContentPageFactory() content_page_1_url = discourse_api.create_topic( title=content_page_1.title, diff --git a/tests/integration/test_discourse.py b/tests/integration/test_discourse.py index 2be555e4..f49aa9b5 100644 --- a/tests/integration/test_discourse.py +++ b/tests/integration/test_discourse.py @@ -84,7 +84,6 @@ async def test_create_retrieve_update_delete_topic( assert ( topic["category_id"] == discourse_category_id ), "post was not created with the correct category id" - assert topic["visible"] is False, "topic is listed" # Check permissions assert discourse_api.check_topic_read_permission( @@ -105,7 +104,9 @@ async def test_create_retrieve_update_delete_topic( discourse_api.delete_topic(url=url) topic = discourse_client.topic(slug=slug, topic_id=topic_id) - assert "withdrawn" in topic["post_stream"]["posts"][0]["cooked"], "topic not deleted" + assert ( + "topic deleted by author" in topic["post_stream"]["posts"][0]["cooked"] + ), "topic not deleted" assert topic["post_stream"]["posts"][0]["user_deleted"], "topic not deleted" with pytest.raises(DiscourseError): @@ -182,7 +183,7 @@ async def test_delete_wrong_slug(discourse_api: Discourse): @pytest.mark.usefixtures("discourse_user_api_key") @pytest.mark.asyncio async def test_create_topic_auth_error( - discourse_hostname: str, + discourse_address: str, discourse_user_credentials: types.Credentials, discourse_category_id: int, ): @@ -192,7 +193,7 @@ async def test_create_topic_auth_error( assert: then DiscourseError is raised. """ discourse = Discourse( - base_path=f"http://{discourse_hostname}", + base_path=discourse_address, api_username=discourse_user_credentials.username, api_key="invalid key", category_id=discourse_category_id, @@ -208,7 +209,7 @@ async def test_create_topic_auth_error( @pytest.mark.asyncio async def test_retrieve_topic_auth_error( - discourse_hostname: str, + discourse_address: str, discourse_user_credentials: types.Credentials, discourse_category_id: int, discourse_api: Discourse, @@ -224,7 +225,7 @@ async def test_retrieve_topic_auth_error( url = discourse_api.create_topic(title=title, content=content_1) unauth_discourse = Discourse( - base_path=f"http://{discourse_hostname}", + base_path=discourse_address, api_username=discourse_user_credentials.username, api_key="invalid key", category_id=discourse_category_id, @@ -236,7 +237,7 @@ async def test_retrieve_topic_auth_error( @pytest.mark.asyncio async def test_update_topic_auth_error( - discourse_hostname: str, + discourse_address: str, discourse_user_credentials: types.Credentials, discourse_category_id: int, discourse_api: Discourse, @@ -253,7 +254,7 @@ async def test_update_topic_auth_error( url = discourse_api.create_topic(title=title, content=content_1) unauth_discourse = Discourse( - base_path=f"http://{discourse_hostname}", + base_path=discourse_address, api_username=discourse_user_credentials.username, api_key="invalid key", category_id=discourse_category_id, @@ -266,7 +267,7 @@ async def test_update_topic_auth_error( @pytest.mark.asyncio async def test_delete_topic_auth_error( - discourse_hostname: str, + discourse_address: str, discourse_user_credentials: types.Credentials, discourse_category_id: int, discourse_api: Discourse, @@ -283,7 +284,7 @@ async def test_delete_topic_auth_error( url = discourse_api.create_topic(title=title, content=content_1) unauth_discourse = Discourse( - base_path=f"http://{discourse_hostname}", + base_path=discourse_address, api_username=discourse_user_credentials.username, api_key="invalid key", category_id=discourse_category_id, @@ -298,7 +299,7 @@ async def test_delete_topic_auth_error( @pytest.mark.asyncio async def test_read_write_permission( discourse_alternate_user_api_key: str, - discourse_hostname: str, + discourse_address: str, discourse_alternate_user_credentials: types.Credentials, discourse_category_id: int, discourse_api: Discourse, @@ -315,7 +316,7 @@ async def test_read_write_permission( url = discourse_api.create_topic(title=title, content=content_1) alternate_user_discourse = Discourse( - base_path=f"http://{discourse_hostname}", + base_path=discourse_address, api_username=discourse_alternate_user_credentials.username, api_key=discourse_alternate_user_api_key, category_id=discourse_category_id, diff --git a/tests/integration/types.py b/tests/integration/types.py index 60a6bf56..677d2bb6 100644 --- a/tests/integration/types.py +++ b/tests/integration/types.py @@ -18,3 +18,15 @@ class Credentials(NamedTuple): email: str username: str password: str + + +class APICredentials(NamedTuple): + """Credentials needed to access discourse API. + + Attrs: + username: The API Key user's username. + key: The Discourse API key. + """ + + username: str + key: str diff --git a/tests/unit/helpers.py b/tests/unit/helpers.py index b86a2b30..a05fd92e 100644 --- a/tests/unit/helpers.py +++ b/tests/unit/helpers.py @@ -62,3 +62,17 @@ def get_discourse_topic_url() -> str: A topic url for discourse. """ return f"{get_discourse_base_path()}{_URL_PATH_PREFIX}slug/1" + + +def mock_discourse_raw_topic_api(content: str) -> str: + """Transform a content into raw topic content returned from discourse. + + Args: + content: Content to post to discourse. + + Returns: + Content returned by discourse when accessing through /raw/{id} api endpoint. + """ + return ( + f" | | \n\n{content}\n\n-------------------------\n\n" + ) diff --git a/tests/unit/test_discourse.py b/tests/unit/test_discourse.py index b714d5f1..9403f595 100644 --- a/tests/unit/test_discourse.py +++ b/tests/unit/test_discourse.py @@ -6,6 +6,7 @@ # Need access to protected functions for testing # pylint: disable=protected-access +import textwrap from unittest import mock import pydiscourse @@ -590,6 +591,55 @@ def test_function_discourse_error( assert expected_message_content in exc_message +@pytest.mark.parametrize( + "raw_content, expected_content", + [ + pytest.param("test content", "test content", id="version 2.6.0 response"), + pytest.param( + textwrap.dedent( + """\ + test-username | timestamp | # 23 + + test content + + ------------------------- + + """ + ), + "test content", + id="version 2.8.14 response", + ), + pytest.param( + textwrap.dedent( + """\ + test-username | timestamp | # 23 + + test content + + ------------------------- + + reply-username | timestamp | # 23 + + reply content + + ------------------------- + + """ + ), + "test content", + id="version 2.8.14 response with post replies", + ), + ], +) +def test__parse_raw_content(discourse: Discourse, raw_content: str, expected_content: str): + """ + arrange: given raw response content from the discourse API + act: when _parse_raw_content is called + assert: a parsed topic content is returned. + """ + assert discourse._parse_raw_content(raw_content) == expected_content + + def test_retrieve_topic_read_error( monkeypatch: pytest.MonkeyPatch, discourse: Discourse, topic_url: str ): @@ -690,7 +740,9 @@ def test_retrieve_topic( content = "content 1" # mypy complains that _get_requests_session has no attribute ..., it is actually mocked mocked_get = discourse._get_requests_session.return_value.get # type: ignore - mocked_get.return_value.content = content.encode(encoding="utf-8") + mocked_get.return_value.content = helpers.mock_discourse_raw_topic_api(content=content).encode( + encoding="utf-8" + ) returned_content = discourse.retrieve_topic(url=topic_url) diff --git a/tests/unit/test_migration.py b/tests/unit/test_migration.py index 3618f2ba..99602d05 100644 --- a/tests/unit/test_migration.py +++ b/tests/unit/test_migration.py @@ -21,7 +21,7 @@ @pytest.mark.parametrize( "path, table_path, expected", [ - pytest.param(Path(""), "test", "test", id="table path only"), + pytest.param(Path(), "test", "test", id="table path only"), pytest.param(Path("group-1"), "group-1-test", "test", id="test in group"), pytest.param( Path("group-1/nested/path"), @@ -602,7 +602,7 @@ def test__migrate_document(tmp_path: Path): document_meta=document_meta, discourse=mocked_discourse, docs_path=tmp_path ) - assert (file_path := (tmp_path / path)).is_file() + assert (file_path := tmp_path / path).is_file() assert file_path.read_text(encoding="utf-8") == content mocked_discourse.retrieve_topic.assert_called_once_with(url=link_str) assert returned_report.table_row is not None @@ -625,7 +625,7 @@ def test__migrate_index(tmp_path: Path): returned_report = migration._migrate_index(index_meta=document_meta, docs_path=tmp_path) - assert (file_path := (tmp_path / path)).is_file() + assert (file_path := tmp_path / path).is_file() assert file_path.read_text(encoding="utf-8") == content assert returned_report.table_row is None assert returned_report.result == types_.ActionResult.SUCCESS diff --git a/tox.ini b/tox.ini index 0c83bd4d..e434321d 100644 --- a/tox.ini +++ b/tox.ini @@ -97,7 +97,6 @@ commands = description = Run integration tests deps = pytest - juju ops pytest-operator pytest-asyncio @@ -109,8 +108,7 @@ commands = [testenv:static] description = Run static analysis tests deps = - bandit - toml + bandit[toml] -r{toxinidir}/requirements.txt commands = bandit -c {toxinidir}/pyproject.toml -r {[vars]src_path} {[vars]tst_path}