From 559d938b728283d20c744a70167ab6b6baa831e5 Mon Sep 17 00:00:00 2001 From: Thomas Aglassinger Date: Thu, 21 Nov 2024 17:51:32 +0100 Subject: [PATCH] #31 Change config file handling - Rename command line argument to --config - Change --config to expect a file instead of a root folder - Change default config lookup to simply traverse folder tree upwards --- README.md | 82 +++++++++++++++++++------------------- check_done/command.py | 91 +++++++++++++++++++------------------------ check_done/config.py | 31 +++++++++------ check_done/graphql.py | 3 -- tests/_common.py | 25 ++++++++++-- tests/test_command.py | 26 ++++++++----- tests/test_config.py | 35 +++++++++-------- 7 files changed, 157 insertions(+), 136 deletions(-) diff --git a/README.md b/README.md index 3d36bce..ac7a4b8 100644 --- a/README.md +++ b/README.md @@ -1,78 +1,78 @@ # check_done -check_done is a command line tool to check that finished issues and pull requests in a GitHub project board column are really done. +Check_done is a command line tool to check that GitHub issues and pull request in a project board with a status of "Done" are really done. -## Configuration - -To use check_done for your project, you need to configure the fields in the `yaml` file found in the `configuration` folder. - -### General settings - -#### project_url (required) +For each issue or pull request in the "Done" column of the project board, it checks that: -The project URL as is when viewing your GitHub V2 project. E.g.: `"https://github.com/orgs/my_project_owner_name/projects/1/views/1"` +- It is closed. +- It has an assignee. +- It is assigned to a milestone. +- All tasks are completed (list with check boxes in the description). -#### project_status_name_to_check (optional) +For pull requests, it additionally checks if they reference an issue. -By default, check_done checks all issues and pull requests in the column rightmost / last column of the project board. If you left the default names when creating the GitHub project board, this will be the `"✅ Done"` column. +This ensures a consistent quality on done issues and pull request, and helps to notice if they were accidentally deemed to be done too early. -If you want to check a different column, you can specify its name with this option. For example: +## Installation -```yaml -project_status_name_to_check = "Done" -``` +In order to gain access to your project board, issues, and pull request, check_done needs to autorize. The exact way to do that depends on whether your project belogs to a single user or an GitHub organization. -The name takes the first column that partially matches case sensitively. For example, `"Done"` will also match `"✅ Done"`, but not `"done"`. +For user projects, [create a personal access token](https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/managing-your-personal-access-tokens) with the permission: `read:project`. -If no column matches, the resulting error messages will tell you the exact names of all available columns. +For organization projects, follow the instructions to [Installing a GitHub App from a third party](https://docs.github.com/en/apps/using-github-apps/installing-a-github-app-from-a-third-party) using the [Check_done app](https://github.com/apps/check-done-app). -### Authorization settings for a project belonging to a GitHub user +Remember the **app ID** and **private key** of the installed app. -#### personal_access_token +## Configuration -A personal access token with the permission: `read:project`. +Check_done can be configured by having a `.check_done.yaml` in your current directory, or any of directory above. -Example: +Alternatively, you can specify a specific location, for example: -```yaml -project_url: "https://github.com/orgs/my_username/projects/1/views/1" -personal_access_token: "ghp_xxxxxxxxxxxxxxxxxxxxxx" -# Since no `project_status_name_to_check` was specified, the checks will apply to the last project status/column. +```bash +check_done --config some/path/.check_done.yaml ``` -### Authorization settings for a project belonging to a GitHub organization - -Follow the instructions to [Installing a GitHub App from a third party](https://docs.github.com/en/apps/using-github-apps/installing-a-github-app-from-a-third-party) using the [Siisurit's check_done app](https://github.com/apps/siisurit-s-check-done). +### Project and authentication -You can also derive your own GitHub app from it with the following permissions: +A minimum configuration requires the URL of the project board and the authentication information from the installation. -- `pull requests` -- `projects` -- `read:issues` +The project URL can be seen in the web browser's URL bar when visiting the project board, for example: `https://github.com/users/my-username/projects/1/views/1` (for a user) or `https://github.com/my-organization/projects/1/` (for an organization). -Remember the App ID and the app private key. Then add the following settings to the configuration file: +An example for a user project could look like this: ```yaml -check_done_github_app_id = ... # Typically a decimal number with at least 6 digits -check_done_github_app_private_key = ""-----BEGIN RSA PRIVATE KEY..." +project_url: "https://github.com/users/my-username/projects/1/views/1" +personal_access_token: "ghp_xxxxxxxxxxxxxxxxxxxxxx" ``` -Example: +For an organization project: ```yaml project_url: "https://github.com/orgs/my_username/projects/1/views/1" -check_done_github_app_id: "0123456" +check_done_github_app_id: "1234567" check_done_github_app_private_key: "-----BEGIN RSA PRIVATE KEY----- something_something -----END RSA PRIVATE KEY----- " -project_status_name_to_check: "Done" # This will match the name of a project status/column containing "Done" like "✅ Done". The checks will then be applied to this project status/column. ``` -### Using environment variables and examples - -You can use environment variables for the values in the configuration yaml by starting them with a `$` symbol and wrapping it with curly braces. For example: +In order to avoid having to commit token and keys into your repository, you can use environment variables for the values in the configuration YAML by starting them with a `$` symbol and wrapping it with curly braces. For example: ```yaml personal_access_token: ${MY_PERSONAL_ACCESS_TOKEN_ENVVAR} ``` + +### Changing the board status column to check + +By default, check_done checks all issues and pull requests in the column rightmost / last column of the project board. If you left the default names when creating the GitHub project board, this will be the `"✅ Done"` column. + +If you want to check a different column, you can specify its name with this option. For example: + +```yaml +project_status_name_to_check = "Done" +``` + +The name takes the first column that partially matches case sensitively. For example, `"Done"` will also match `"✅ Done"`, but not `"done"`. + +If no column matches, the resulting error messages will tell you the exact names of all available columns. diff --git a/check_done/command.py b/check_done/command.py index d75458b..4755578 100644 --- a/check_done/command.py +++ b/check_done/command.py @@ -7,9 +7,9 @@ import check_done from check_done.config import ( + CONFIG_BASE_NAME, + default_config_path, map_from_yaml_file_path, - resolve_configuration_yaml_file_path_from_root_path, - resolve_root_repository_path, validate_configuration_info_from_yaml_map, ) from check_done.done_project_items_info import done_project_items_info @@ -20,56 +20,10 @@ _HELP_DESCRIPTION = "Checks that finished issues and pull requests in a GitHub project board column are really done." -class Command: - """Command interface for check_done""" - - def __init__(self): - self.args = None - - @staticmethod - def argument_parser(): - parser = argparse.ArgumentParser(prog="check_done", description=_HELP_DESCRIPTION) - parser.add_argument("--version", "-v", action="version", version="%(prog)s " + check_done.__version__) - # TODO#13: Is this implementation of the --root-dir argument proper? If not how to improve it? - parser.add_argument( - "--root-dir", - "-r", - type=Path, - default=resolve_root_repository_path(), - help="Specify a different root directory to check for the YAML config file.", - ) - return parser - - def apply_arguments(self, arguments=None): - parser = self.argument_parser() - self.args = parser.parse_args(arguments) - - def execute(self): - configuration_yaml_path = resolve_configuration_yaml_file_path_from_root_path(self.args.root_dir) - yaml_map = map_from_yaml_file_path(configuration_yaml_path) - configuration_info = validate_configuration_info_from_yaml_map(yaml_map) - done_project_items = done_project_items_info(configuration_info) - done_project_items_count = len(done_project_items) - if done_project_items_count == 0: - logger.info("Nothing to check. Project has no items in the selected project status.") - else: - warnings = warnings_for_done_project_items(done_project_items) - if len(warnings) == 0: - logger.info( - f"All project items are correct, " - f"{done_project_items_count!s} checked in the selected project status. " - ) - else: - for warning in warnings: - logger.warning(warning) - - -def check_done_command(arguments=None): +def check_done_command(arguments=None) -> int: result = 1 - command = Command() try: - command.apply_arguments(arguments) - command.execute() + execute(arguments) result = 0 except KeyboardInterrupt: logger.error("Interrupted as requested by user.") # noqa: TRY400 @@ -78,6 +32,43 @@ def check_done_command(arguments=None): return result +def execute(arguments=None): + parser = _argument_parser() + args = parser.parse_args(arguments) + configuration_yaml_path = args.config or default_config_path() + yaml_map = map_from_yaml_file_path(configuration_yaml_path) + configuration_info = validate_configuration_info_from_yaml_map(yaml_map) + done_project_items = done_project_items_info(configuration_info) + done_project_items_count = len(done_project_items) + if done_project_items_count == 0: + logger.info("Nothing to check. Project has no items in the selected project status.") + else: + warnings = warnings_for_done_project_items(done_project_items) + if len(warnings) == 0: + logger.info( + f"All project items are correct, " + f"{done_project_items_count!s} checked in the selected project status. " + ) + else: + for warning in warnings: + logger.warning(warning) + + +def _argument_parser(): + parser = argparse.ArgumentParser(prog="check_done", description=_HELP_DESCRIPTION) + parser.add_argument( + "--config", + "-c", + type=Path, + help=( + f"Path to configuration file with project URL, authentication details, and possibly other options; " + f"default: {CONFIG_BASE_NAME}.yaml in the current working directory or any of the above." + ), + ) + parser.add_argument("--version", "-v", action="version", version="%(prog)s " + check_done.__version__) + return parser + + def main(): logging.basicConfig(level=logging.INFO) sys.exit(check_done_command()) diff --git a/check_done/config.py b/check_done/config.py index e9db487..261291b 100644 --- a/check_done/config.py +++ b/check_done/config.py @@ -9,13 +9,12 @@ import yaml from dotenv import load_dotenv -from git import Repo from pydantic import Field, field_validator, model_validator from pydantic.dataclasses import dataclass load_dotenv() -CONFIG_FILE_NAME = ".check_done.yaml" +CONFIG_BASE_NAME = ".check_done" _GITHUB_ORGANIZATION_NAME_AND_PROJECT_NUMBER_URL_REGEX = re.compile( r"https://github\.com/orgs/(?P[a-zA-Z0-9\-]+)/projects/(?P[0-9]+).*" ) @@ -133,14 +132,22 @@ def map_from_yaml_file_path(configuration_path: Path) -> dict: return result -def resolve_root_repository_path(path: Path | str = ".", search_parent_directories=True) -> Path: - root_repository = Repo(path, search_parent_directories=search_parent_directories) - result = Path(root_repository.git.rev_parse("--show-toplevel")) - return result - - -def resolve_configuration_yaml_file_path_from_root_path(root_path: Path) -> Path: - result = Path(root_path / CONFIG_FILE_NAME) - if not Path.is_file(result): - raise FileNotFoundError(f"Configuration file missing in the specified root path: {root_path}") from None +def default_config_path() -> Path: + initial_config_folder = Path().absolute() + config_folder = initial_config_folder + previous_config_folder = None + result = None + while result is None and config_folder != previous_config_folder: + # TODO#32 Check yaml and yml. + config_path_to_check = (config_folder / CONFIG_BASE_NAME).with_suffix(".yaml") + if config_path_to_check.is_file(): + result = config_path_to_check + else: + previous_config_folder = config_folder + config_folder = config_folder.parent + if result is None: + raise FileNotFoundError( + f"Cannot find configuration file by looking for {CONFIG_BASE_NAME}.yaml " + f"and traversing upwards starting in {initial_config_folder}" + ) return result diff --git a/check_done/graphql.py b/check_done/graphql.py index c3c9c95..85c7f55 100644 --- a/check_done/graphql.py +++ b/check_done/graphql.py @@ -34,9 +34,6 @@ def __call__(self, request): return request -# TODO#13: Is the lru_cache useful? There shouldn't be multiple identical queries due to GraphQL pagination. - - @lru_cache def minimized_graphql(graphql_query: str) -> str: single_spaced_query = re.sub(r"\s+", " ", graphql_query) diff --git a/tests/_common.py b/tests/_common.py index a2e3f16..9dd67bb 100644 --- a/tests/_common.py +++ b/tests/_common.py @@ -1,6 +1,7 @@ # Copyright (C) 2024 by Siisurit e.U., Austria. # All rights reserved. Distributed under the MIT License. import os +from contextlib import contextmanager from check_done.config import ( github_project_owner_name_and_project_number_and_is_project_owner_of_type_organization_from_url_if_matches, @@ -33,8 +34,14 @@ DEMO_CHECK_DONE_PROJECT_OWNER_NAME, DEMO_CHECK_DONE_PROJECT_NUMBER, DEMO_CHECK_DONE_IS_PROJECT_OWNER_OF_TYPE_ORGANIZATION, -) = github_project_owner_name_and_project_number_and_is_project_owner_of_type_organization_from_url_if_matches( - DEMO_CHECK_DONE_GITHUB_PROJECT_URL +) = ( + github_project_owner_name_and_project_number_and_is_project_owner_of_type_organization_from_url_if_matches( + DEMO_CHECK_DONE_GITHUB_PROJECT_URL + ) + if DEMO_CHECK_DONE_GITHUB_PROJECT_URL + else None, + None, + None, ) HAS_DEMO_CHECK_DONE_ORGANIZATION_PROJECT_CONFIGURED = ( @@ -44,8 +51,8 @@ and DEMO_CHECK_DONE_IS_PROJECT_OWNER_OF_TYPE_ORGANIZATION ) REASON_SHOULD_HAVE_DEMO_CHECK_DONE_ORGANIZATION_PROJECT_CONFIGURED = ( - "To enable, setup the configuration file for an organization as described in the readme, " - "and set the following environment variables:" + f"To enable, setup the configuration file for an organization as described in the readme, " + f"and set the following environment variables:" f"{_ENVVAR_DEMO_CHECK_DONE_GITHUB_PROJECT_URL}" f"{_ENVVAR_DEMO_CHECK_DONE_GITHUB_APP_ID}" f"{_ENVVAR_DEMO_CHECK_DONE_GITHUB_APP_PRIVATE_KEY}" @@ -90,3 +97,13 @@ def new_fake_project_item_info( repository=repository, title=title, ) + + +@contextmanager +def change_current_folder(path): + old_folder = os.getcwd() + os.chdir(path) + try: + yield + finally: + os.chdir(old_folder) diff --git a/tests/test_command.py b/tests/test_command.py index 9550d25..ddd76e9 100644 --- a/tests/test_command.py +++ b/tests/test_command.py @@ -4,16 +4,17 @@ import os import re import subprocess +import tempfile from pathlib import Path from unittest.mock import patch import pytest -from check_done.command import check_done_command -from check_done.config import resolve_root_repository_path +from check_done.command import CONFIG_BASE_NAME, check_done_command from tests._common import ( HAS_DEMO_CHECK_DONE_ORGANIZATION_PROJECT_CONFIGURED, REASON_SHOULD_HAVE_DEMO_CHECK_DONE_ORGANIZATION_PROJECT_CONFIGURED, + change_current_folder, ) @@ -34,9 +35,16 @@ def test_can_show_version(): reason=REASON_SHOULD_HAVE_DEMO_CHECK_DONE_ORGANIZATION_PROJECT_CONFIGURED, ) def test_can_set_root_dir_argument(): - root_repository_path = str(resolve_root_repository_path()) - exit_code = check_done_command([f"--root-dir={root_repository_path}"]) - assert exit_code == 0 + with tempfile.TemporaryDirectory() as temp_folder, change_current_folder(temp_folder): + current_folder = Path(os.getcwd()) + config_path = (current_folder / CONFIG_BASE_NAME).with_suffix(".yaml") + config_path.write_text( + "project_url: ${CHECK_DONE_GITHUB_PROJECT_URL}\n" + "check_done_github_app_id: ${CHECK_DONE_GITHUB_APP_ID}\n" + "check_done_github_app_private_key: ${CHECK_DONE_GITHUB_APP_PRIVATE_KEY}\n" + ) + exit_code = check_done_command(["--root-dir", config_path]) + assert exit_code == 0 @pytest.mark.skipif( @@ -123,14 +131,14 @@ def test_main_script(): assert "Checking project items" in result.stderr -def test_keyboard_interrupt_handling(): - with patch("check_done.command.Command.execute", side_effect=KeyboardInterrupt): +def test_can_handle_keyboard_interrupt(): + with patch("check_done.command.execute", side_effect=KeyboardInterrupt): exit_code = check_done_command([]) assert exit_code == 1 -def test_general_exception_handling(): - with patch("check_done.command.Command.execute", side_effect=Exception("Fake exception")): +def test_can_handle_exception_handling(): + with patch("check_done.command.execute", side_effect=Exception("Fake exception")): exit_code = check_done_command([]) assert exit_code == 1 diff --git a/tests/test_config.py b/tests/test_config.py index 7670ffd..aba9c4e 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,20 +1,21 @@ # Copyright (C) 2024 by Siisurit e.U., Austria. # All rights reserved. Distributed under the MIT License. import os +import tempfile from pathlib import Path import pytest import yaml from check_done.config import ( - CONFIG_FILE_NAME, + CONFIG_BASE_NAME, ConfigurationInfo, + default_config_path, github_project_owner_name_and_project_number_and_is_project_owner_of_type_organization_from_url_if_matches, map_from_yaml_file_path, - resolve_configuration_yaml_file_path_from_root_path, - resolve_root_repository_path, resolved_environment_variables, ) +from tests._common import change_current_folder _TEST_DATA_BASE_FOLDER_FOR_MAP_FROM_YAML_FILE_TESTS = ( Path(__file__).parent / "data" / "test_configuration_map_from_yaml" @@ -173,19 +174,19 @@ def test_can_resolve_project_details_from_organization_project_url(): assert configuration_info.project_owner_name == "fake-organization-name" -def test_can_resolve_root_repository_path(): - root_repository_path = resolve_root_repository_path() - assert "check_done" in str(root_repository_path) +def test_can_resolve_config_default_path(): + with tempfile.TemporaryDirectory() as temp_folder, change_current_folder(temp_folder): + current_folder = Path(os.getcwd()) + expected_config_path = (current_folder / CONFIG_BASE_NAME).with_suffix(".yaml") + expected_config_path.write_text("") + actual_config_path = default_config_path() + assert actual_config_path == expected_config_path -def test_can_resolve_configuration_yaml_file_path_from_root_path(): - root_path = resolve_root_repository_path() - resolved_configuration_yaml_file_path_from_root_path = resolve_configuration_yaml_file_path_from_root_path( - root_path - ) - assert root_path / CONFIG_FILE_NAME == resolved_configuration_yaml_file_path_from_root_path - - -def test_fails_to_resolve_configuration_yaml_file_path_from_root_path(): - with pytest.raises(FileNotFoundError): - resolve_configuration_yaml_file_path_from_root_path(Path("dummy_root_path")) +def test_fails_on_missing_default_config_path(): + with ( + tempfile.TemporaryDirectory() as temp_folder, + change_current_folder(temp_folder), + pytest.raises(FileNotFoundError), + ): + default_config_path()