Skip to content

Commit

Permalink
Apply suggestions from code review
Browse files Browse the repository at this point in the history 
Co-authored-by: Ruth Netser <[email protected]>
Co-authored-by: Miles Dunn <[email protected]>
  • Loading branch information
@amp-rh @rnetser @madunn
3 people committed Jan 18, 2024
1 parent 2994035 commit e1b3bf4
Show file tree
Hide file tree
Showing 6 changed files with 120 additions and 155 deletions.
5 changes: 1 addition & 4 deletions cli/objects/failure_rule.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,5 @@ def matches_failure(self, failure: Failure) -> bool:
return (
hasattr(self, "step")
and fnmatch.fnmatch(failure.step, self.step)
and (
(failure.failure_type == self.failure_type)
or self.failure_type == "all"
)
and failure.failure_type in (self.failure_type, "all")
)
8 changes: 3 additions & 5 deletions cli/objects/job.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#
import itertools
import json
import os
from typing import Any
Expand Down Expand Up @@ -324,13 +323,12 @@ def _find_failures(self, logs_dir: str, junit_dir: str) -> Optional[list[Failure
unique_steps_with_failures = set()

# Combine lists into one list
for failure in itertools.chain(test_failures, pod_failures):
for failure in test_failures + pod_failures:
if failure.step not in unique_steps_with_failures:
unique_steps_with_failures.update([failure.step])
unique_steps_with_failures.add(failure.step)
if self.firewatch_config.failure_rules:
for rule in self.firewatch_config.failure_rules:
if rule.matches_failure(failure) and rule.ignore:
failure.ignore = True
failure.ignore = rule.matches_failure(failure) and rule.ignore
failures_list.append(failure)

if len(failures_list) > 0:
Expand Down
241 changes: 106 additions & 135 deletions tests/unittests/conftest.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
from cli.objects.job import Job
from cli.report import Report

_logger = simple_logger.logger.get_logger(__name__)
LOGGER = simple_logger.logger.get_logger(__name__)

BUILD_ID_ENV_VAR = "BUILD_ID"
FIREWATCH_DEFAULT_JIRA_PROJECT_ENV_VAR = "FIREWATCH_DEFAULT_JIRA_PROJECT"
Expand Down Expand Up @@ -54,6 +54,8 @@
["junit_install.xml", "junit_install_status.xml", "junit_symptoms.xml"],
]

DEFAULT_MOCK_ISSUE_KEY = "LPTOCPCI-MOCK"


@pytest.fixture
def jira(jira_config_path):
Expand Down Expand Up @@ -85,18 +87,18 @@ def job(firewatch_config, build_id):
def patch_jira(monkeypatch):
@dataclass
class MockIssue:
key: str = "LPTOCPCI-MOCK"
key: str = DEFAULT_MOCK_ISSUE_KEY

def create_jira_issue(*args, **kwargs):
_logger.info("Patching Report.create_jira_issue")
_logger.info(
LOGGER.info("Patching Report.create_jira_issue")
LOGGER.info(
f"Attempted call Report.create_issue with the following keywords: \n{pprint.pformat(kwargs)}",
)
return MockIssue()

def add_duplicate_comment(*args, **kwargs):
_logger.info("Patching Report.add_duplicate_comment")
_logger.info(
LOGGER.info("Patching Report.add_duplicate_comment")
LOGGER.info(
f"Attempted to call Report.add_duplicate_comment with the following keywords: \n{pprint.pformat(kwargs)}",
)
return
Expand Down Expand Up @@ -128,12 +130,12 @@ def __post_init__(self):
cap = CapJira()

def create_jira_issue(*args, **kwargs):
_logger.info("Patching Report.create_jira_issue")
LOGGER.info("Patching Report.create_jira_issue")
cap.create_jira_issue.append(CapInputs(args, kwargs))
return MockIssue()

def add_duplicate_comment(*args, **kwargs):
_logger.info("Patching Report.add_duplicate_comment")
LOGGER.info("Patching Report.add_duplicate_comment")
cap.add_duplicate_comment.append(CapInputs(args, kwargs))

monkeypatch.setattr(Report, "create_jira_issue", create_jira_issue)
Expand All @@ -143,7 +145,7 @@ def add_duplicate_comment(*args, **kwargs):

@pytest.fixture
def patch_job_log_dir(monkeypatch, job_log_dir):
_logger.info("Patching Job log dir path")
LOGGER.info("Patching Job log dir path")

def _download_logs(*args, **kwargs):
return job_log_dir.as_posix()
Expand All @@ -153,20 +155,14 @@ def _download_logs(*args, **kwargs):

@pytest.fixture
def patch_job_junit_dir(monkeypatch, job_junit_dir):
_logger.info("Patching Job junit dir path")
LOGGER.info("Patching Job junit dir path")

def _download_junit(*args, **kwargs):
return job_junit_dir.as_posix()

monkeypatch.setattr(Job, "_download_junit", _download_junit)


@pytest.fixture
def assert_job_dir_exists(job_dir):
job_dir.mkdir(exist_ok=True, parents=True)
assert job_dir.is_dir()


@pytest.fixture
def fake_log_secret_path(job_log_dir):
yield job_log_dir.joinpath("fake-step/fake_build_log.txt")
Expand All @@ -178,10 +174,78 @@ def fake_junit_secret_path(job_junit_dir):


@pytest.fixture
def assert_jira_config_file_exists(jira_config_path):
if not jira_config_path.is_file():
jira_config_path.parent.mkdir(exist_ok=True, parents=True)
jira_config_path.write_text(
def firewatch_config_json(monkeypatch):
config_json = json.dumps(
{
"failure_rules": [
{
"step": "exact-step-name",
"failure_type": "pod_failure",
"classification": "Infrastructure",
"jira_project": "!default",
"jira_component": ["some-component"],
"jira_assignee": "[email protected]",
"jira_security_level": "Restricted",
},
{
"step": "*partial-name*",
"failure_type": "all",
"classification": "Misc.",
"jira_project": "OTHER",
"jira_component": ["component-1", "component-2", "!default"],
"jira_priority": "major",
"group": {"name": "some-group", "priority": 1},
},
{
"step": "*ends-with-this",
"failure_type": "test_failure",
"classification": "Test failures",
"jira_epic": "!default",
"jira_additional_labels": [
"test-label-1",
"test-label-2",
"!default",
],
"group": {"name": "some-group", "priority": 2},
},
{
"step": "*ignore*",
"failure_type": "test_failure",
"classification": "NONE",
"jira_project": "NONE",
"ignore": "true",
},
{
"step": "affects-version",
"failure_type": "all",
"classification": "Affects Version",
"jira_project": "TEST",
"jira_epic": "!default",
"jira_affects_version": "4.14",
"jira_assignee": "!default",
},
{
"step": "affects-version",
"failure_type": "all",
"classification": "Affects Version",
"jira_project": "TEST",
"jira_epic": "!default",
"jira_affects_version": "4.14",
"jira_assignee": "!default",
},
],
},
)
monkeypatch.setenv(FIREWATCH_CONFIG_ENV_VAR, config_json)
yield config_json


@pytest.fixture
def jira_config_path(tmp_path):
config_path = tmp_path.joinpath("jira_config.json")
if not config_path.is_file():
config_path.parent.mkdir(exist_ok=True, parents=True)
config_path.write_text(
json.dumps(
{
"token": os.getenv(JIRA_TOKEN_ENV_VAR),
Expand All @@ -193,87 +257,7 @@ def assert_jira_config_file_exists(jira_config_path):
},
),
)
assert jira_config_path.is_file()


@pytest.fixture
def assert_firewatch_config_in_env(monkeypatch):
monkeypatch.setenv(
FIREWATCH_CONFIG_ENV_VAR,
json.dumps(
{
"failure_rules": [
{
"step": "exact-step-name",
"failure_type": "pod_failure",
"classification": "Infrastructure",
"jira_project": "!default",
"jira_component": ["some-component"],
"jira_assignee": "[email protected]",
"jira_security_level": "Restricted",
},
{
"step": "*partial-name*",
"failure_type": "all",
"classification": "Misc.",
"jira_project": "OTHER",
"jira_component": ["component-1", "component-2", "!default"],
"jira_priority": "major",
"group": {"name": "some-group", "priority": 1},
},
{
"step": "*ends-with-this",
"failure_type": "test_failure",
"classification": "Test failures",
"jira_epic": "!default",
"jira_additional_labels": [
"test-label-1",
"test-label-2",
"!default",
],
"group": {"name": "some-group", "priority": 2},
},
{
"step": "*ignore*",
"failure_type": "test_failure",
"classification": "NONE",
"jira_project": "NONE",
"ignore": "true",
},
{
"step": "affects-version",
"failure_type": "all",
"classification": "Affects Version",
"jira_project": "TEST",
"jira_epic": "!default",
"jira_affects_version": "4.14",
"jira_assignee": "!default",
},
{
"step": "affects-version",
"failure_type": "all",
"classification": "Affects Version",
"jira_project": "TEST",
"jira_epic": "!default",
"jira_affects_version": "4.14",
"jira_assignee": "!default",
},
],
},
),
)
assert os.getenv(FIREWATCH_CONFIG_ENV_VAR)


@pytest.fixture
def assert_artifact_dir_exists(artifact_dir):
artifact_dir.mkdir(exist_ok=True, parents=True)
assert artifact_dir.is_dir()


@pytest.fixture
def jira_config_path(tmp_path):
yield tmp_path.joinpath("jira_config.json")
yield config_path


@pytest.fixture(params=JOB_STEP_DIRS_TO_TEST)
Expand Down Expand Up @@ -307,14 +291,11 @@ def job_dir_junit_artifact_paths(request, job_junit_step_dirs):


@pytest.fixture
def assert_artifact_dir_in_env(monkeypatch, artifact_dir):
monkeypatch.setenv(ARTIFACT_DIR_ENV_VAR, artifact_dir.as_posix())
assert os.getenv(ARTIFACT_DIR_ENV_VAR)


@pytest.fixture
def artifact_dir(tmp_path):
yield tmp_path / "artifacts"
def artifact_dir(monkeypatch, tmp_path):
path = tmp_path / "artifacts"
monkeypatch.setenv(ARTIFACT_DIR_ENV_VAR, path.as_posix())
path.mkdir(exist_ok=True, parents=True)
yield path


@pytest.fixture
Expand All @@ -323,38 +304,28 @@ def job_dir(tmp_path, build_id):


@pytest.fixture(params=BUILD_IDS_TO_TEST, ids=BUILD_IDS_TO_TEST)
def build_id(request):
yield request.param
def build_id(monkeypatch, request):
param = request.param
monkeypatch.setenv(BUILD_ID_ENV_VAR, param)
yield param


@pytest.fixture(params=DEFAULT_JIRA_PROJECTS_TO_TEST, ids=DEFAULT_JIRA_PROJECTS_TO_TEST)
def default_jira_project(request):
yield request.param
def default_jira_project(monkeypatch, request):
param = request.param
monkeypatch.setenv(FIREWATCH_DEFAULT_JIRA_PROJECT_ENV_VAR, param)
yield param


@pytest.fixture(params=DEFAULT_JIRA_EPICS_TO_TEST, ids=DEFAULT_JIRA_EPICS_TO_TEST)
def default_jira_epic(request):
yield request.param


@pytest.fixture
def assert_build_id_in_env(build_id, monkeypatch):
monkeypatch.setenv(BUILD_ID_ENV_VAR, build_id)
assert os.getenv(BUILD_ID_ENV_VAR)


@pytest.fixture
def assert_default_jira_project_in_env(default_jira_project, monkeypatch):
monkeypatch.setenv(FIREWATCH_DEFAULT_JIRA_PROJECT_ENV_VAR, default_jira_project)
assert os.getenv(FIREWATCH_DEFAULT_JIRA_PROJECT_ENV_VAR)


@pytest.fixture
def assert_default_jira_epic_in_env(default_jira_epic, monkeypatch):
monkeypatch.setenv(FIREWATCH_DEFAULT_JIRA_EPIC_ENV_VAR, default_jira_epic)
assert os.getenv(FIREWATCH_DEFAULT_JIRA_EPIC_ENV_VAR)
def default_jira_epic(monkeypatch, request):
param = request.param
monkeypatch.setenv(FIREWATCH_DEFAULT_JIRA_EPIC_ENV_VAR, param)
yield param


@pytest.fixture
def assert_jira_token_in_env():
assert os.getenv(JIRA_TOKEN_ENV_VAR)
def jira_token():
token = os.getenv(JIRA_TOKEN_ENV_VAR)
assert token
yield token
2 changes: 1 addition & 1 deletion tests/unittests/objects/failure_rule/test_firewatch_objects_failure_rule_matches_failure.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@


@pytest.fixture(autouse=True)
def setup_tests(assert_default_jira_project_in_env):
def setup_tests(default_jira_project):
...


Expand Down
Loading

0 comments on commit e1b3bf4

Please sign in to comment.