Skip to content

Commit 3b741ee

Browse files
committed
Notify users on GitHub when comment contains id/label missmatch
Signed-off-by: Jakub Stejskal <[email protected]>
1 parent d99b36d commit 3b741ee

File tree

15 files changed

+625
-66
lines changed

15 files changed

+625
-66
lines changed

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,4 @@ secrets
99
*.egg-info
1010
.mypy_cache/
1111
/build/
12+
node_modules

packit_service/worker/checker/abstract.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,28 @@ def __init__(
2929
self.job_config = job_config
3030
self.data = EventData.from_event_dict(event)
3131
self.task_name = task_name
32+
self._mismatch_data: Optional[dict] = None
3233

3334
@abstractmethod
3435
def pre_check(self) -> bool: ...
3536

37+
def get_failure_message(self) -> Optional[dict]:
38+
"""
39+
Get the failure message/mismatch data if the check failed.
40+
This is used to aggregate failure messages into a single comment.
41+
42+
Returns:
43+
Failure message/mismatch data if check failed, None otherwise.
44+
Dict with structured data:
45+
{
46+
"type": str, # type of the matcher failure
47+
"job_value": str | list[str], # job's identifier or labels
48+
"comment_value": str | list[str], # what was specified in command
49+
"targets": list[str], # all targets for this job
50+
}
51+
"""
52+
return self._mismatch_data
53+
3654

3755
class ActorChecker(Checker):
3856
@property

packit_service/worker/checker/testing_farm.py

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,8 @@ class IsIdentifierFromCommentMatching(Checker, GetTestingFarmJobHelperMixin):
154154
otherwise only jobs with the same identifier.
155155
"""
156156

157+
DESCRIPTION: str = "**Identifiers don't match** - test run will be skipped"
158+
157159
def pre_check(self) -> bool:
158160
if (
159161
not self.testing_farm_job_helper.comment_arguments.labels
@@ -163,7 +165,17 @@ def pre_check(self) -> bool:
163165
logger.info(
164166
f"Using the default identifier for test command: {default_identifier}",
165167
)
166-
return self.job_config.identifier == default_identifier
168+
if self.job_config.identifier != default_identifier:
169+
# Create structured mismatch data for aggregation
170+
self._mismatch_data = {
171+
"type": "identifier_default",
172+
"job_value": self.job_config.identifier,
173+
"comment_value": default_identifier,
174+
"targets": list(self.job_config.targets),
175+
}
176+
177+
return False
178+
return True
167179

168180
if (
169181
not self.testing_farm_job_helper.comment_arguments.identifier
@@ -177,6 +189,15 @@ def pre_check(self) -> bool:
177189
f"(job:{self.job_config.identifier} "
178190
f"!= comment:${self.testing_farm_job_helper.comment_arguments.identifier})",
179191
)
192+
193+
# Create structured mismatch data for aggregation
194+
self._mismatch_data = {
195+
"type": "identifier_explicit",
196+
"job_value": self.job_config.identifier,
197+
"comment_value": self.testing_farm_job_helper.comment_arguments.identifier,
198+
"targets": list(self.job_config.targets),
199+
}
200+
180201
return False
181202

182203

@@ -187,17 +208,39 @@ class IsLabelFromCommentMatching(Checker, GetTestingFarmJobHelperMixin):
187208
otherwise only jobs with the same label.
188209
"""
189210

211+
DESCRIPTION: str = "**Labels don't match** - test run will be skipped"
212+
190213
def pre_check(self) -> bool:
191214
if (
192215
not self.testing_farm_job_helper.comment_arguments.labels
193216
and not self.testing_farm_job_helper.comment_arguments.identifier
194217
and (default_labels := self.job_config.test_command.default_labels)
195218
):
196219
logger.info(f"Using the default labels for test command: {default_labels}")
220+
197221
if not self.job_config.labels:
222+
# Create structured mismatch data for aggregation
223+
self._mismatch_data = {
224+
"type": "labels_default",
225+
"job_value": self.job_config.labels or [],
226+
"comment_value": list(default_labels),
227+
"targets": list(self.job_config.targets),
228+
}
229+
198230
return False
199231

200-
return any(x in default_labels for x in self.job_config.labels)
232+
if not any(x in default_labels for x in self.job_config.labels):
233+
# Create structured mismatch data for aggregation
234+
self._mismatch_data = {
235+
"type": "labels_default",
236+
"job_value": list(self.job_config.labels),
237+
"comment_value": list(default_labels),
238+
"targets": list(self.job_config.targets),
239+
}
240+
241+
return False
242+
243+
return True
201244

202245
if not self.testing_farm_job_helper.comment_arguments.labels or (
203246
self.job_config.labels
@@ -213,4 +256,13 @@ def pre_check(self) -> bool:
213256
f"(job:{self.job_config.labels} "
214257
f"!= comment:${self.testing_farm_job_helper.comment_arguments.labels})",
215258
)
259+
260+
# Create structured mismatch data for aggregation
261+
self._mismatch_data = {
262+
"type": "labels_explicit",
263+
"job_value": list(self.job_config.labels) if self.job_config.labels else [],
264+
"comment_value": list(self.testing_farm_job_helper.comment_arguments.labels),
265+
"targets": list(self.job_config.targets),
266+
}
267+
216268
return False

packit_service/worker/handlers/abstract.py

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -336,12 +336,18 @@ def pre_check(
336336
package_config: PackageConfig,
337337
job_config: JobConfig,
338338
event: dict,
339-
) -> bool:
339+
) -> tuple[bool, list[dict]]:
340340
"""
341-
Returns
342-
bool: False if we have to skip the job execution.
341+
Run pre-checks for the handler.
342+
343+
Returns:
344+
tuple[bool, list[dict]]: (checks_pass, failure_messages)
345+
- checks_pass: False if we have to skip the job execution
346+
- failure_messages: List of failure messages from failed checkers
343347
"""
344348
checks_pass = True
349+
failure_messages = []
350+
345351
for checker_cls in cls.get_checkers():
346352
task_name = getattr(cls, "task_name", None)
347353
checker = checker_cls(
@@ -350,9 +356,14 @@ def pre_check(
350356
event=event,
351357
task_name=task_name.value if task_name else None,
352358
)
353-
checks_pass = checks_pass and checker.pre_check()
359+
passed = checker.pre_check()
360+
checks_pass = checks_pass and passed
361+
362+
# Collect failure messages to be aggregated at event level
363+
if not passed and (failure_msg := checker.get_failure_message()):
364+
failure_messages.append(failure_msg)
354365

355-
return checks_pass
366+
return checks_pass, failure_messages
356367

357368
@staticmethod
358369
def get_handler_specific_task_accepted_message(

0 commit comments

Comments
 (0)