Skip to content

Commit

Permalink
Remove project/... label functionalities
Browse files Browse the repository at this point in the history
We will no longer add `project/...` labels to our issues because we're
only building the one big `llvm` project for a while now. We could
invest in better classification of LLVM sub-projects as described in
[this issue](fedora-llvm-team#579) but that's a different story. For now, let's remove
the `project/...` label facility.

NOTE: In places where the testing code was using `project/clang` for
example, I've picked another label that doesn't interfere with the test.

Fixes: fedora-llvm-team#1103
See: fedora-llvm-team#579
  • Loading branch information
kwk committed Feb 24, 2025
1 parent 75ae055 commit 79439b1
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 48 deletions.
15 changes: 2 additions & 13 deletions snapshot_manager/snapshot_manager/github_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,12 +127,12 @@ def initial_comment(self) -> str:
<summary>At certain intervals the CI system will update this very comment over time to reflect the progress of builds.</summary>
<dl>
<dt>Log analysis</dt>
<dd>For example if a build of the <code>llvm</code> project fails on the <code>fedora-rawhide-x86_64</code> platform,
<dd>For example if a build fails on the <code>fedora-rawhide-x86_64</code> platform,
we'll analyze the build log (if any) to identify the cause of the failure. The cause can be any of <code>{build_status.ErrorCause.list()}</code>.
For each cause we will list the packages and the relevant log excerpts.</dd>
<dt>Use of labels</dt>
<dd>Let's assume a unit test test in upstream LLVM was broken.
We will then add these labels to this issue: <code>error/test</code>, <code>build_failed_on/fedora-rawhide-x86_64</code>, <code>project/llvm</code>.
We will then add these labels to this issue: <code>error/test</code>, <code>build_failed_on/fedora-rawhide-x86_64</code>.
If you manually restart a build in Copr and can bring it to a successful state, we will automatically
remove the aforementioned labels.
</dd>
Expand Down Expand Up @@ -259,10 +259,6 @@ def get_error_label_names_on_issue(cls, issue: github.Issue.Issue) -> list[str]:
def get_build_failed_on_names_on_issue(cls, issue: github.Issue.Issue) -> list[str]:
return cls.get_label_names_on_issue(issue, prefix="build_failed_on/")

@classmethod
def get_project_label_names_on_issue(cls, issue: github.Issue.Issue) -> list[str]:
return cls.get_label_names_on_issue(issue, prefix="project/")

def create_labels_for_error_causes(
self, labels: list[str], **kw_args
) -> list[github.Label.Label]:
Expand All @@ -277,13 +273,6 @@ def create_labels_for_build_failed_on(
labels=labels, prefix="build_failed_on/", color="F9D0C4", **kw_args
)

def create_labels_for_projects(
self, labels: list[str], **kw_args
) -> list[github.Label.Label]:
return self.create_labels(
labels=labels, prefix="project/", color="BFDADC", **kw_args
)

def create_labels_for_strategies(
self, labels: list[str], **kw_args
) -> list[github.Label.Label]:
Expand Down
10 changes: 1 addition & 9 deletions snapshot_manager/snapshot_manager/snapshot_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,6 @@ def handle_labels(
):
logging.info("Gather labels based on the errors we've found")
error_labels = list({f"error/{err.err_cause}" for err in errors})
project_labels = list({f"project/{err.package_name}" for err in errors})
build_failed_on_labels = list(
{f"build_failed_on/{err.chroot}" for err in errors}
)
Expand All @@ -457,7 +456,6 @@ def handle_labels(
)
self.github.create_labels_for_error_causes(error_labels)
self.github.create_labels_for_build_failed_on(build_failed_on_labels)
self.github.create_labels_for_projects(project_labels)
self.github.create_labels_for_strategies(strategy_labels)
self.github.create_labels_for_in_testing(all_chroots)
self.github.create_labels_for_tested_on(all_chroots)
Expand All @@ -470,13 +468,11 @@ def handle_labels(

labels_to_be_removed: list[str] = []
old_error_labels = self.github.get_error_label_names_on_issue(issue=issue)
old_project_labels = self.github.get_project_label_names_on_issue(issue=issue)
old_build_failed_labels = self.github.get_build_failed_on_names_on_issue(
issue=issue
)

labels_to_be_removed.extend(set(old_error_labels) - set(error_labels))
labels_to_be_removed.extend(set(old_project_labels) - set(project_labels))
labels_to_be_removed.extend(
set(old_build_failed_labels) - set(build_failed_on_labels)
)
Expand All @@ -487,11 +483,7 @@ def handle_labels(

# Labels must be added or removed manually in order to not remove manually added labels :/
labels_to_add = (
error_labels
+ project_labels
+ build_failed_on_labels
+ strategy_labels
+ other_labels
error_labels + build_failed_on_labels + strategy_labels + other_labels
)
logging.info(f"Adding label: {labels_to_add}")
issue.add_to_labels(*labels_to_add)
36 changes: 10 additions & 26 deletions snapshot_manager/tests/github_util_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -281,12 +281,6 @@ def label_testdata(only_ids: bool = False):
lambda gh, labels: gh.create_labels_for_build_failed_on(labels=labels),
lambda lbl: MyLabel(name=f"build_failed_on/{lbl}", color="F9D0C4"),
),
(
"create_labels_for_projects",
"myproject",
lambda gh, labels: gh.create_labels_for_projects(labels=labels),
lambda lbl: MyLabel(name=f"project/{lbl}", color="BFDADC"),
),
(
"create_labels_for_strategies",
"mystrategy",
Expand Down Expand Up @@ -349,7 +343,7 @@ def test_get_label_names_on_issue(issue_mock: mock.Mock):
issue_mock.get_labels.return_value = [
MyLabel(name="error/foo"),
MyLabel(name="error/bar"),
MyLabel(name="project/clang"),
MyLabel(name="tested_on/fedora-rawhide-x86_64"),
]
actual = github_util.GithubClient.get_label_names_on_issue(
issue=issue_mock, prefix="error/"
Expand All @@ -363,7 +357,7 @@ def test_get_error_label_names_on_issue(issue_mock: mock.Mock):
issue_mock.get_labels.return_value = [
MyLabel(name="error/foo"),
MyLabel(name="error/bar"),
MyLabel(name="project/clang"),
MyLabel(name="tested_on/fedora-rawhide-x86_64"),
]
actual = github_util.GithubClient.get_error_label_names_on_issue(issue=issue_mock)
expected = ["error/foo", "error/bar"]
Expand All @@ -375,7 +369,7 @@ def test_get_build_failed_on_names_on_issue(issue_mock: mock.Mock):
issue_mock.get_labels.return_value = [
MyLabel(name="build_failed_on/foo"),
MyLabel(name="build_failed_on/bar"),
MyLabel(name="project/clang"),
MyLabel(name="tested_on/fedora-rawhide-x86_64"),
]
actual = github_util.GithubClient.get_build_failed_on_names_on_issue(
issue=issue_mock
Expand All @@ -384,18 +378,6 @@ def test_get_build_failed_on_names_on_issue(issue_mock: mock.Mock):
assert actual == expected


@mock.patch("github.Issue.Issue", autospec=True)
def test_get_project_label_names_on_issue(issue_mock: mock.Mock):
issue_mock.get_labels.return_value = [
MyLabel(name="build_failed_on/foo"),
MyLabel(name="build_failed_on/bar"),
MyLabel(name="project/clang"),
]
actual = github_util.GithubClient.get_project_label_names_on_issue(issue=issue_mock)
expected = ["project/clang"]
assert actual == expected


@mock.patch("github.Issue.Issue", autospec=True)
def test_get_comment__no_comments(issue_mock: mock.Mock, github_client_fxt):
issue_mock.get_comments = mock.Mock()
Expand Down Expand Up @@ -487,14 +469,16 @@ def test_create_or_update_comment__edit(
def test_remove_labels_safe(issue_mock: mock.Mock):
issue_mock.get_labels.return_value = [
MyLabel(name="build_failed_on/fedora-rawhide-s390x"),
MyLabel(name="project/clang"),
MyLabel(name="tested_on/fedora-rawhide-x86_64"),
]

github_util.GithubClient.remove_labels_safe(
issue=issue_mock, label_names_to_be_removed=["project/clang"]
issue=issue_mock, label_names_to_be_removed=["tested_on/fedora-rawhide-x86_64"]
)

issue_mock.remove_from_labels.assert_called_once_with("project/clang")
issue_mock.remove_from_labels.assert_called_once_with(
"tested_on/fedora-rawhide-x86_64"
)


def test_minimize_comment_as_outdated__with_issue_comment(github_client_fxt):
Expand Down Expand Up @@ -706,7 +690,7 @@ def test_flip_test_label(github_client_fxt):
new_label = f"tests_failed_on/{chroot}"

issue_mock.get_labels.return_value = [
MyLabel(name="project/clang"),
MyLabel(name="error/test"),
# This will be removed
MyLabel(name="in_testing/fedora-rawhide-x86_64"),
MyLabel(name="tested_on/fedora-rawhide-ppc64le"),
Expand All @@ -730,7 +714,7 @@ def test_flip_test_label__already_present(github_client_fxt):
new_label = f"tests_failed_on/{chroot}"

issue_mock.get_labels.return_value = [
MyLabel(name="project/clang"),
MyLabel(name="error/tests"),
MyLabel(name="tests_failed_on/fedora-rawhide-x86_64"),
MyLabel(name="tested_on/fedora-rawhide-ppc64le"),
]
Expand Down

0 comments on commit 79439b1

Please # to comment.