Skip to content

Commit

Permalink
fix typing, linting and tests
Browse files Browse the repository at this point in the history
  • Loading branch information
deusebio committed Aug 3, 2023
1 parent 456e3aa commit 3b7269c
Show file tree
Hide file tree
Showing 11 changed files with 115 additions and 54 deletions.
25 changes: 10 additions & 15 deletions main.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from src import GETTING_STARTED, exceptions, pre_flight_checks, run_migrate, run_reconcile, types_
from src.clients import get_clients
from src.constants import DEFAULT_BRANCH
from src.types_ import ActionResult, PullRequestAction

GITHUB_HEAD_REF_ENV_NAME = "GITHUB_HEAD_REF"
GITHUB_OUTPUT_ENV_NAME = "GITHUB_OUTPUT"
Expand Down Expand Up @@ -56,7 +57,7 @@ def _parse_env_vars() -> types_.UserInputs:
# Use the commit SHA if not running as a pull request
commit_sha = os.environ["GITHUB_SHA"]

logging.info(f"Base branch: {base_branch}")
logging.info("Base branch: %s", base_branch)

return types_.UserInputs(
discourse=types_.UserInputsDiscourse(
Expand All @@ -73,7 +74,9 @@ def _parse_env_vars() -> types_.UserInputs:
)


def _serialize_for_github(urls_with_actions_dict: dict[str, str]) -> str:
def _serialize_for_github(
urls_with_actions_dict: str | dict[str, ActionResult] | PullRequestAction | typing.Any
) -> str:
"""Serialize dictionary output into a string to be outputted to GitHub.
Args:
Expand All @@ -88,13 +91,13 @@ def _serialize_for_github(urls_with_actions_dict: dict[str, str]) -> str:


def _write_github_output(
migrate: types_.MigrateOutputs | None,
reconcile: types_.ReconcileOutputs | None
migrate: types_.MigrateOutputs | None, reconcile: types_.ReconcileOutputs | None
) -> None:
"""Writes results produced by the action to github_output.
Args:
urls_with_actions_dicts: list of key value pairs of link to result of action.
migrate: outputs of the migrate process
reconcile: outputs of the reconcile process
Raises:
InputError: if not running inside a github actions environment.
Expand All @@ -108,16 +111,8 @@ def _write_github_output(
)

output_dict = (
{
"index_url": reconcile.index_url,
"topics": reconcile.topics
} if reconcile else {}
) | (
{
"pr_action": migrate.action,
"pr_link": migrate.pull_request_url
} if migrate else {}
)
{"index_url": reconcile.index_url, "topics": reconcile.topics} if reconcile else {}
) | ({"pr_action": migrate.action, "pr_link": migrate.pull_request_url} if migrate else {})

output: str = "\n".join(
f"{key}={_serialize_for_github(value)}" for key, value in output_dict.items()
Expand Down
29 changes: 21 additions & 8 deletions prepare_check_cleanup/reconcile.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import contextlib
import json
import logging
import os
import pathlib
from enum import Enum

Expand All @@ -19,11 +18,13 @@
from src.constants import DOCUMENTATION_TAG
from src.discourse import Discourse, create_discourse
from src.exceptions import DiscourseError
from src.repository import create_repository_client, Client as RepositoryClient
from src.repository import Client as RepositoryClient
from src.repository import create_repository_client

E2E_SETUP = "origin/tests/e2e"
E2E_BASE = "tests/base"


class Action(str, Enum):
"""The actions the utility can take.
Expand All @@ -34,6 +35,7 @@ class Action(str, Enum):
CHECK_DELETE_TOPICS: Check that the delete_topics e2e test succeeded.
CHECK_DELETE: Check that the delete e2e test succeeded.
CLEANUP: Discourse cleanup after the testing.
PREPARE: Preparation steps for setting the stage for the integration tests
"""

CHECK_DRAFT = "check-draft"
Expand Down Expand Up @@ -113,22 +115,33 @@ def main() -> None:
raise NotImplementedError(f"{args.action} has not been implemented")


def _prepare(repository:RepositoryClient, discourse: Discourse) -> bool:
repository._git_repo.git.fetch("--all")
def _prepare(repository: RepositoryClient, discourse: Discourse) -> bool:
"""Prepare the stage for the tests.
with repository.create_branch(E2E_BASE, E2E_SETUP).with_branch(E2E_BASE) as repo:
Args:
repository: Client repository to used
discourse: Dicourse client class
repo._git_repo.git.push("origin", "-f", repository.current_branch)
Returns:
boolean representing whether the preparation was successful.
"""
repository._git_repo.git.fetch("--all") # pylint: disable=W0212

with repository.create_branch(E2E_BASE, E2E_SETUP).with_branch(E2E_BASE) as repo:
repo._git_repo.git.push("origin", "-f", repository.current_branch) # pylint: disable=W0212

if repository.tag_exists(DOCUMENTATION_TAG):
logging.info("Removing tag %s", DOCUMENTATION_TAG)
repo._git_repo.git.tag("-d", DOCUMENTATION_TAG)
repo._git_repo.git.push("--delete", "origin", DOCUMENTATION_TAG)
repo._git_repo.git.tag("-d", DOCUMENTATION_TAG) # pylint: disable=W0212
repo._git_repo.git.push( # pylint: disable=W0212
"--delete", "origin", DOCUMENTATION_TAG
)

assert discourse

return True


def _check_url_count(
urls_with_actions: dict[str, str], expected_count: int, test_name: str
) -> bool:
Expand Down
20 changes: 11 additions & 9 deletions src/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,16 @@
from .action import DRY_RUN_NAVLINK_LINK, FAIL_NAVLINK_LINK
from .clients import Clients
from .constants import DOCUMENTATION_FOLDER_NAME, DOCUMENTATION_TAG # DEFAULT_BRANCH,
from .docs_directory import read as read_docs_directory
from .download import recreate_docs
from .exceptions import InputError
from .repository import DEFAULT_BRANCH_NAME
from .types_ import (
ActionResult, UserInputs, PullRequestAction, ReconcileOutputs, MigrateOutputs, Url
ActionResult,
MigrateOutputs,
PullRequestAction,
ReconcileOutputs,
Url,
UserInputs,
)

GETTING_STARTED = (
Expand Down Expand Up @@ -101,8 +105,8 @@ def run_reconcile(clients: Clients, user_inputs: UserInputs) -> ReconcileOutputs
str(report.location): report.result
for report in reports
if report.location is not None
and report.location != DRY_RUN_NAVLINK_LINK
and report.location != FAIL_NAVLINK_LINK
and report.location != DRY_RUN_NAVLINK_LINK
and report.location != FAIL_NAVLINK_LINK
}

if not user_inputs.dry_run:
Expand All @@ -113,7 +117,7 @@ def run_reconcile(clients: Clients, user_inputs: UserInputs) -> ReconcileOutputs
return ReconcileOutputs(
index_url=index_url,
topics=urls_with_actions,
documentation_tag=clients.repository.tag_exists(DOCUMENTATION_TAG)
documentation_tag=clients.repository.tag_exists(DOCUMENTATION_TAG),
)


Expand Down Expand Up @@ -156,17 +160,15 @@ def run_migrate(clients: Clients, user_inputs: UserInputs) -> MigrateOutputs | N
if pull_request is not None:
pull_request.edit(state="closed")
return MigrateOutputs(
action=PullRequestAction.CLOSED,
pull_request_url=pull_request.html_url
action=PullRequestAction.CLOSED, pull_request_url=pull_request.html_url
)
return None

if pull_request is None:
logging.info("PR not existing: creating a new one...")
pull_request = clients.repository.create_pull_request(user_inputs.base_branch)
return MigrateOutputs(
action=PullRequestAction.OPENED,
pull_request_url=pull_request.html_url
action=PullRequestAction.OPENED, pull_request_url=pull_request.html_url
)

logging.info("upload-charm-documents pull request already open at %s", pull_request.html_url)
Expand Down
12 changes: 8 additions & 4 deletions src/repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,9 @@ def create_pull_request(self, base: str) -> PullRequest:
msg = str(repo.get_summary())
logging.info("Creating new branch with new commit: %s", msg)
repo.update_branch(msg, force=True)
pull_request = _create_github_pull_request(self._github_repo, DEFAULT_BRANCH_NAME, base)
pull_request = _create_github_pull_request(
self._github_repo, DEFAULT_BRANCH_NAME, base
)
logging.info("Opening new PR with community contribution: %s", pull_request.html_url)

return pull_request
Expand Down Expand Up @@ -598,12 +600,12 @@ def tag_commit(self, tag_name: str, commit_sha: str) -> None:
self._git_repo.git.tag("-d", tag_name)
self._git_repo.git.push("--delete", "origin", tag_name)

logging.error(f"Tagging commit {commit_sha} with tag {tag_name}")
logging.error("Tagging commit %s with tag %s", commit_sha, tag_name)
self._git_repo.git.tag(tag_name, commit_sha)
self._git_repo.git.push("origin", tag_name)

except GitCommandError as exc:
logging.error(f"Tagging commit failed because of {exc}")
logging.error("Tagging commit failed because of %s", exc)
raise RepositoryClientError(f"Tagging commit failed. {exc=!r}") from exc

def get_file_content_from_tag(self, path: str, tag_name: str) -> str:
Expand Down Expand Up @@ -664,7 +666,9 @@ def get_file_content_from_tag(self, path: str, tag_name: str) -> str:
return base64.b64decode(content_file.content).decode("utf-8")


def _create_github_pull_request(github_repo: Repository, branch_name: str, base: str) -> PullRequest:
def _create_github_pull_request(
github_repo: Repository, branch_name: str, base: str
) -> PullRequest:
"""Create pull request using the provided branch.
Args:
Expand Down
11 changes: 4 additions & 7 deletions src/types_.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@
from pathlib import Path
from urllib.parse import urlparse


Content = str
Url = str


class UserInputsDiscourse(typing.NamedTuple):
"""Configurable user input values used to run upload-charm-docs.
Expand Down Expand Up @@ -353,7 +353,6 @@ class PullRequestAction(str, Enum):
"""Result of taking an action.
Attrs:
NOOP: No action was done on the PR.
OPENED: A new PR has been opened.
CLOSED: An existing PR has been closed.
UPDATED: An existing PR has been updated.
Expand All @@ -364,8 +363,6 @@ class PullRequestAction(str, Enum):
UPDATED = "updated"




class ActionReport(typing.NamedTuple):
"""Post execution report for an action.
Expand Down Expand Up @@ -454,21 +451,21 @@ class ReconcileOutputs(typing.NamedTuple):
Attrs:
index_url: url with the root documentation topic on Discourse
topics: List of urls with actions
documentation_tag: commit sha to which the tag was created
"""

index_url: Url
topics: dict[Url, ActionResult]
documentation_tag: str
documentation_tag: str | None


class MigrateOutputs(typing.NamedTuple):
"""Output provided by the reconcile workflow.
Attrs:
action: Action taken on the PR
url: url of the pull-request when relevant
pull_request_url: url of the pull-request when relevant
"""

action: PullRequestAction
pull_request_url: Url

2 changes: 1 addition & 1 deletion tests/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ class Meta:
discourse = factory.SubFactory(UserInputDiscourseFactory)
github_access_token = factory.Sequence(lambda n: f"test-token-{n}")
commit_sha = factory.Sequence(lambda n: f"commit-sha-{n}")
base_branch = DEFAULT_BRANCH
base_branch = "main"
dry_run = False
delete_pages = False

Expand Down
4 changes: 4 additions & 0 deletions tests/integration/test___init__run_conflict.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ async def test_run_conflict(
),
)

assert reconcile_output is not None
urls_with_actions = reconcile_output.topics

assert len(urls_with_actions) == 2
Expand Down Expand Up @@ -205,6 +206,7 @@ async def test_run_conflict(
),
)

assert reconcile_output is not None
urls_with_actions = reconcile_output.topics

assert (urls := tuple(urls_with_actions)) == (doc_url, index_url)
Expand Down Expand Up @@ -237,6 +239,7 @@ async def test_run_conflict(
),
)

assert reconcile_output is not None
urls_with_actions = reconcile_output.topics

assert len(urls_with_actions) == 3
Expand Down Expand Up @@ -314,6 +317,7 @@ async def test_run_conflict(
),
)

assert reconcile_output is not None
urls_with_actions = reconcile_output.topics

assert len(urls_with_actions) == 3
Expand Down
5 changes: 4 additions & 1 deletion tests/integration/test___init__run_migrate.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
from src.discourse import Discourse
from src.repository import DEFAULT_BRANCH_NAME
from src.repository import Client as RepositoryClient
from src.types_ import ActionResult, PullRequestAction
from src.types_ import PullRequestAction

from .. import factories
from ..conftest import BASE_REMOTE_BRANCH
Expand Down Expand Up @@ -124,6 +124,7 @@ async def test_run_migrate(

upstream_git_repo.git.checkout(DEFAULT_BRANCH_NAME)
upstream_doc_dir = upstream_repository_path / constants.DOCUMENTATION_FOLDER_NAME
assert output_migrate is not None
assert output_migrate.pull_request_url == mock_pull_request.html_url
assert output_migrate.action == PullRequestAction.OPENED
assert (group_1_path := upstream_doc_dir / "group-1").is_dir()
Expand Down Expand Up @@ -152,6 +153,7 @@ async def test_run_migrate(
user_inputs=factories.UserInputsFactory(commit_sha=repository_client.current_commit),
)

assert output_migrate is not None
assert "test_url" in output_migrate.pull_request_url
assert output_migrate.action == PullRequestAction.UPDATED
assert_substrings_in_string(
Expand All @@ -171,6 +173,7 @@ async def test_run_migrate(
user_inputs=factories.UserInputsFactory(commit_sha=repository_client.current_commit),
)

assert output_migrate is not None
assert "test_url" in output_migrate.pull_request_url
assert output_migrate.action == PullRequestAction.UPDATED
assert_substrings_in_string(
Expand Down
Loading

0 comments on commit 3b7269c

Please sign in to comment.