From 0a6132142492e461e96c5f590a29d56762e1d232 Mon Sep 17 00:00:00 2001 From: Caleb Evans Date: Tue, 2 Jul 2024 01:06:45 -0600 Subject: [PATCH] [Issue 185] Allow adding labels to issues via a file in the $SHARED_DIR (#203) --- README.md | 3 ++ docs/cli_usage_guide.md | 32 +++++++++++-------- src/commands/report.py | 7 ++++ src/objects/configuration.py | 16 ++-------- src/report/report.py | 6 ++++ ...label_to_open_bugs_with_passing_job_run.py | 4 +-- tests/unittests/conftest.py | 4 +-- ...watch_functions_report_get_issue_labels.py | 15 +++++++++ tests/unittests/helpers.py | 4 +++ .../resources/firewatch-additional-labels | 2 ++ 10 files changed, 63 insertions(+), 30 deletions(-) create mode 100644 tests/unittests/resources/firewatch-additional-labels diff --git a/README.md b/README.md index 9b65a4a1..634832a4 100644 --- a/README.md +++ b/README.md @@ -29,6 +29,9 @@ Thank you for your interest in the firewatch project! Please find some informati - Firewatch can optionally be configured to report test failures in a more verbose way (verbose test failure reporting). - When configured to do this, firewatch will report on EVERY test failure in all JUnit files. The issues created from this will specify the failed test name in the title and description. - **This functionality has the potential to create A LOT of tickets if cascading failures occur.** Because of this, firewatch is configured by default to only report up to 10 test failures per run. This value can be overridden, but do so with caution. + - Firewatch will add any additional labels listed in the file provided in the `--additional-labels-file` argument to any bug created. Each label must be separated by a new line. + - How to add a label to a file: `echo "some-label" >> $SHARED_DIR/firewatch-additional-labels` + - This should be used if you'd like to dynamically add labels based on parameters that can only be determined during test execution in OpenShift CI. ## Getting Started diff --git a/docs/cli_usage_guide.md b/docs/cli_usage_guide.md index 20b9215f..a320ba3b 100644 --- a/docs/cli_usage_guide.md +++ b/docs/cli_usage_guide.md @@ -2,14 +2,15 @@ ## Table of Contents -* [Using the firewatch CLI](#using-the-firewatch-cli) - * [Installation](#installation) - * [Docker (recommended)](#docker-recommended) - * [Local Machine (using venv)](#local-machine-using-venv) - * [Configuration](#configuration) - * [Usage](#usage) - * [`report`](#report) - * [`jira-config-gen`](#jiraconfiggen) +- [Using the firewatch CLI](#using-the-firewatch-cli) + - [Table of Contents](#table-of-contents) + - [Installation](#installation) + - [Docker (recommended)](#docker-recommended) + - [Local Machine (using venv)](#local-machine-using-venv) + - [Configuration](#configuration) + - [Usage](#usage) + - [`report`](#report) + - [`jira-config-gen`](#jira-config-gen) ## Installation @@ -50,6 +51,10 @@ Many of the arguments for this command have set defaults or will use an environm Usage: firewatch report [OPTIONS] Options: + --pdb Drop to `ipdb` shell on exception + --additional-labels-file PATH A file containing a list of additional + labels separated by new lines to add to any + new Jira issue. --verbose-test-failure-reporting-ticket-limit INTEGER Used to limit the number of bugs created when --verbose-test-reporting is set. If not @@ -63,16 +68,14 @@ Options: execution. --fail-with-test-failures Firewatch will fail with a non-zero exit code if a test failure is found. - --jira-config-path PATH The path to the jira configuration file. - Can be used as a Base configutation file with additional - rules set in $FIREWATCH_CONFIG env var. + --jira-config-path PATH The path to the jira configuration file --firewatch-config-path PATH The path to the firewatch configuration file --gcs-bucket TEXT The name of the GCS bucket that holds OpenShift CI logs + --pr-id TEXT The pull request number that the rehearsal + job is for. The value of $PULL_NUMBER --build-id TEXT The build ID that needs to be reported. The value of $BUILD_ID - --pr-id TEXT The pull request number that rehearsal job is - for. The value of $PULL_NUMBER --job-name-safe TEXT The safe name of a test in a Prow job. The value of $JOB_NAME_SAFE --job-name TEXT The full name of a Prow job. The value of @@ -114,6 +117,9 @@ $ firewatch report --keep-job-dir # Report a bug for each test failure found in a JUnit file for a step $ firewatch report --verbose-test-failure-reporting $ firewatch report --verbose-test-failure-reporting --verbose-test-failure-reporting-ticket-limit 100 + +# Include additional labels from a file on all issues created. All labels must be separated by a new line. +$ firewatch report --additional-labels-file /some/additional-labels-file ``` **Example of Jira Ticket Created:** diff --git a/src/commands/report.py b/src/commands/report.py index bc5af154..14c76cb6 100644 --- a/src/commands/report.py +++ b/src/commands/report.py @@ -117,6 +117,11 @@ def validate_verbose_test_failure_reporting_ticket_limit( type=click.INT, callback=validate_verbose_test_failure_reporting_ticket_limit, ) +@click.option( + "--additional-labels-file", + help="A file containing a list of additional labels separated by new lines to add to any new Jira issue.", + type=click.Path(exists=True), +) @click.option( "--pdb", help="Drop to `ipdb` shell on exception", @@ -137,6 +142,7 @@ def report( keep_job_dir: bool, verbose_test_failure_reporting: bool, verbose_test_failure_reporting_ticket_limit: Optional[int], + additional_labels_file: Optional[str], pdb: bool, ) -> None: ctx.obj["PDB"] = pdb @@ -150,6 +156,7 @@ def report( verbose_test_failure_reporting=verbose_test_failure_reporting, verbose_test_failure_reporting_ticket_limit=verbose_test_failure_reporting_ticket_limit, config_file_path=firewatch_config_path, + additional_lables_file=additional_labels_file, ) job = Job( name=job_name, diff --git a/src/objects/configuration.py b/src/objects/configuration.py index 5e41869e..adf3da92 100644 --- a/src/objects/configuration.py +++ b/src/objects/configuration.py @@ -44,6 +44,7 @@ def __init__( verbose_test_failure_reporting: bool, verbose_test_failure_reporting_ticket_limit: Optional[int] = 10, config_file_path: Union[str, None] = None, + additional_lables_file: Optional[str] = None, ): """ Constructs the Configuration object. This class is mainly used to validate the firewatch configuration given. @@ -55,29 +56,18 @@ def __init__( verbose_test_failure_reporting (bool): If true, firewatch will report all test failures found in the job. verbose_test_failure_reporting_ticket_limit (Optional[int]): Used as a safeguard to prevent firewatch from filing too many bugs. If verbose_test_reporting is set to true, this value will be used to limit the number of bugs filed. Defaults to 10. config_file_path (Union[str, None], optional): The firewatch config can be stored in a file or an environment var. Defaults to None. + additional_lables_file (Optional[str]): If set, the filepath provided will be parsed for additional labels. Each label should be separated by a new line. """ self.logger = get_logger(__name__) - # Jira Connection self.jira = jira - - # Get defaults self.default_jira_project = self._get_default_jira_project() - - # Boolean value representing if the program should fail if test failures are found. self.fail_with_test_failures = fail_with_test_failures - - # Boolean value to decide if firewatch should delete the job directory following execution. self.keep_job_dir = keep_job_dir - - # Boolean value to decide if firewatch should report all test failures found in the job. + self.additional_labels_file = additional_lables_file self.verbose_test_failure_reporting = verbose_test_failure_reporting self.verbose_test_failure_reporting_ticket_limit = verbose_test_failure_reporting_ticket_limit - - # Get the config data self.config_data = self._get_config_data(base_config_file_path=config_file_path) - - # Create the lists of Rule objects using the config data self.success_rules = self._get_success_rules( rules_list=self.config_data.get("success_rules"), ) diff --git a/src/report/report.py b/src/report/report.py index 8997cfce..53a156f9 100644 --- a/src/report/report.py +++ b/src/report/report.py @@ -183,6 +183,7 @@ def file_jira_issues( step_name=pair["failure"].step, # type: ignore type=pair["failure"].failure_type, # type: ignore jira_additional_labels=pair["rule"].jira_additional_labels, # type: ignore + jira_additional_labels_filepath=firewatch_config.additional_labels_file, failed_test_name=( pair["failure"].failed_test_name # type: ignore if firewatch_config.verbose_test_failure_reporting @@ -256,6 +257,7 @@ def report_success(self, job: Job, firewatch_config: Configuration) -> None: job_name=job.name, type="success", jira_additional_labels=rule.jira_additional_labels, # type: ignore + jira_additional_labels_filepath=firewatch_config.additional_labels_file, ) firewatch_config.jira.create_issue( @@ -571,6 +573,7 @@ def _get_issue_labels( job_name: Optional[str], type: str, jira_additional_labels: Optional[list[str]], + jira_additional_labels_filepath: Optional[str], failed_test_name: Optional[str] = None, step_name: Optional[str] = None, ) -> list[Optional[str]]: @@ -595,6 +598,9 @@ def _get_issue_labels( # Add any additional labels if jira_additional_labels: labels.extend(jira_additional_labels) + if jira_additional_labels_filepath: + with open(jira_additional_labels_filepath, "r") as file: + labels.extend(line.strip() for line in file) return list(set(labels)) diff --git a/tests/e2e/test_add_jira_label_to_open_bugs_with_passing_job_run.py b/tests/e2e/test_add_jira_label_to_open_bugs_with_passing_job_run.py index 362c15b8..eff1efa5 100644 --- a/tests/e2e/test_add_jira_label_to_open_bugs_with_passing_job_run.py +++ b/tests/e2e/test_add_jira_label_to_open_bugs_with_passing_job_run.py @@ -30,7 +30,7 @@ @pytest.fixture def job_name(): - yield "periodic-ci-openshift-pipelines-release-tests-release-v1.11-openshift-pipelines-ocp4.14-lp-interop-openshift-pipelines-interop-aws" + yield "periodic-ci-openshift-pipelines-release-tests-release-v1.14-openshift-pipelines-ocp4.16-lp-interop-openshift-pipelines-interop-aws" @pytest.fixture @@ -40,7 +40,7 @@ def job_name_safe(): @pytest.fixture def build_id(): - yield "1739164176636448768" + yield "1805119554108526592" @pytest.fixture diff --git a/tests/unittests/conftest.py b/tests/unittests/conftest.py index 0c3af94e..4c345e54 100644 --- a/tests/unittests/conftest.py +++ b/tests/unittests/conftest.py @@ -43,7 +43,7 @@ FIREWATCH_CONFIG_ENV_VAR = "FIREWATCH_CONFIG" BUILD_IDS_TO_TEST = [ - "1739164176636448768", + "1805119554108526592", ] DEFAULT_JIRA_PROJECTS_TO_TEST = [ @@ -216,7 +216,7 @@ def firewatch_config(jira): @pytest.fixture def job(firewatch_config, build_id): yield Job( - name="periodic-ci-openshift-pipelines-release-tests-release-v1.11-openshift-pipelines-ocp4.14-lp-interop-openshift-pipelines-interop-aws", + name="periodic-ci-openshift-pipelines-release-tests-release-v1.14-openshift-pipelines-ocp4.16-lp-interop-openshift-pipelines-interop-aws", name_safe="openshift-pipelines-interop-aws", build_id=build_id, gcs_bucket="test-platform-results", diff --git a/tests/unittests/functions/report/test_firewatch_functions_report_get_issue_labels.py b/tests/unittests/functions/report/test_firewatch_functions_report_get_issue_labels.py index afde540a..3e7b1a14 100644 --- a/tests/unittests/functions/report/test_firewatch_functions_report_get_issue_labels.py +++ b/tests/unittests/functions/report/test_firewatch_functions_report_get_issue_labels.py @@ -1,3 +1,4 @@ +from tests.unittests import helpers from tests.unittests.functions.report.report_base_test import ReportBaseTest @@ -8,6 +9,7 @@ def test_get_issue_labels_no_additional_labels(self): step_name="test-step-name", type="test_failure", jira_additional_labels=[], + jira_additional_labels_filepath=None, ) assert len(labels) == 4 @@ -22,6 +24,7 @@ def test_get_issue_labels_with_additional_labels(self): step_name="test-step-name", type="test_failure", jira_additional_labels=["additional-label-1", "additional-label-2"], + jira_additional_labels_filepath=None, ) assert len(labels) == 6 @@ -31,6 +34,16 @@ def test_get_issue_labels_with_additional_labels(self): assert "firewatch" in labels assert "additional-label-1" and "additional-label-2" in labels + def test_get_issue_labels_with_additional_labels_file(self): + labels = self.report._get_issue_labels( + job_name=self.job.name, + step_name="test-step-name", + type="test_failure", + jira_additional_labels=[], + jira_additional_labels_filepath=helpers._get_additional_labels_filepath(), + ) + assert "test-1" and "test-2" in labels + def test_get_issue_labels_with_failed_test_name(self): labels = self.report._get_issue_labels( job_name=self.job.name, @@ -38,6 +51,7 @@ def test_get_issue_labels_with_failed_test_name(self): type="test_failure", failed_test_name="test-name", jira_additional_labels=[], + jira_additional_labels_filepath=None, ) assert len(labels) == 5 @@ -53,6 +67,7 @@ def test_get_issue_labels_with_duplicate_labels(self): step_name="test-step-name", type="test_failure", jira_additional_labels=["test-step-name", "test_failure"], + jira_additional_labels_filepath=None, ) assert len(labels) == 4 diff --git a/tests/unittests/helpers.py b/tests/unittests/helpers.py index 28240812..e0d5c9b7 100644 --- a/tests/unittests/helpers.py +++ b/tests/unittests/helpers.py @@ -66,3 +66,7 @@ def _get_tmp_junit_dir(tmp_path: str) -> str: if not os.path.exists(junit_dir): os.mkdir(junit_dir) return junit_dir + + +def _get_additional_labels_filepath() -> str: + return f"{str(os.path.dirname(__file__))}/resources/firewatch-additional-labels" diff --git a/tests/unittests/resources/firewatch-additional-labels b/tests/unittests/resources/firewatch-additional-labels new file mode 100644 index 00000000..58d7b076 --- /dev/null +++ b/tests/unittests/resources/firewatch-additional-labels @@ -0,0 +1,2 @@ +test-1 +test-2