diff --git a/services/hackbot-pulse-listener/README.md b/services/hackbot-pulse-listener/README.md index 8ab006b7ec..1b5b039e75 100644 --- a/services/hackbot-pulse-listener/README.md +++ b/services/hackbot-pulse-listener/README.md @@ -1,35 +1,51 @@ # Hackbot Pulse Listener -Listens to Taskcluster build-failure pulse messages, and for failed **Firefox build -tasks** triggers the `build-repair` hackbot agent through the hackbot-api. When a run -finishes (minutes later) it emails a link to the hackbot UI, a summary of the analysis -and fix, a Treeherder link to the failing job, and the commit the agent blamed for the -failure. The email's primary recipient is the author of that blamed commit. +Its job is to **subscribe** to Taskcluster failure messages, **filter** them down to +failures worth acting on, **dedupe** them, and dispatch a hackbot agent through the +hackbot-api. It deliberately holds no investigation logic: each agent resolves the push +and commits itself, so the listener only decides _what to hand off_. When a run finishes +(minutes later) the listener polls the result and emails a report. + +Failed **build** tasks go to `build-repair`; failed **test** tasks go to `test-repair`. ## How it works -1. Consume `task-failed` messages from `pulse.mozilla.org`. -2. Keep only **build** tasks (`tags.label` contains `build` and not `test`) on a watched - `project` (`WATCHED_REPOS`, default `autoland`). Build tasks don't run tests, so a - failure is a compilation/link error. -3. Fetch the task definition to read `GECKO_HEAD_REV` (the revision is not in the message). -4. Skip tasks scheduled by an **action task** rather than by the push: `extra.parent` - points at the decision task (= the task group) for everything the push scheduled, and at - the action-callback task for a backfill or retrigger. -5. Skip pushes that landed more than `MAX_PUSH_AGE_HOURS` ago (default 24). A failure can - surface long after its push, and by then the push has been superseded. -6. Skip failures **inherited** from an ancestor push, waiting for an unsettled ancestor - build to finish first. Both this and the push-age check fail open on error, so a real - regression is never silently dropped. -7. Dedupe by revision with an in-memory TTL cache, so only one agent run is triggered per - revision even when many build tasks fail for the same push. -8. Trigger the agent with the failing tasks -- it resolves the push commits itself and - returns the commit it blamed for the failure, so the listener does no pushlog lookups. -9. `POST /agents/build-repair/runs`, then poll `GET /runs/{run_id}` until terminal. Look the - blamed commit up in the firefox GitHub mirror to get its author's email and send the - report email to that developer. - -The dedupe cache and pending-run tracking are in-memory (reset on restart). +1. **Subscribe.** Consume `task-failed` messages from `pulse.mozilla.org`. +2. **Route** to a watched `project` (`WATCHED_REPOS`, default `autoland`), then by task + kind: build tasks (`tags.label` contains `build` and not `test`, so a failure is a + compilation/link error) take the build-repair path; test tasks take the test-repair + path. Fetch the task definition for `GECKO_HEAD_REV` (not in the message). +3. **Discard what is not this push's failure**, on both paths: + - Tasks scheduled by an **action task** rather than by the push: `extra.parent` points + at the decision task (= the task group) for everything the push scheduled, and at the + action-callback task for a backfill or retrigger. + - Pushes that landed more than `MAX_PUSH_AGE_HOURS` ago (default 24). A failure can + surface long after its push, and by then the push has been superseded. +4. **Dedupe** with in-memory TTL caches, both keyed by revision: one run per push per + agent, triggered on the first failing task worth investigating. The test-repair + agent works from a single task but reads the push's other failures itself, so a + second run for the same push would re-tread the same ground. A revision is recorded + only once a run is actually triggered, so a task rejected as intermittent or + inherited leaves the push open for the next one. +5. **Judge** whether the failure is worth a run. Every check fails open — an upstream + error runs the agent rather than dropping a possible regression. + - _Build:_ keep only failures this push introduced, waiting for an unsettled ancestor + build to finish first. + - _Test:_ first drop whatever Treeherder judges not to be a new regression + (intermittent, infra, expected-fail, fixed-by-commit). Treeherder ingests a minute or + so behind us and classifies a few minutes after that, so this gate waits for the job + to appear and then for its verdict — it is the cheap filter, and most test failures + stop here, before any group is fetched or any ancestor walked. What survives is + narrowed to the groups that are new for this task's own configuration (platform and + build option), then Treeherder is asked once more, since a verdict can still land + while that walk runs. The run carries only the task id. +6. **Dispatch & report.** `POST /agents/{agent}/runs`, poll `GET /runs/{run_id}` until + terminal, then email a hackbot UI link, the analysis summary, a Treeherder link, and the + commit the agent blamed. Build-repair looks the blamed commit up in the firefox GitHub + mirror and mails its author; test-repair mails the notification address + (`TEST_REPAIR_NOTIFICATION_EMAIL`). + +The dedupe caches and pending-run tracking are in-memory (reset on restart). ## Run locally @@ -37,18 +53,19 @@ The dedupe cache and pending-run tracking are in-memory (reset on restart). export PULSE_USER=... PULSE_PASSWORD=... # https://pulseguardian.mozilla.org export HACKBOT_API_URL=https://hackbot-api.../ HACKBOT_API_KEY=... export HACKBOT_UI_URL=https://hackbot-ui.../ -export WATCHED_REPOS=try +export WATCHED_REPOS=autoland export DRY_RUN=true # log intended calls, don't POST uv run --package hackbot-pulse-listener python -m app ``` Email is sent only when `SENDGRID_API_KEY` and `NOTIFICATION_SENDER` are set; otherwise it -is logged and skipped. The blamed commit's author (looked up in the firefox GitHub mirror), -the pushing developer, and, if set, the -`NOTIFICATION_TEAM_EMAIL` team address all get the email, which spells out why each -recipient is on it. Set `NOTIFICATION_OVERRIDE_EMAIL` to route every notification to a -single address (useful for local testing). By default only runs that produced a patch are -emailed; set `NOTIFY_ONLY_WITH_PATCH=false` to also notify on transient / not-to-blame runs. +is logged and skipped. Build-repair mails the blamed commit's author (looked up in the +firefox GitHub mirror), the pushing developer, and the `NOTIFICATION_TEAM_EMAIL` team +address if set; test-repair mails `TEST_REPAIR_NOTIFICATION_EMAIL` (with the culprit author CC'd). Set +`NOTIFICATION_OVERRIDE_EMAIL` to route every notification to a single address (useful for +local testing). By default only build-repair runs that produced a patch are emailed; set +`NOTIFY_ONLY_WITH_PATCH=false` to also notify on transient / not-to-blame runs (test-repair always +notifies). When `NOTIFICATION_TEAM_EMAIL` is set, notifications use it as `Reply-To` so recipients can reply with feedback on the analysis. diff --git a/services/hackbot-pulse-listener/app/__main__.py b/services/hackbot-pulse-listener/app/__main__.py index 76d736f138..5ffc2a4123 100644 --- a/services/hackbot-pulse-listener/app/__main__.py +++ b/services/hackbot-pulse-listener/app/__main__.py @@ -1,11 +1,13 @@ import logging +import os import signal from concurrent.futures import ThreadPoolExecutor +# Configures logging on import; must precede the imports that pull in mozci. +import app.logging_setup # noqa: F401 from app import consumer from app.config import settings -logging.basicConfig(level=logging.INFO) logger = logging.getLogger(__name__) @@ -30,14 +32,35 @@ def shutdown(signum, _frame): signal.signal(signal.SIGINT, shutdown) logger.info( - "Listening for build failures on %s; watched repos: %s", + "Listening for build and test failures on %s; watched repos: %s%s", ", ".join(consumer.EXCHANGES), sorted(settings.watched_repos_set), + " (DRY RUN: no agent runs will be triggered)" if settings.dry_run else "", ) + code = 0 try: consumer_obj.run() + except Exception: + logger.exception("Consumer loop failed") + code = 1 finally: - executor.shutdown(wait=False) + _exit_now(executor, code) + + +def _exit_now(executor: ThreadPoolExecutor, code: int) -> None: + """Drop in-flight work and exit immediately. + + Worker threads block for a long time by design (a regression check waits up + to an hour for an ancestor push, run polling up to ``run_max_age_minutes``), + and ThreadPoolExecutor's threads are non-daemon: its atexit hook joins them, + so a normal exit would hang for hours and Ctrl+C would appear to do nothing. + That hook also breaks any thread still submitting to a pool mid-shutdown. + In-flight work is disposable (pending runs are only tracked in memory), so + abandon it rather than wait. + """ + executor.shutdown(wait=False, cancel_futures=True) + logging.shutdown() + os._exit(code) if __name__ == "__main__": diff --git a/services/hackbot-pulse-listener/app/client.py b/services/hackbot-pulse-listener/app/client.py index a4f98ba518..ff6b1d1379 100644 --- a/services/hackbot-pulse-listener/app/client.py +++ b/services/hackbot-pulse-listener/app/client.py @@ -13,13 +13,23 @@ def _headers() -> dict[str, str]: return {"X-API-Key": settings.hackbot_api_key} -def trigger_run(inputs: dict) -> str | None: - """Create a build-repair run. Returns the run id, or None in dry-run mode.""" +def trigger_run(inputs: dict, agent_name: str | None = None) -> str | None: + """Create an agent run. Returns the run id, or None in dry-run mode. + + The model/turn overrides are settings shared by every agent, so they are added + here rather than by each caller. + """ + agent = agent_name or settings.agent_name + inputs = dict(inputs) + if settings.model: + inputs["model"] = settings.model + if settings.max_turns is not None: + inputs["max_turns"] = settings.max_turns if settings.dry_run: - logger.info("[dry-run] would trigger %s run: %s", settings.agent_name, inputs) + logger.info("[dry-run] would trigger %s run: %s", agent, inputs) return None - url = f"{settings.hackbot_api_url}/agents/{settings.agent_name}/runs" + url = f"{settings.hackbot_api_url}/agents/{agent}/runs" resp = httpx.post(url, json=inputs, headers=_headers(), timeout=_TIMEOUT) resp.raise_for_status() return resp.json()["run_id"] diff --git a/services/hackbot-pulse-listener/app/config.py b/services/hackbot-pulse-listener/app/config.py index f7992eb53f..6e17660945 100644 --- a/services/hackbot-pulse-listener/app/config.py +++ b/services/hackbot-pulse-listener/app/config.py @@ -12,6 +12,8 @@ class Settings(BaseSettings): hackbot_api_key: str = "" hackbot_ui_url: str = "" agent_name: str = "build-repair" + # Agent that analyzes test failures (separate Cloud Run Job from build-repair). + test_repair_agent_name: str = "test-repair" # Source links shown in notifications. firefox_git_url: str = "https://github.com/mozilla-firefox/firefox" @@ -25,11 +27,19 @@ class Settings(BaseSettings): # Pushes older than this are not repaired. A build failure can arrive long # after its push (a backfill, a long-queued task, a replayed message), and by # then the push has been superseded. Generous next to the minutes a build - # needs plus the hour the regression check may wait for an ancestor. + # needs plus the ten the regression check may wait for an ancestor. max_push_age_hours: float = 24 run_try_push: bool = False model: str | None = None max_turns: int | None = None + # Treeherder classifies a failing job shortly after we see the failure, so the + # gate waits for the job to be ingested before reading that verdict. + treeherder_ingest_poll_seconds: int = 30 + treeherder_ingest_max_wait_seconds: int = 240 + # How long to wait for a verdict once the job is ingested. Classification lands a + # few minutes after ingestion, and most test failures turn out to be intermittent + # or expected-fail, so waiting here rejects them before the ancestor walk. + treeherder_classification_wait_seconds: int = 300 # Dedupe (in-memory, by hg revision) dedupe_ttl_seconds: int = 6 * 60 * 60 @@ -50,12 +60,18 @@ class Settings(BaseSettings): notification_sender: str | None = None # Team address CC'd on every notification alongside the revision author. notification_team_email: str | None = None + # Distribution address; primary recipient of test-repair verdicts. + test_repair_notification_email: str | None = None # Send all notifications to this address instead of the developer (local testing). notification_override_email: str | None = None # Only notify when the run produced a patch (skip transient / not-to-blame runs). notify_only_with_patch: bool = True dry_run: bool = False + log_level: str = "INFO" + # mozci's own (loguru) logging. Its per-task "missing results" warnings are + # normal operation, not something to act on; lower this to debug mozci. + mozci_log_level: str = "ERROR" environment: str = "development" sentry_dsn: str | None = None diff --git a/services/hackbot-pulse-listener/app/consumer.py b/services/hackbot-pulse-listener/app/consumer.py index e110294f67..40c83aced0 100644 --- a/services/hackbot-pulse-listener/app/consumer.py +++ b/services/hackbot-pulse-listener/app/consumer.py @@ -6,7 +6,7 @@ from kombu import Connection, Exchange, Queue from kombu.mixins import ConsumerMixin -from app import client, lando, regression, taskcluster, worker +from app import client, lando, regression, taskcluster, treeherder, worker from app.config import settings from app.models import RunContext @@ -16,30 +16,65 @@ EXCHANGES = ("exchange/taskcluster-queue/v1/task-failed",) -# In-memory dedupe of hg revisions already handed to the agent. A revision is -# recorded only once we actually trigger a run, so an inherited failure on one -# build label never suppresses a genuine regression on another label of the -# same push, while a revision that breaks many builds still triggers only once. -# Messages are handled on worker threads, so the check-and-record is done under -# a lock. +# Taskcluster ``kind`` tags that denote test tasks (vs build tasks). +TEST_KINDS = {"test", "mochitest", "web-platform-tests", "source-test"} + +# In-memory dedupe of hg revisions already handed to the build-repair agent. A +# revision is recorded only once we actually trigger a run, so an inherited +# failure on one build label never suppresses a genuine regression on another +# label of the same push, while a revision that breaks many builds still triggers +# only once. Messages are handled on worker threads, so the check-and-record is +# done under a lock. _seen: TTLCache = TTLCache( maxsize=settings.dedupe_max_size, ttl=settings.dedupe_ttl_seconds ) _seen_lock = threading.Lock() +# Independent dedupe for test-repair runs, keyed by hg revision: one run per push, +# on the first of its failing tasks worth investigating. The agent works from a single +# task but reads the push's other failures from Treeherder, so a second run for the +# same push would re-tread the same ground. As on the build path a revision is +# recorded only once a run is actually triggered, so a task rejected as intermittent +# or inherited never suppresses a genuine regression on another task of the push. +_seen_tests: TTLCache = TTLCache( + maxsize=settings.dedupe_max_size, ttl=settings.dedupe_ttl_seconds +) +_seen_tests_lock = threading.Lock() + + +def _is_test_task(tags: dict) -> bool: + return tags.get("kind") in TEST_KINDS or bool(tags.get("test-suite")) + def process(body: dict, executor: Executor) -> str | None: """Handle one Taskcluster failure message. Returns the triggered run id.""" tags = (body.get("task") or {}).get("tags") or {} - task_label = tags.get("label") or "" - if "build" not in task_label or "test" in task_label: - return None - project = tags.get("project") if project not in settings.watched_repos_set: + logger.debug("Ignoring failure on unwatched project %s", project) return None + task_label = tags.get("label") or "" + if "build" in task_label and "test" not in task_label: + return _process_build(body, tags, executor) + if _is_test_task(tags): + return _process_test(body, tags, executor) + logger.debug("Ignoring non-build, non-test task %s", task_label) + return None + + +def _release(cache: TTLCache, lock: threading.Lock, keys) -> None: + """Give up claimed dedupe keys so a later message can retry them.""" + with lock: + for key in keys: + cache.pop(key, None) + + +def _process_build(body: dict, tags: dict, executor: Executor) -> str | None: + """Build-failure path: trigger the build-repair agent.""" + project = tags.get("project") + task_label = tags.get("label") or "" task_id = body["status"]["taskId"] task_name = tags.get("label") or task_id developer_email = tags.get("createdForUser") @@ -93,30 +128,24 @@ def process(body: dict, executor: Executor) -> str | None: task_id, project, ) - with _seen_lock: - _seen.pop(hg_revision, None) + _release(_seen, _seen_lock, [hg_revision]) return None - inputs: dict = { - "failure_tasks": {task_name: task_id}, - "run_try_push": settings.run_try_push, - } - if settings.model: - inputs["model"] = settings.model - if settings.max_turns is not None: - inputs["max_turns"] = settings.max_turns - try: - run_id = client.trigger_run(inputs) + run_id = client.trigger_run( + { + "failure_tasks": {task_name: task_id}, + "run_try_push": settings.run_try_push, + } + ) except Exception: logger.exception("Failed to trigger build-repair run for %s", hg_revision) - with _seen_lock: - _seen.pop(hg_revision, None) + _release(_seen, _seen_lock, [hg_revision]) return None logger.info( - "Triggered build-repair run %s for %s@%s (git %s)", - run_id, + "%s build-repair for %s@%s (git %s)", + f"Triggered run {run_id}" if run_id else "Would trigger", project, hg_revision, git_commit, @@ -134,6 +163,204 @@ def process(body: dict, executor: Executor) -> str | None: return run_id +def _process_test(body: dict, tags: dict, executor: Executor) -> str | None: + """Test-failure path: filter, then trigger the test-repair agent for the task. + + One push emits many failing test tasks. We wait for Treeherder's verdict on this + one, then keep only the failing groups this push introduced, and trigger a single + run for the task -- the agent resolves the failing tests and the commit range + itself from the task id. Dedupe is per push, on the first task worth investigating. + """ + status = body.get("status") or {} + task_id = status.get("taskId") + project = tags.get("project") + label = tags.get("label") or task_id + developer_email = tags.get("createdForUser") + + task = taskcluster.get_task(task_id) + + if taskcluster.is_action_scheduled(task): + logger.info( + "Task %s (%s) was scheduled by an action task, not by the push " + "(backfill or retrigger); skipping", + task_id, + label, + ) + return None + + hg_revision = taskcluster.get_hg_revision(task) + if not hg_revision: + logger.warning("No GECKO_HEAD_REV for test task %s; skipping", task_id) + return None + + if regression.is_stale_push( + project, hg_revision, settings.max_push_age_hours * 3600 + ): + return None + + # One run per push: a task whose push has already been handed off can stop before + # any Treeherder work. + if _push_claimed(hg_revision): + logger.info( + "Push %s already handed to test-repair; skipping task %s", + hg_revision, + task_id, + ) + return None + + # Cheapest gate first: it rules out intermittents and infra failures for every + # harness before any group resolution or ancestor walking. Treeherder classifies + # a few minutes after ingesting, so the verdict is waited for rather than read + # once. The same record carries the configuration the regression check compares + # against. + job = treeherder.job_for_task(project, task_id) + reason = treeherder.await_skip_reason(project, task_id, job) + if reason: + logger.info("Treeherder classified task %s as %s; skipping", task_id, reason) + return None + config = ((job or {}).get("platform"), (job or {}).get("platform_option")) + + trigger = (project, hg_revision, task_id, label, developer_email, executor) + try: + groups = treeherder.failing_groups(project, hg_revision, task_id) + except treeherder.GroupResultsUnavailable as exc: + # Routine: a task-level failure (crash, timeout, harness error) produces no + # per-manifest results, and a log may still be being parsed. Either way we + # cannot say what failed, so fail open rather than drop it. There is no + # manifest to compare against an ancestor, so the gate above is the only + # filter such a failure gets. + logger.info("%s; running the agent on the whole task", exc) + return _trigger_test_repair([], *trigger) + except Exception: + logger.exception( + "Could not read the failing groups of task %s; " + "running the agent on the whole task", + task_id, + ) + return _trigger_test_repair([], *trigger) + + if not groups: + logger.info("Task %s reported no failing test groups; skipping", task_id) + return None + + fresh = _fresh_groups(groups, project, hg_revision, config) + if not fresh: + logger.info("No new, non-intermittent groups for task %s; skipping", task_id) + return None + + # One last cheap look before spending a run. The gate above already waited for a + # verdict, so this catches one that landed during the walk: a sheriff's + # classification, or autoclassification once a retrigger came back green. + reason = treeherder.recheck_skip_reason(project, task_id) + if reason: + logger.info( + "Treeherder classified task %s as %s while it was being checked; skipping", + task_id, + reason, + ) + return None + + return _trigger_test_repair(fresh, *trigger) + + +def _push_claimed(hg_revision: str) -> bool: + """Whether a run has already been triggered for this push.""" + with _seen_tests_lock: + return hg_revision in _seen_tests + + +def _claim_push(hg_revision: str) -> bool: + """Claim the push, or False if another task claimed it first. + + Deliberately taken at the moment of triggering rather than before the checks: + sibling tasks may then repeat those checks, which is affordable because the + Treeherder results they read are cached per push, and it keeps a task that turns + out to be intermittent or inherited from claiming a push it will not investigate. + """ + with _seen_tests_lock: + if hg_revision in _seen_tests: + return False + _seen_tests[hg_revision] = True + return True + + +def _fresh_groups( + candidates: list[str], project: str, hg_revision: str, config: tuple[str, str] +) -> list[str]: + """The groups whose failure this push introduced.""" + new = regression.new_test_failures(project, hg_revision, config, candidates) + for group in candidates: + if group not in new: + logger.info( + "Group %s at %s inherited from an ancestor; skipping", + group, + hg_revision, + ) + return [group for group in candidates if group in new] + + +def _trigger_test_repair( + test_groups: list[str], + project: str, + hg_revision: str, + task_id: str, + label: str, + developer_email: str | None, + executor: Executor, +) -> str | None: + if not _claim_push(hg_revision): + logger.info( + "Push %s was claimed while task %s was being checked; skipping", + hg_revision, + task_id, + ) + return None + + try: + run_id = client.trigger_run( + {"failure_tasks": {label: task_id}}, + agent_name=settings.test_repair_agent_name, + ) + except Exception: + logger.exception("Failed to trigger test-repair run for task %s", task_id) + _release(_seen_tests, _seen_tests_lock, [hg_revision]) + return None + + logger.info( + "%s test-repair for %s task %s (%s) at %s", + f"Triggered run {run_id}" if run_id else "Would trigger", + project, + task_id, + f"{len(test_groups)} group(s)" if test_groups else "groups unresolved", + hg_revision, + ) + if run_id is not None: + git_commit = lando.hg_to_git(hg_revision) + if not git_commit: + # Not fatal, unlike on the build path: the agent works from the task id + # alone, and a revision Lando has not mirrored yet is routine for a + # just-landed push. The notification omits the git revision instead of + # linking to an empty commit. + logger.warning( + "Could not map hg revision %s to git for task %s; " + "the notification will omit the git revision", + hg_revision, + task_id, + ) + ctx = RunContext( + run_id=run_id, + repo=project, + git_commit=git_commit or "", + hg_revision=hg_revision, + task_id=task_id, + developer_email=developer_email, + agent=settings.test_repair_agent_name, + test_groups=list(test_groups), + ) + executor.submit(worker.poll_and_notify, ctx) + return run_id + + def make_handler(executor: Executor): def run(body: dict) -> None: try: diff --git a/services/hackbot-pulse-listener/app/logging_setup.py b/services/hackbot-pulse-listener/app/logging_setup.py new file mode 100644 index 0000000000..fbbb6fab71 --- /dev/null +++ b/services/hackbot-pulse-listener/app/logging_setup.py @@ -0,0 +1,57 @@ +"""Logging configuration, applied on import. + +Import this before anything that pulls in mozci. mozci logs through loguru, +which writes straight to stderr in its own format and ignores stdlib logging +levels, and it emits lines while being imported -- so configuring later would +both miss those lines and leave its per-task chatter unfiltered. +""" + +import logging + +from app.config import settings + + +def _forward_to_stdlib(message) -> None: + """Re-emit a loguru record through stdlib logging (levels are numbered alike).""" + record = message.record + logging.getLogger(record["name"] or "mozci").log( + record["level"].no, record["message"] + ) + + +def configure() -> None: + logging.basicConfig( + level=settings.log_level.upper(), + format="%(asctime)s %(levelname)-7s %(name)s: %(message)s", + datefmt="%Y-%m-%d %H:%M:%S", + ) + # httpx logs every request at INFO, which drowns out our own lines. + logging.getLogger("httpx").setLevel(logging.WARNING) + _capture_loguru() + + +def _capture_loguru() -> None: + """Route mozci's loguru output into stdlib logging at ``mozci_log_level``. + + mozci warns per task about groups missing from a task's errorsummary, dozens + of times a minute; that is inherent to how it reads CI data and is not + actionable here. Importing ``mozci`` runs its own ``setup_logging()``, which + removes every loguru sink and installs its own on stderr, so the import is + bracketed: drop the default sink first (silencing what mozci logs while being + imported), then take the sink back from it. + """ + try: + from loguru import logger as loguru_logger + except ImportError: + return + + loguru_logger.remove() + try: + import mozci # noqa: F401 + except ImportError: + pass + loguru_logger.remove() + loguru_logger.add(_forward_to_stdlib, level=settings.mozci_log_level.upper()) + + +configure() diff --git a/services/hackbot-pulse-listener/app/models.py b/services/hackbot-pulse-listener/app/models.py index 2bcaf33cfa..3863ee2fdb 100644 --- a/services/hackbot-pulse-listener/app/models.py +++ b/services/hackbot-pulse-listener/app/models.py @@ -1,9 +1,9 @@ -from dataclasses import dataclass +from dataclasses import dataclass, field @dataclass class RunContext: - """What the notifier needs about a triggered build-repair run.""" + """What the notifier needs about a triggered agent run.""" run_id: str repo: str @@ -11,3 +11,7 @@ class RunContext: hg_revision: str task_id: str developer_email: str | None + # Which agent produced the run, and (for test-repair) every failing test group + # the run covers. These drive the notifier's recipient/body routing. + agent: str = "build-repair" + test_groups: list[str] = field(default_factory=list) diff --git a/services/hackbot-pulse-listener/app/notify.py b/services/hackbot-pulse-listener/app/notify.py index 2e2f8aab6f..a22a1cbe14 100644 --- a/services/hackbot-pulse-listener/app/notify.py +++ b/services/hackbot-pulse-listener/app/notify.py @@ -13,14 +13,22 @@ def send_email(ctx: RunContext, run_doc: dict) -> None: - """Email the developer and team the build failure analysis. + """Email the failure analysis. Only succeeded runs are notified. - Only succeeded runs are notified. + Routes on the agent that produced the run: test-repair sends a + verdict-led body to the test-repair notification address; build-repair keeps its + existing behavior. """ if run_doc.get("status") != "succeeded": logger.info("Run %s did not succeed; skipping notification", ctx.run_id) return + if ctx.agent == settings.test_repair_agent_name: + _send_test_repair_email(ctx, run_doc) + else: + _send_build_repair_email(ctx, run_doc) + +def _send_build_repair_email(ctx: RunContext, run_doc: dict) -> None: patch = _fetch_patch(ctx.run_id, run_doc) if settings.notify_only_with_patch and not patch: logger.info("Run %s produced no patch; skipping notification", ctx.run_id) @@ -37,6 +45,42 @@ def send_email(ctx: RunContext, run_doc: dict) -> None: logger.info("SendGrid not configured; skipping email for run %s", ctx.run_id) return + subject = ( + f"[build-repair] Build failure analysis for {ctx.repo}@{ctx.git_commit[:12]}" + ) + body_md = _build_body(ctx, run_doc, patch, blamed_author) + _deliver(subject, body_md, recipients, patch) + + +def _send_test_repair_email(ctx: RunContext, run_doc: dict) -> None: + findings = (run_doc.get("summary") or {}).get("findings") or {} + culprit = findings.get("culprit_commit") + culprit_author = ( + github.commit_author_email(culprit) + if culprit and findings.get("classification") == "regression" + else None + ) + # test-repair verdicts are always notified (including do-not-backout verdicts), so + # the build-repair notify_only_with_patch gate does not apply here. + recipients = _recipients(settings.test_repair_notification_email, culprit_author) + if not recipients: + logger.info( + "No recipients for test-repair run %s; skipping notification", ctx.run_id + ) + return + if not (settings.sendgrid_api_key and settings.notification_sender): + logger.info("SendGrid not configured; skipping email for run %s", ctx.run_id) + return + + patch = _fetch_patch(ctx.run_id, run_doc) + subject = ( + f"[test-repair] {_banner(findings)} - {_test_groups_label(ctx)} ({ctx.repo})" + ) + body_md = _build_test_repair_body(ctx, findings, patch, culprit_author) + _deliver(subject, body_md, recipients, patch) + + +def _deliver(subject: str, body_md: str, recipients: list[str], patch: str | None): import markdown2 import sendgrid from sendgrid.helpers.mail import ( @@ -55,13 +99,7 @@ def send_email(ctx: RunContext, run_doc: dict) -> None: To, ) - subject = ( - f"[build-repair] Build failure analysis for {ctx.repo}@{ctx.git_commit[:12]}" - ) - - body_md = _build_body(ctx, run_doc, patch, blamed_author) html = markdown2.markdown(body_md, extras=["fenced-code-blocks", "tables"]) - sg = sendgrid.SendGridAPIClient(api_key=settings.sendgrid_api_key) to_emails = [To(recipients[0])] + [Cc(addr) for addr in recipients[1:]] message = Mail( @@ -82,31 +120,134 @@ def send_email(ctx: RunContext, run_doc: dict) -> None: message.reply_to = ReplyTo(settings.notification_team_email) response = sg.send(message=message) logger.info( - "Sent build-repair notification to %s (status %s)", + "Sent notification to %s (status %s)", ", ".join(recipients), response.status_code, ) -def _recipients( - author_email: str | None, developer_email: str | None = None -) -> list[str]: - """Recipients for a run, deduped and ordered by priority. +def _recipients(primary: str | None, secondary: str | None = None) -> list[str]: + """Recipients for a run, deduped and ordered by priority, team address last. - The blamed commit's author is the primary To (the person who introduced the - failure), then the pushing developer, then the team address. - ``notification_override_email`` short-circuits to a single address so local - testing never mails real developers or the team. + build-repair puts the blamed commit's author first and the pushing developer + second; test-repair puts its distribution address first and the culprit author + second. ``notification_override_email`` short-circuits to a single address so + local testing never mails real developers or the team. """ if settings.notification_override_email: return [settings.notification_override_email] recipients: list[str] = [] - for addr in (author_email, developer_email, settings.notification_team_email): + for addr in (primary, secondary, settings.notification_team_email): if addr and addr not in recipients: recipients.append(addr) return recipients +_RECOMMENDATION_BANNER = { + "backout": "BACK OUT the culprit", + "do_not_backout": "DO NOT back out (intermittent)", + "land_fix": "LAND the proposed fix", +} + + +def _banner(findings: dict) -> str: + """The recommendation as a human-readable headline.""" + recommendation = findings.get("recommendation") + return _RECOMMENDATION_BANNER.get(recommendation, recommendation or "analysis") + + +def _test_groups_label(ctx: RunContext) -> str: + """A one-line name for the run's failing groups, for the email subject.""" + if not ctx.test_groups: + return f"task {ctx.task_id}" + first, *rest = ctx.test_groups + return f"{first} (+{len(rest)} more)" if rest else first + + +def _build_test_repair_body( + ctx: RunContext, + findings: dict, + patch: str | None, + culprit_author: str | None, +) -> str: + groups = ", ".join(f"`{g}`" for g in ctx.test_groups) or "not resolved" + lines = [ + "# Test failure analysis", + "", + f"- **Recommendation:** {_banner(findings)}", + f"- **Failing tests:** {groups}", + f"- **Classification:** {findings.get('classification')}", + f"- **Repository:** {ctx.repo}", + ] + # Omitted rather than linked as an empty commit when Lando has not mirrored the + # revision yet; the hg revision below always identifies the push. + if ctx.git_commit: + lines.append( + f"- **Revision (git):** [`{ctx.git_commit[:12]}`]({_git_url(ctx.git_commit)})" + ) + lines += [ + f"- **Revision (hg):** [`{ctx.hg_revision[:12]}`]({_hg_url(ctx.hg_revision)})", + f"- **Failed task:** [`{ctx.task_id}`]({_task_url(ctx.task_id)})", + f"- **Treeherder:** " + f"[jobs]({_treeherder_url(ctx.repo, ctx.hg_revision, ctx.task_id)})", + ] + + confidence = findings.get("confidence") + if confidence is not None: + lines.append(f"- **Confidence:** {confidence}") + + culprit = findings.get("culprit_commit") + if culprit: + by = f" by {culprit_author}" if culprit_author else "" + lines.append( + f"- **Culprit commit:** [`{culprit[:12]}`]({_git_url(culprit)}){by}" + ) + + last_green = findings.get("last_green_revision") + if last_green: + lines.append(f"- **Last green revision:** `{last_green}`") + + bug = findings.get("culprit_bug") + if bug: + lines.append(f"- **Bug:** [{bug}]({_bug_url(bug)})") + + lines += _run_details(ctx) + _analysis_sections(findings) + _patch_section(patch) + lines += _team_footer() + return "\n".join(lines) + + +def _run_details(ctx: RunContext) -> list[str]: + if not settings.hackbot_ui_url: + return [] + return [ + f"- **Run details:** {settings.hackbot_ui_url.rstrip('/')}/runs/{ctx.run_id}" + ] + + +def _analysis_sections(findings: dict) -> list[str]: + lines: list[str] = [] + for key, title in (("summary", "Summary"), ("analysis", "Analysis")): + if findings.get(key): + lines += ["", f"## {title}", "", _demote_headings(findings[key])] + return lines + + +def _patch_section(patch: str | None) -> list[str]: + return ["", "## Proposed patch", "", _patch_block(patch)] if patch else [] + + +def _team_footer() -> list[str]: + if not settings.notification_team_email: + return [] + return [ + "", + "---", + "", + "_Reply to this email with any feedback on this analysis; it reaches " + "the hackbot team._", + ] + + def _fetch_patch(run_id: str, run_doc: dict) -> str | None: """Download the proposed-fix patch artifact, if the run produced one.""" artifacts = run_doc.get("artifacts") or [] @@ -174,16 +315,9 @@ def _build_body( if bug_id: lines.append(f"- **Bug:** [{bug_id}]({_bug_url(bug_id)})") - if settings.hackbot_ui_url: - run_url = f"{settings.hackbot_ui_url.rstrip('/')}/runs/{ctx.run_id}" - lines.append(f"- **Run details:** {run_url}") - + lines += _run_details(ctx) lines += _recipients_note(ctx, blamed_commit, blamed_author) - - if findings.get("summary"): - lines += ["", "## Summary", "", _demote_headings(findings["summary"])] - if findings.get("analysis"): - lines += ["", "## Analysis", "", _demote_headings(findings["analysis"])] + lines += _analysis_sections(findings) if findings.get("local_build_verified") is not None: lines += [ @@ -193,18 +327,7 @@ def _build_body( f"- Local build verified: {findings['local_build_verified']}", ] - if patch: - lines += ["", "## Proposed patch", "", _patch_block(patch)] - - if settings.notification_team_email: - lines += [ - "", - "---", - "", - "_Reply to this email with any feedback on this analysis; it reaches " - "the hackbot team._", - ] - + lines += _patch_section(patch) + _team_footer() return "\n".join(lines) diff --git a/services/hackbot-pulse-listener/app/regression.py b/services/hackbot-pulse-listener/app/regression.py index 1aef366ec3..6eadcfa92a 100644 --- a/services/hackbot-pulse-listener/app/regression.py +++ b/services/hackbot-pulse-listener/app/regression.py @@ -1,154 +1,209 @@ +"""Whether a CI failure is new at a push or inherited from an ancestor. + +The ancestor chain comes from mozci, which resolves it from the hg pushlog -- +Treeherder itself uses mozci for exactly this. The per-push results come from +Treeherder, which keeps them small: mozci would instead pull the push's whole task +list from the Taskcluster queue, tens of megabytes per ancestor. + +A build is looked up by its task label, which is stable across pushes. A test group +is looked up by configuration (platform and build option) instead, because a test +label carries its chunk number and chunk assignments drift between pushes. +""" + import logging import time from mozci.errors import ParentPushNotFound from mozci.push import MAX_DEPTH, Push -logger = logging.getLogger(__name__) +from app import treeherder +logger = logging.getLogger(__name__) -# When the nearest ancestor that ran the build has not produced a decisive -# result yet, wait for it in-process instead of racing ahead and misreporting an -# inherited failure as new. A build can take tens of minutes and may hit an -# infra exception and be auto-retried, so we poll until it settles or the -# deadline elapses (well above a normal build + one retry). Fails open after the -# deadline so a real regression is never silently dropped. +# Poll an unsettled ancestor for up to MAX_WAIT_SECONDS before giving up and running +# the agent. An ancestor that has not settled within ten minutes is usually not about +# to: the walk either resolves in a few polls or the run it waits on is itself stuck, +# and holding the decision longer only delays the repair. POLL_INTERVAL_SECONDS = 120 -MAX_WAIT_SECONDS = 60 * 60 - -# A green build; mozci reports it as "passed" (Taskcluster) or "success" -# (Treeherder). Failures are read from mozci's Task.failed attribute instead of -# reimplementing the vocabulary. -_PASSED_RESULTS = ("passed", "success") - -# A build in one of these states, or with one of these (infra) results, has not -# settled: it is still running or was retried after an exception, so its outcome -# is not knowable yet and we wait for it. Anything else that is not a decisive -# pass/fail (unscheduled, canceled, superseded, ...) or a build that never ran -# at all (coalesced) is treated as non-decisive and skipped. -_UNSETTLED_STATES = ("pending", "running", "exception") -_UNSETTLED_RESULTS = ("exception", "retry") - -# Sentinel meaning the decision cannot be made yet because an ancestor build has -# not settled; the caller waits and re-checks. -_PENDING = object() +MAX_WAIT_SECONDS = 60 * 10 +# Treeherder job results and states. +_PASSED_RESULTS = ("success",) +_FAILED_RESULTS = ("testfailed", "busted") +# Outcomes that aren't knowable yet: still queued or running, or awaiting an +# auto-retry after an infra exception. +_UNSETTLED_STATES = ("pending", "running") +_UNSETTLED_RESULTS = ("retry", "exception", "unknown") -def _build_status(push: Push, label: str): - """Return 'passed', 'failed', _PENDING, or None for a build label on a push. +# The deciding ancestor hasn't settled yet; the caller waits and re-checks. +_PENDING = object() - 'passed'/'failed' come from a run with a decisive result. _PENDING means the - result is not knowable yet: a task exists but has not settled (still running, - or exceptioned and awaiting a retry), or no task is visible yet but the build - was scheduled to run on this push (its result has not propagated). None means - the build produced no decisive result and none is coming: it was coalesced / - never scheduled, or it only reached a non-decisive terminal state. None is - deliberately non-decisive so coalescing gaps are skipped and never suppress a - real regression. Retriggers are collapsed: any green run means 'passed'; only - a genuine build failure counts as 'failed'. - """ - label_tasks = [t for t in push.tasks if t.label == label] - if label_tasks: - if any(t.result in _PASSED_RESULTS for t in label_tasks): - return "passed" - # Checked before failure: a retrigger that is still running or was - # exceptioned and auto-retried may yet turn green, and any green run wins, - # so we wait for it rather than prematurely inheriting a failure. - if any( - t.state in _UNSETTLED_STATES or t.result in _UNSETTLED_RESULTS - for t in label_tasks - ): - return _PENDING - # Not "not t.failed": a run can be neither passed nor failed (canceled, - # superseded, ...). mozci's Task.failed also counts `exception`, but those - # are unsettled and already returned above, so only genuine build failures - # reach here. - if any(t.failed for t in label_tasks): - return "failed" - # Ran but reached a non-decisive terminal state (canceled, ...): skip. - return None - - # No task for this label on the push. Distinguish a build that was scheduled - # to run here but whose result is not visible yet (wait) from one that was - # never scheduled / coalesced away (skip), using the decision task's - # scheduled set (available well before the builds themselves finish). - try: - scheduled = label in push.scheduled_task_labels - except Exception: - logger.debug("Could not read scheduled task labels for %s", push.rev) - scheduled = False - return _PENDING if scheduled else None +def _unsettled(jobs: list[dict]) -> bool: + """Whether any of these runs may still change outcome.""" + return any( + job["state"] in _UNSETTLED_STATES or job["result"] in _UNSETTLED_RESULTS + for job in jobs + ) -def _classify(branch: str, rev: str, label: str): - """Walk ancestors once. Returns True (new), False (inherited), or _PENDING. - A fresh Push is built each call so re-checks re-fetch live data (recent - pushes are not finalized in mozci, so their tasks are never served from - cache). +def _build_status(project: str, rev: str, label: str): + """'passed'/'failed'/_PENDING/None for a build label on a push. + + None is non-decisive (never ran here, or a non-pass/fail terminal state), so such + gaps are skipped rather than mistaken for an inherited failure. + """ + jobs = treeherder.label_jobs(project, rev, label) + if any(job["result"] in _PASSED_RESULTS for job in jobs): + return "passed" + # Checked before failure: a still-running or auto-retried run may yet turn green, + # and any green run wins, so wait rather than inherit prematurely. + if _unsettled(jobs): + return _PENDING + if any(job["result"] in _FAILED_RESULTS for job in jobs): + return "failed" + return None + + +def _group_status(project: str, rev: str, config: tuple[str, str], group: str): + """'passed'/'failed'/_PENDING/None for a test group on a push, in one config. + + Only this configuration's runs are consulted, so a manifest already broken on + another platform cannot mask a genuine new failure on this one. Same precedence + as _build_status: pass, then pending, then fail. + """ + jobs = treeherder.config_jobs(project, rev, *config) + results = treeherder.group_results(project, rev) + recorded = [ + results[job["task_id"]][group] + for job in jobs + if group in (results.get(job["task_id"]) or {}) + ] + if any(recorded): + return "passed" + if _unsettled(jobs): + return _PENDING + if recorded: + return "failed" + + # Nothing recorded for the group: this push never ran it (coalesced, or the + # manifest was chunked into another task). + return None + + +def _classify(project: str, rev: str, status_fn, describe: str, first_pass=True): + """Walk ancestors of `rev`; True (new), False (inherited) or _PENDING. + + status_fn(ancestor_rev) reports 'passed'/'failed'/_PENDING/None for the failing + unit (build label or test group). `first_pass` keeps the "not settled" notice to + one line per unit rather than repeating it on every poll for up to an hour. """ - ancestor = Push(rev, branch=branch) + push = Push(rev, branch=project) for _ in range(MAX_DEPTH): try: - ancestor = ancestor.parent + push = push.parent except ParentPushNotFound: break - status = _build_status(ancestor, label) + status = status_fn(push.rev) if status is None: continue if status is _PENDING: - logger.info( - "Build %s not settled yet at %s; deferring decision for %s", - label, - ancestor.rev, - rev, - ) + notice = logger.info if first_pass else logger.debug + notice("%s not settled at %s; deferring for %s", describe, push.rev, rev) return _PENDING if status == "failed": logger.info( - "Build %s already failing at %s; inherited failure at %s", - label, - ancestor.rev, - rev, + "%s already failing at %s; inherited at %s", describe, push.rev, rev ) return False - logger.info( - "Build %s passed at %s; new failure introduced at %s", - label, - ancestor.rev, - rev, - ) + logger.info("%s passed at %s; new failure at %s", describe, push.rev, rev) return True logger.warning( - "No ancestor within %s pushes ran build %s; running agent", MAX_DEPTH, label + "No ancestor within %s pushes ran %s; running agent", MAX_DEPTH, describe ) return True -def is_stale_push(branch: str, rev: str, max_age_seconds: float) -> bool: - """Return True if the push landed more than ``max_age_seconds`` ago. +def _await_new_failures(project: str, rev: str, status_fn, units, describe: str) -> set: + """The units whose failure `rev` introduced; the rest were inherited. + + status_fn(ancestor_rev, unit) reports 'passed'/'failed'/_PENDING/None. - A build task can fail long after its push -- a backfill scheduled weeks later, - a task that sat queued, a listener restart replaying an old message -- and - repairing a push that has long since been superseded helps nobody. Fails open - (returns False) when the push date cannot be read, so a real regression is - never silently dropped. + Fails open -- undecided units counted as new -- on any error, an ancestor still + unsettled past MAX_WAIT_SECONDS, or no deciding ancestor within MAX_DEPTH, so a + real regression is never silently dropped. + """ + deadline = time.monotonic() + MAX_WAIT_SECONDS + unresolved = list(units) + new: set = set() + first_pass = True + while True: + try: + pending = [] + for unit in unresolved: + state = _classify( + project, + rev, + lambda ancestor, u=unit: status_fn(ancestor, u), + f"{describe} {unit}", + first_pass=first_pass, + ) + if state is _PENDING: + pending.append(unit) + elif state: + new.add(unit) + except Exception: + logger.exception( + "Regression check failed for %s@%s; running agent", describe, rev + ) + return new | set(unresolved) + + if not pending: + return new + if time.monotonic() >= deadline: + logger.warning( + "%s still unsettled after %ss at %s; running agent", + describe, + MAX_WAIT_SECONDS, + rev, + ) + return new | set(pending) + # _classify already logged which ancestor is unsettled; keep the per-poll + # heartbeat at debug so a long wait isn't dozens of lines. + logger.debug( + "Waiting %ss for an unsettled ancestor of %s (%s)", + POLL_INTERVAL_SECONDS, + describe, + rev, + ) + time.sleep(POLL_INTERVAL_SECONDS) + unresolved = pending + first_pass = False + + +def is_stale_push(project: str, rev: str, max_age_seconds: float) -> bool: + """Whether the push landed more than ``max_age_seconds`` ago. + + A task can fail long after its push -- a backfill scheduled weeks later, a task + that sat queued, a listener restart replaying an old message -- and repairing a + push that has long since been superseded helps nobody. Fails open (returns False) + when the push date cannot be read, so a real regression is never silently dropped. """ try: # Push.date is in seconds since the epoch, despite what mozci's docstring # says: it is hgmo's `pushdate[0]`, which is a Unix timestamp. - age = time.time() - Push(rev, branch=branch).date + age = time.time() - Push(rev, branch=project).date except Exception: logger.exception( - "Could not read the push date for %s@%s; running agent", branch, rev + "Could not read the push date for %s@%s; running agent", project, rev ) return False if age > max_age_seconds: logger.info( "Push %s@%s landed %.1fh ago (limit %.1fh); skipping", - branch, + project, rev, age / 3600, max_age_seconds / 3600, @@ -157,41 +212,32 @@ def is_stale_push(branch: str, rev: str, max_age_seconds: float) -> bool: return False -def is_new_build_failure(branch: str, rev: str, label: str) -> bool: - """Return True if this push introduced the failure, False if it inherited it. +def is_new_build_failure(project: str, rev: str, label: str) -> bool: + """True if this push introduced the build failure, False if it inherited it.""" + return label in _await_new_failures( + project, + rev, + lambda ancestor, unit: _build_status(project, ancestor, unit), + [label], + "build", + ) - Walks back over pushes that did not run the build (coalescing) until it - finds the nearest ancestor that did. When that ancestor's build has not - settled yet, waits in-process and re-checks until it produces a decisive - result or MAX_WAIT_SECONDS elapses, rather than racing ahead and - misreporting an inherited failure as new. Fails open (returns True) on any - mozci/network error, if the build stays unsettled past the deadline, or if - no ancestor within MAX_DEPTH ran the build, so we never silently drop a real - regression. - """ - try: - deadline = time.monotonic() + MAX_WAIT_SECONDS - while True: - result = _classify(branch, rev, label) - if result is not _PENDING: - return result - if time.monotonic() >= deadline: - break - logger.info( - "Waiting %ss for an unsettled ancestor build of %s (%s)", - POLL_INTERVAL_SECONDS, - rev, - label, - ) - time.sleep(POLL_INTERVAL_SECONDS) - except Exception: - logger.exception("Regression check failed for %s@%s; running agent", label, rev) - return True - logger.warning( - "Build %s still unsettled after %ss at %s; running agent", - label, - MAX_WAIT_SECONDS, +def new_test_failures( + project: str, rev: str, config: tuple[str, str], groups: list[str] +) -> set[str]: + """The failing groups this push introduced, for one configuration. + + `config` is (platform, platform_option). With no configuration to compare + against, every group is reported as new rather than silently dropped. + """ + if not all(config): + logger.info("No configuration for %s; running agent", rev) + return set(groups) + return _await_new_failures( + project, rev, + lambda ancestor, group: _group_status(project, ancestor, config, group), + groups, + "group", ) - return True diff --git a/services/hackbot-pulse-listener/app/treeherder.py b/services/hackbot-pulse-listener/app/treeherder.py new file mode 100644 index 0000000000..dd74c83d51 --- /dev/null +++ b/services/hackbot-pulse-listener/app/treeherder.py @@ -0,0 +1,292 @@ +"""Treeherder classification gate for test-repair. + +Treeherder ingests the same Taskcluster failures the listener sees and classifies +each failing job while parsing its log -- a cross-push analysis of per-manifest +pass rates, refined afterwards by sheriffs and by mozci's autoclassifier. Reading +that verdict is cheaper and broader than judging intermittency ourselves: it covers +every harness, where the tests.firefox.dev timings datasets it replaces published +only mochitest and xpcshell, leaving reftest and web-platform-tests unjudged. + +Ingestion trails the failure message (measured on autoland: ~50s median, ~3min at +p90), so the lookup waits for the job to appear before reading its classification. +""" + +from __future__ import annotations + +import logging +import threading +import time +from urllib.parse import quote + +import httpx +from cachetools import TTLCache + +from app.config import settings + +logger = logging.getLogger(__name__) + +_TIMEOUT = 15 + +# Read-through caches, so one ancestor walk fetches each push once instead of once +# per failing group, and the many failing tasks of a push share a fetch. The TTL is +# deliberately shorter than the regression check's poll interval, so a walk waiting +# on an unsettled ancestor still sees fresh results on its next attempt. +_TTL_SECONDS = 60 +_group_cache: TTLCache = TTLCache(maxsize=128, ttl=_TTL_SECONDS) +_jobs_cache: TTLCache = TTLCache(maxsize=512, ttl=_TTL_SECONDS) +# A revision never maps to a different push, so this one is held far longer. +_push_id_cache: TTLCache = TTLCache(maxsize=512, ttl=6 * 60 * 60) +_cache_lock = threading.Lock() + +# /api/failureclassification/ +_CLASSIFICATIONS = { + 1: "not classified", + 2: "fixed by commit", + 3: "expected fail", + 4: "intermittent", + 5: "infra", + 6: "new failure not classified", + 7: "autoclassified intermittent", + 8: "intermittent needs bugid", +} + +# Verdicts that mean this failure is not a new regression worth an agent run. +# The two left out -- "not classified" and "new failure not classified" -- are the +# ones that still may be a real regression. +_NOT_A_REGRESSION = {2, 3, 4, 5, 7, 8} + + +def _job(project: str, task_id: str) -> dict | None: + """The Treeherder job for a Taskcluster task, or None if not ingested yet. + + Keyed on the task id, which is the one identifier a failure message carries. + ``jobs/?revision=`` looks like it would work but silently ignores the filter + and returns unrelated jobs, so it must not be used here. + """ + url = ( + f"{settings.treeherder_url.rstrip('/')}/api/project/{project}/jobs/" + f"?task_id={task_id}" + ) + resp = httpx.get(url, timeout=_TIMEOUT, follow_redirects=True) + resp.raise_for_status() + results = resp.json().get("results") or [] + return results[0] if results else None + + +def _get(path: str) -> dict: + url = f"{settings.treeherder_url.rstrip('/')}/api/project/{path}" + resp = httpx.get(url, timeout=_TIMEOUT, follow_redirects=True) + resp.raise_for_status() + return resp.json() + + +def group_results( + project: str, revision: str, refresh: bool = False +) -> dict[str, dict[str, bool]]: + """``{task_id: {group: passed}}`` for a push, as Treeherder recorded it. + + Needs the full 40-character revision; a short one 404s. + + Note a group can be recorded as failing on a task whose job did not fail (wpt + expected-fail metadata), so this is only meaningful for a task already known to + have failed. + """ + key = (project, revision) + with _cache_lock: + if not refresh and key in _group_cache: + return _group_cache[key] + + results = _get(f"{project}/push/group_results/?revision={revision}") + with _cache_lock: + _group_cache[key] = results + return results + + +def push_id(project: str, revision: str) -> int | None: + """Treeherder's push id for a revision, or None if it has no push.""" + key = (project, revision) + with _cache_lock: + if key in _push_id_cache: + return _push_id_cache[key] + + results = _get(f"{project}/push/?revision={revision}").get("results") or [] + value = results[0]["id"] if results else None + with _cache_lock: + _push_id_cache[key] = value + return value + + +def _summarize(job: dict) -> dict: + return { + "result": job.get("result"), + "state": job.get("state"), + "task_id": job.get("task_id"), + "platform_option": job.get("platform_option"), + } + + +def _push_jobs(project: str, revision: str, query: str, key) -> list[dict]: + """Cached job query on a push. + + Pending and running jobs are included, which is how a not-yet-finished ancestor + is told apart from one that never ran the task at all. + """ + with _cache_lock: + if key in _jobs_cache: + return _jobs_cache[key] + + push = push_id(project, revision) + jobs = [] + if push is not None: + results = _get(f"{project}/jobs/?push_id={push}&{query}&count=2000") + jobs = [_summarize(job) for job in results.get("results") or []] + with _cache_lock: + _jobs_cache[key] = jobs + return jobs + + +def label_jobs(project: str, revision: str, label: str) -> list[dict]: + """The runs of one exact task label on a push, for build labels. + + Build labels carry no chunk number, so they are stable across pushes. + """ + return _push_jobs( + project, + revision, + f"job_type_name={quote(label, safe='')}", + (project, revision, "label", label), + ) + + +def config_jobs( + project: str, revision: str, platform: str, platform_option: str +) -> list[dict]: + """The runs of one configuration on a push -- platform plus build option. + + Not keyed on the task label: a test label carries its chunk number, and chunk + assignments drift between pushes, so comparing by label makes ancestors look as + though they never ran the manifest. Platform and option are stable, and a group + only appears on the tasks that actually ran it, so the group name itself selects + the right suite. + + ``platform_option`` is filtered here rather than in the query: Treeherder accepts + the parameter but silently ignores it. + """ + jobs = _push_jobs( + project, + revision, + f"platform={quote(platform, safe='')}", + (project, revision, "platform", platform), + ) + return [job for job in jobs if job["platform_option"] == platform_option] + + +class GroupResultsUnavailable(Exception): + """Treeherder records no per-manifest results for a task.""" + + +def failing_groups(project: str, revision: str, task_id: str) -> list[str]: + """The manifests this task reported as failing. + + An empty list means the task failed nothing at manifest level. Raises when the + task has no results at all -- which is not the same thing, and must not be + mistaken for it: a task-level failure (crash, timeout, harness error) or a log + that is still being parsed would otherwise be silently dropped. Network errors + propagate for the same reason. + """ + results = group_results(project, revision) + if task_id not in results: + # A cached snapshot can predate this task's ingestion; refetch once before + # concluding that Treeherder has nothing for it. + results = group_results(project, revision, refresh=True) + if task_id not in results: + raise GroupResultsUnavailable( + f"Treeherder records no group results for task {task_id}" + ) + return [ + group + for group, passed in results[task_id].items() + if not passed and group.strip() and group != "/" + ] + + +def job_for_task(project: str, task_id: str) -> dict | None: + """The Treeherder job for a failing task, waiting for it to be ingested. + + None when Treeherder never ingests it or cannot be reached, which callers must + treat as "no verdict" rather than as a reason to drop the failure. + """ + deadline = time.monotonic() + settings.treeherder_ingest_max_wait_seconds + while True: + try: + job = _job(project, task_id) + except Exception: + logger.exception( + "Treeherder lookup failed for task %s; investigating", task_id + ) + return None + if job is not None: + return job + if time.monotonic() >= deadline: + logger.info( + "Treeherder has not ingested task %s after %ss; investigating", + task_id, + settings.treeherder_ingest_max_wait_seconds, + ) + return None + time.sleep(settings.treeherder_ingest_poll_seconds) + + +def await_skip_reason(project: str, task_id: str, job: dict | None) -> str | None: + """Wait a bounded time for a verdict that this failure is not worth a run. + + ``job`` carries the verdict as of ingestion, which on an intermittent is usually + still "not classified": Treeherder classifies a few minutes later. Waiting for it + here rejects such a failure before the caller's ancestor walk rather than after, + so the filter no longer depends on how long that walk happens to take. + + Returns None once the wait is spent, so an unclassified failure is investigated + rather than dropped, and at once for a task Treeherder holds no job for, since + then there is no verdict to wait for. + """ + if job is None: + return None + reason = skip_reason(job) + if reason: + return reason + + deadline = time.monotonic() + settings.treeherder_classification_wait_seconds + while time.monotonic() < deadline: + time.sleep(settings.treeherder_ingest_poll_seconds) + reason = recheck_skip_reason(project, task_id) + if reason: + return reason + return None + + +def recheck_skip_reason(project: str, task_id: str) -> str | None: + """Re-read the classification without waiting for ingestion. + + Classification of an intermittent usually lands after we first look, but a + regression check takes minutes, so it is worth asking again before spending a + run. Returns None on any error: a failed re-check must not drop the failure. + """ + try: + return skip_reason(_job(project, task_id)) + except Exception: + logger.exception( + "Treeherder re-check failed for task %s; investigating", task_id + ) + return None + + +def skip_reason(job: dict | None) -> str | None: + """Treeherder's reason not to investigate this failure, or None to proceed. + + Fails open (None) for a missing or unclassified job: an absent verdict must never + drop a real regression. + """ + classification = (job or {}).get("failure_classification_id") + if classification in _NOT_A_REGRESSION: + return _CLASSIFICATIONS.get(classification, str(classification)) + return None diff --git a/services/hackbot-pulse-listener/deploy.sh b/services/hackbot-pulse-listener/deploy.sh index 94913f00b1..8aed83e552 100755 --- a/services/hackbot-pulse-listener/deploy.sh +++ b/services/hackbot-pulse-listener/deploy.sh @@ -41,6 +41,8 @@ PULSE_USER="${PULSE_USER:?set PULSE_USER (https://pulseguardian.mozilla.org)}" WATCHED_REPOS="${WATCHED_REPOS:-autoland}" NOTIFICATION_SENDER="${NOTIFICATION_SENDER:?set NOTIFICATION_SENDER (verified SendGrid sender)}" NOTIFICATION_TEAM_EMAIL="${NOTIFICATION_TEAM_EMAIL:-}" +# Distribution address; primary recipient of test-repair verdicts. +TEST_REPAIR_NOTIFICATION_EMAIL="${TEST_REPAIR_NOTIFICATION_EMAIL:-}" SA_NAME="${SA_NAME:-hackbot-pulse-listener-run}" SA_EMAIL="${SA_EMAIL:-${SA_NAME}@${PROJECT}.iam.gserviceaccount.com}" @@ -104,6 +106,7 @@ ENV_VARS="${ENV_VARS},ENVIRONMENT=production" ENV_VARS="${ENV_VARS},PULSE_USER=${PULSE_USER},WATCHED_REPOS=${WATCHED_REPOS}" ENV_VARS="${ENV_VARS},NOTIFICATION_SENDER=${NOTIFICATION_SENDER}" ENV_VARS="${ENV_VARS},NOTIFICATION_TEAM_EMAIL=${NOTIFICATION_TEAM_EMAIL}" +ENV_VARS="${ENV_VARS},TEST_REPAIR_NOTIFICATION_EMAIL=${TEST_REPAIR_NOTIFICATION_EMAIL}" gcloud beta run worker-pools deploy "${SERVICE}" \ --image "${IMAGE}" \ diff --git a/services/hackbot-pulse-listener/tests/test_consumer.py b/services/hackbot-pulse-listener/tests/test_consumer.py index da5c1ae8d2..87b4387bbd 100644 --- a/services/hackbot-pulse-listener/tests/test_consumer.py +++ b/services/hackbot-pulse-listener/tests/test_consumer.py @@ -1,5 +1,6 @@ import json from pathlib import Path +from types import SimpleNamespace from unittest.mock import MagicMock, patch import pytest @@ -12,6 +13,7 @@ def setup_function(): consumer._seen.clear() + consumer._seen_tests.clear() @pytest.fixture(autouse=True) @@ -57,15 +59,45 @@ def _build_msg(task_id="ABC", project="autoland", label="build-linux64/opt"): } -def test_sample_messages_are_all_tests_and_skipped(): +def _test_msg( + task_id="TT", + project="autoland", + kind="mochitest", + label="test-linux1804-64/opt-mochitest-browser-chrome-1", + suite="mochitest-browser-chrome", + group_id="G1", +): + return { + "status": {"taskId": task_id, "taskGroupId": group_id}, + "runId": 0, + "task": { + "tags": { + "kind": kind, + "project": project, + "label": label, + "test-suite": suite, + "createdForUser": "dev@mozilla.com", + } + }, + } + + +def test_sample_messages_route_to_test_repair_not_build(): + # The captured samples are all test tasks. They now reach the test-repair path + # (they were ignored outright when the listener only handled builds), and none of + # them triggers the build-repair agent. executor = MagicMock() with ( - patch.object(consumer.taskcluster, "get_task") as get_task, + patch.object(consumer.taskcluster, "get_task", return_value=_task_def()), + patch.object(consumer.treeherder, "failing_groups", return_value=[]) as groups, + patch.object(consumer.treeherder, "job_for_task", return_value=None), + patch.object(consumer.treeherder, "await_skip_reason", return_value=None), patch.object(consumer.client, "trigger_run") as trigger, ): for body in _sample_bodies(): assert consumer.process(body, executor) is None - get_task.assert_not_called() + # At least the autoland samples were routed to the test-repair path. + assert groups.called trigger.assert_not_called() executor.submit.assert_not_called() @@ -285,6 +317,215 @@ def test_trigger_failure_releases_revision_for_retry(): assert trigger.call_count == 2 +_GROUP = "dom/base/test/mochitest.ini" + + +@pytest.fixture +def env(monkeypatch): + """The test-repair path's external seams, as named mocks. + + Defaults describe the happy path: one failing group, not intermittent, a new + regression, revision mappable, trigger succeeds. + """ + mocks = SimpleNamespace( + get_task=MagicMock(return_value=_task_def()), + failing_groups=MagicMock(return_value=[_GROUP]), + job_for_task=MagicMock( + return_value={ + "failure_classification_id": 6, + "platform": "linux1804-64", + "platform_option": "opt", + } + ), + recheck_skip_reason=MagicMock(return_value=None), + await_skip_reason=MagicMock(return_value=None), + new_test_failures=MagicMock( + side_effect=lambda p, r, label, groups: set(groups) + ), + hg_to_git=MagicMock(return_value="gitH"), + trigger_run=MagicMock(return_value="tr-1"), + executor=MagicMock(), + ) + monkeypatch.setattr(consumer.taskcluster, "get_task", mocks.get_task) + monkeypatch.setattr(consumer.treeherder, "failing_groups", mocks.failing_groups) + monkeypatch.setattr(consumer.treeherder, "job_for_task", mocks.job_for_task) + monkeypatch.setattr( + consumer.treeherder, "recheck_skip_reason", mocks.recheck_skip_reason + ) + monkeypatch.setattr( + consumer.treeherder, "await_skip_reason", mocks.await_skip_reason + ) + monkeypatch.setattr( + consumer.regression, "new_test_failures", mocks.new_test_failures + ) + monkeypatch.setattr(consumer.lando, "hg_to_git", mocks.hg_to_git) + monkeypatch.setattr(consumer.client, "trigger_run", mocks.trigger_run) + return mocks + + +def test_test_failure_triggers_rca_run(env): + run_id = consumer.process(_test_msg(), env.executor) + + assert run_id == "tr-1" + env.trigger_run.assert_called_once() + inputs = env.trigger_run.call_args.args[0] + assert env.trigger_run.call_args.kwargs["agent_name"] == "test-repair" + # The agent resolves the test, commit range and clone depth itself; the + # listener only hands it the failing task. + assert inputs["failure_tasks"] == { + "test-linux1804-64/opt-mochitest-browser-chrome-1": "TT" + } + assert "test_id" not in inputs + assert "candidate_commits" not in inputs + fn, ctx = env.executor.submit.call_args.args + assert fn is consumer.worker.poll_and_notify + assert ctx.agent == "test-repair" + assert ctx.test_groups == [_GROUP] + + +def test_treeherder_intermittent_skipped_before_any_walk(env): + # Treeherder's own verdict rules the failure out before any mozci work. + env.await_skip_reason.return_value = "intermittent" + assert consumer.process(_test_msg(), env.executor) is None + env.failing_groups.assert_not_called() + env.new_test_failures.assert_not_called() + env.trigger_run.assert_not_called() + + +def test_unclassified_failure_is_investigated(env): + # "not classified" / "new failure" leave the decision to the mozci walk. + env.await_skip_reason.return_value = None + assert consumer.process(_test_msg(), env.executor) == "tr-1" + env.new_test_failures.assert_called_once() + + +def test_inherited_test_group_skipped(env): + env.new_test_failures.side_effect = lambda *_: set() + assert consumer.process(_test_msg(), env.executor) is None + env.trigger_run.assert_not_called() + + +def test_one_run_per_push(env): + # The agent reads the push's other failures itself, so the first task worth + # investigating is enough; later failing tasks of the same push are skipped. + assert consumer.process(_test_msg(task_id="A"), env.executor) == "tr-1" + assert consumer.process(_test_msg(task_id="B"), env.executor) is None + env.trigger_run.assert_called_once() + + +def test_later_task_of_a_claimed_push_stops_before_treeherder(env): + assert consumer.process(_test_msg(task_id="A"), env.executor) == "tr-1" + env.job_for_task.reset_mock() + env.failing_groups.reset_mock() + assert consumer.process(_test_msg(task_id="B"), env.executor) is None + env.job_for_task.assert_not_called() + env.failing_groups.assert_not_called() + + +def test_no_failing_groups_skips(env): + env.failing_groups.return_value = [] + assert consumer.process(_test_msg(), env.executor) is None + env.trigger_run.assert_not_called() + + +def test_task_without_group_results_still_triggers_run(env): + # A task-level failure (crash, timeout) records no per-manifest results; that + # must not be mistaken for "nothing failed" and drop a real regression. + env.failing_groups.side_effect = consumer.treeherder.GroupResultsUnavailable( + "no group results for task ERR" + ) + assert consumer.process(_test_msg(task_id="ERR"), env.executor) == "tr-1" + env.trigger_run.assert_called_once() + # With no groups resolved there is nothing to filter or to name. + env.new_test_failures.assert_not_called() + _, ctx = env.executor.submit.call_args.args + assert ctx.test_groups == [] + + +def test_unreadable_group_results_triggers_once_per_push(env): + env.failing_groups.side_effect = RuntimeError("treeherder down") + consumer.process(_test_msg(task_id="A"), env.executor) + consumer.process(_test_msg(task_id="B"), env.executor) + env.trigger_run.assert_called_once() + + +def test_rejected_task_does_not_suppress_a_real_regression_on_the_push(env): + # The push is claimed only when a run is triggered, so a task rejected as + # inherited leaves the push open for the next failing task -- which may be the + # genuine regression. + env.new_test_failures.side_effect = [set(), {_GROUP}] + assert consumer.process(_test_msg(task_id="A"), env.executor) is None + assert consumer.process(_test_msg(task_id="B"), env.executor) == "tr-1" + env.trigger_run.assert_called_once() + + +def test_intermittent_task_does_not_suppress_the_next_task(env): + # Same for a task Treeherder has already classified: it must not claim a push + # it will not investigate. + env.await_skip_reason.side_effect = ["intermittent", None] + assert consumer.process(_test_msg(task_id="A"), env.executor) is None + assert consumer.process(_test_msg(task_id="B"), env.executor) == "tr-1" + env.trigger_run.assert_called_once() + + +def test_missing_git_mapping_still_triggers_run(env): + # Unlike a build failure, the run is still useful (the agent works from the + # task id), so a revision Lando has not mirrored yet must not drop it. + env.hg_to_git.return_value = None + assert consumer.process(_test_msg(), env.executor) == "tr-1" + env.trigger_run.assert_called_once() + _, ctx = env.executor.submit.call_args.args + assert ctx.git_commit == "" + + +def test_unwatched_project_test_skipped(env): + assert consumer.process(_test_msg(project="try"), env.executor) is None + env.get_task.assert_not_called() + env.failing_groups.assert_not_called() + + +def test_test_repair_trigger_failure_releases_group_for_retry(env): + env.trigger_run.side_effect = [RuntimeError("boom"), "tr-2"] + assert consumer.process(_test_msg(task_id="A"), env.executor) is None + assert consumer.process(_test_msg(task_id="B"), env.executor) == "tr-2" + assert env.trigger_run.call_count == 2 + + +def test_multiple_failing_groups_trigger_one_run_per_task(env): + groups = ["dom/base/test/mochitest.ini", "layout/test/mochitest.ini"] + env.failing_groups.return_value = groups + + assert consumer.process(_test_msg(), env.executor) == "tr-1" + # The whole task gets a single run; the agent investigates every failing group. + env.trigger_run.assert_called_once() + assert env.trigger_run.call_args.args[0]["failure_tasks"] == { + "test-linux1804-64/opt-mochitest-browser-chrome-1": "TT" + } + assert env.executor.submit.call_count == 1 + # Every failing group is named, not an arbitrary one of them. + _, ctx = env.executor.submit.call_args.args + assert ctx.test_groups == groups + + +def test_missing_hg_revision_skips_test_task(env): + env.get_task.return_value = _task_def(revision=None) + assert consumer.process(_test_msg(), env.executor) is None + # Bail before doing the (network-heavy) group resolution. + env.failing_groups.assert_not_called() + env.trigger_run.assert_not_called() + + +def test_every_failing_group_reaches_the_mozci_walk(env): + groups = ["a/mochitest.ini", "b/mochitest.ini"] + env.failing_groups.return_value = groups + env.new_test_failures.side_effect = lambda p, r, cfg, gs: {"b/mochitest.ini"} + + assert consumer.process(_test_msg(), env.executor) == "tr-1" + assert env.new_test_failures.call_args.args[3] == groups + _, ctx = env.executor.submit.call_args.args + assert ctx.test_groups == ["b/mochitest.ini"] + + def test_queue_name_includes_non_production_environment(): with patch.object(consumer.settings, "environment", "development"): (queue,) = consumer._build_queues("guest") @@ -295,3 +536,122 @@ def test_queue_name_omits_production_environment(): with patch.object(consumer.settings, "environment", "production"): (queue,) = consumer._build_queues("guest") assert queue.name == "queue/guest/build-repair-task-failed" + + +def test_classification_landing_during_the_check_cancels_the_run(env): + # The regression check takes minutes, which is about how long Treeherder needs + # to classify an intermittent; a verdict that arrives meanwhile must win. + env.recheck_skip_reason.return_value = "intermittent" + assert consumer.process(_test_msg(), env.executor) is None + env.trigger_run.assert_not_called() + + +def test_recheck_happens_after_the_regression_check(env): + order = [] + env.new_test_failures.side_effect = lambda p, r, cfg, gs: ( + order.append("walk"), + set(gs), + )[1] + env.recheck_skip_reason.side_effect = lambda p, t: order.append("recheck") + + assert consumer.process(_test_msg(), env.executor) == "tr-1" + assert order == ["walk", "recheck"] + + +def test_backfill_in_a_new_task_group_is_deduped(env): + # Backfills and retriggers are dispatched by action tasks, which start their own + # Taskcluster task group. The same push+group must still be investigated once. + assert consumer.process(_test_msg(task_id="A", group_id="G1"), env.executor) + assert ( + consumer.process(_test_msg(task_id="B", group_id="ACTION-GROUP"), env.executor) + is None + ) + env.trigger_run.assert_called_once() + + +def test_different_pushes_are_not_deduped(env): + # Dedupe is per push: the same manifest newly failing on a later push is a + # separate regression and must be investigated again. + revisions = iter(["rev-one", "rev-two"]) + env.get_task.side_effect = lambda task_id: _task_def(next(revisions)) + consumer.process(_test_msg(task_id="A"), env.executor) + consumer.process(_test_msg(task_id="B"), env.executor) + assert env.trigger_run.call_count == 2 + + +def test_verdict_is_awaited_before_resolving_groups(env): + # The wait for a verdict is the cheap gate, so it must come first: an intermittent + # should cost neither a group fetch nor an ancestor walk. + order = [] + env.await_skip_reason.side_effect = lambda p, t, j: order.append("await") + env.failing_groups.side_effect = lambda *_: (order.append("groups"), [_GROUP])[1] + + assert consumer.process(_test_msg(), env.executor) == "tr-1" + assert order == ["await", "groups"] + + +def test_the_verdict_is_awaited_once_on_every_path(env): + # A task with no group results used to run its own second wait; the up-front one + # covers it, and waiting twice would double the delay before a real repair. + env.failing_groups.side_effect = consumer.treeherder.GroupResultsUnavailable("none") + assert consumer.process(_test_msg(), env.executor) == "tr-1" + env.await_skip_reason.assert_called_once() + + +def test_group_less_intermittent_is_dropped_by_the_up_front_gate(env): + # The only filter such a failure gets, since it has no manifest to compare. + env.await_skip_reason.return_value = "intermittent" + env.failing_groups.side_effect = consumer.treeherder.GroupResultsUnavailable("none") + assert consumer.process(_test_msg(), env.executor) is None + env.trigger_run.assert_not_called() + + +def test_the_job_is_passed_to_the_verdict_wait(env): + # The wait needs the ingested job: it is the verdict as of ingestion, and without + # it the wait cannot tell "not classified yet" from "never ingested". + assert consumer.process(_test_msg(), env.executor) == "tr-1" + assert env.await_skip_reason.call_args.args[2] is env.job_for_task.return_value + + +def test_backfilled_test_task_is_skipped(env): + # Same rule as the build path: a backfill or retrigger re-runs work the push + # already scheduled, so it is not a new failure to investigate. + env.get_task.return_value = _task_def(parent="ACTION-CALLBACK") + assert consumer.process(_test_msg(), env.executor) is None + env.failing_groups.assert_not_called() + env.trigger_run.assert_not_called() + + +def test_stale_push_skips_a_test_failure(env, fresh_push): + # A test failure surfacing days after its push is not worth repairing either, + # and the check must precede the ancestor walk, which can block for an hour. + fresh_push.return_value = True + assert consumer.process(_test_msg(), env.executor) is None + fresh_push.assert_called_once_with( + "autoland", "hgrev", consumer.settings.max_push_age_hours * 3600 + ) + env.new_test_failures.assert_not_called() + env.trigger_run.assert_not_called() + + +def test_group_less_task_claims_the_push_for_every_path(env): + # One run per push whichever path triggered it: a task-level failure (no group + # results) and a manifest failure on the same push must not both run. + env.failing_groups.side_effect = [ + consumer.treeherder.GroupResultsUnavailable("none"), + [_GROUP], + ] + assert consumer.process(_test_msg(task_id="A"), env.executor) == "tr-1" + assert consumer.process(_test_msg(task_id="B"), env.executor) is None + env.trigger_run.assert_called_once() + + +def test_manifest_failure_claims_the_push_against_a_group_less_task(env): + # The reverse order must dedupe too. + env.failing_groups.side_effect = [ + [_GROUP], + consumer.treeherder.GroupResultsUnavailable("none"), + ] + assert consumer.process(_test_msg(task_id="A"), env.executor) == "tr-1" + assert consumer.process(_test_msg(task_id="B"), env.executor) is None + env.trigger_run.assert_called_once() diff --git a/services/hackbot-pulse-listener/tests/test_notify.py b/services/hackbot-pulse-listener/tests/test_notify.py index db7b1c50ed..d3e78ec932 100644 --- a/services/hackbot-pulse-listener/tests/test_notify.py +++ b/services/hackbot-pulse-listener/tests/test_notify.py @@ -18,6 +18,15 @@ def _ctx(**over): return RunContext(**base) +def settings_test_repair_address() -> str | None: + return notify.settings.test_repair_notification_email + + +def _test_repair_ctx(**over): + over.setdefault("test_groups", ["dom/base/test/mochitest.ini"]) + return _ctx(agent="test-repair", **over) + + def test_skips_without_recipient(): # No developer, no team, no override -> nothing to send, must not raise. notify.send_email(_ctx(developer_email=None), {"status": "succeeded"}) @@ -282,6 +291,160 @@ def test_recipients_dedupes_and_skips_empty(monkeypatch): assert notify._recipients(None) == [] +def _test_repair_findings(**over): + base = { + "classification": "regression", + "recommendation": "backout", + "culprit_commit": "abc123def456", + "confidence": 0.8, + "last_green_revision": "green99", + "summary": "A landed commit removed a null check.", + "analysis": "# Root cause\nThe diff dropped validation.", + } + base.update(over) + return base + + +def test_test_repair_body_leads_with_recommendation(): + body = notify._build_test_repair_body( + _test_repair_ctx(), _test_repair_findings(), None, "culprit@mozilla.com" + ) + assert "Test failure analysis" in body + assert "BACK OUT the culprit" in body + assert "dom/base/test/mochitest.ini" in body + assert "abc123def456"[:12] in body + assert "by culprit@mozilla.com" in body + assert "green99" in body + assert "## Analysis" in body + + +def test_test_repair_body_names_every_failing_group(): + ctx = _test_repair_ctx( + test_groups=["dom/base/test/mochitest.ini", "layout/test/mochitest.ini"] + ) + body = notify._build_test_repair_body(ctx, _test_repair_findings(), None, None) + assert "dom/base/test/mochitest.ini" in body + assert "layout/test/mochitest.ini" in body + + +def test_test_repair_subject_summarizes_multiple_groups(): + ctx = _test_repair_ctx(test_groups=["a/mochitest.ini", "b/mochitest.ini"]) + assert notify._test_groups_label(ctx) == "a/mochitest.ini (+1 more)" + assert ( + notify._test_groups_label(_test_repair_ctx()) == "dom/base/test/mochitest.ini" + ) + assert notify._test_groups_label(_test_repair_ctx(test_groups=[])) == "task TASK123" + + +def test_test_repair_body_omits_unmapped_git_revision(): + # An unmapped revision must not render an empty commit link. + body = notify._build_test_repair_body( + _test_repair_ctx(git_commit=""), _test_repair_findings(), None, None + ) + assert "Revision (git)" not in body + assert "firefox/commit/)" not in body + assert "Revision (hg)" in body + + +def test_test_repair_intermittent_body_says_do_not_backout(): + findings = _test_repair_findings( + classification="intermittent", + recommendation="do_not_backout", + culprit_commit=None, + ) + body = notify._build_test_repair_body(_test_repair_ctx(), findings, None, None) + assert "DO NOT back out" in body + assert "Culprit commit" not in body + + +def test_test_repair_recipients_address_then_culprit(monkeypatch): + monkeypatch.setattr(notify.settings, "notification_override_email", None) + monkeypatch.setattr( + notify.settings, "test_repair_notification_email", "test-repair@mozilla.com" + ) + monkeypatch.setattr(notify.settings, "notification_team_email", "team@mozilla.com") + assert notify._recipients( + settings_test_repair_address(), "culprit@mozilla.com" + ) == [ + "test-repair@mozilla.com", + "culprit@mozilla.com", + "team@mozilla.com", + ] + + +def test_test_repair_recipients_override_wins(monkeypatch): + monkeypatch.setattr( + notify.settings, "notification_override_email", "me@mozilla.com" + ) + monkeypatch.setattr( + notify.settings, "test_repair_notification_email", "test-repair@mozilla.com" + ) + assert notify._recipients( + settings_test_repair_address(), "culprit@mozilla.com" + ) == ["me@mozilla.com"] + + +def test_test_repair_intermittent_sends_without_patch(monkeypatch): + # No patch, notify_only_with_patch True -> test-repair still sends (unlike build-repair). + monkeypatch.setattr(notify.settings, "sendgrid_api_key", "key") + monkeypatch.setattr(notify.settings, "notification_sender", "from@mozilla.com") + monkeypatch.setattr(notify.settings, "notify_only_with_patch", True) + monkeypatch.setattr(notify.settings, "notification_override_email", None) + monkeypatch.setattr( + notify.settings, "test_repair_notification_email", "test-repair@mozilla.com" + ) + monkeypatch.setattr(notify.settings, "notification_team_email", None) + + run_doc = { + "status": "succeeded", + "summary": { + "findings": _test_repair_findings( + classification="intermittent", + recommendation="do_not_backout", + culprit_commit=None, + ) + }, + } + fake_client = MagicMock() + fake_client.send.return_value = MagicMock(status_code=202) + with patch("sendgrid.SendGridAPIClient", return_value=fake_client): + notify.send_email(_test_repair_ctx(), run_doc) + + fake_client.send.assert_called_once() + personalizations = fake_client.send.call_args.kwargs["message"].get()[ + "personalizations" + ][0] + assert personalizations["to"] == [{"email": "test-repair@mozilla.com"}] + + +def test_test_repair_regression_ccs_culprit_author(monkeypatch): + monkeypatch.setattr(notify.settings, "sendgrid_api_key", "key") + monkeypatch.setattr(notify.settings, "notification_sender", "from@mozilla.com") + monkeypatch.setattr(notify.settings, "notification_override_email", None) + monkeypatch.setattr( + notify.settings, "test_repair_notification_email", "test-repair@mozilla.com" + ) + monkeypatch.setattr(notify.settings, "notification_team_email", None) + + run_doc = {"status": "succeeded", "summary": {"findings": _test_repair_findings()}} + fake_client = MagicMock() + fake_client.send.return_value = MagicMock(status_code=202) + with ( + patch("sendgrid.SendGridAPIClient", return_value=fake_client), + patch.object( + notify.github, "commit_author_email", return_value="culprit@mozilla.com" + ) as author, + ): + notify.send_email(_test_repair_ctx(), run_doc) + + author.assert_called_once_with("abc123def456") + personalizations = fake_client.send.call_args.kwargs["message"].get()[ + "personalizations" + ][0] + assert personalizations["to"] == [{"email": "test-repair@mozilla.com"}] + assert personalizations["cc"] == [{"email": "culprit@mozilla.com"}] + + def test_attaches_patch_file(monkeypatch): monkeypatch.setattr(notify.settings, "sendgrid_api_key", "key") monkeypatch.setattr(notify.settings, "notification_sender", "from@mozilla.com") diff --git a/services/hackbot-pulse-listener/tests/test_regression.py b/services/hackbot-pulse-listener/tests/test_regression.py index 4423b9b1e7..333c1389a3 100644 --- a/services/hackbot-pulse-listener/tests/test_regression.py +++ b/services/hackbot-pulse-listener/tests/test_regression.py @@ -1,125 +1,139 @@ +"""Tests for the build-failure regression gate (is_new_build_failure).""" + from unittest.mock import patch +import pytest from app import regression from mozci.errors import ParentPushNotFound LABEL = "build-linux64/opt" +OTHER_LABEL = "build-macosx64/opt" -class FakeTask: - def __init__(self, label=LABEL, state="completed", result="passed"): - self.label = label - self.state = state - self.result = result +def _job(result="success", state="completed", task_id="T"): + return {"result": result, "state": state, "task_id": task_id} - @property - def failed(self): - return self.result in ("busted", "failed", "exception") +def passed(): + return [_job(result="success")] -class FakePush: - def __init__(self, rev, tasks, parent=None): - self.rev = rev - self.tasks = tasks - self._parent = parent - @property - def parent(self): - if self._parent is None: - raise ParentPushNotFound("no parent", rev=self.rev, branch="autoland") - return self._parent +def failed(): + return [_job(result="testfailed")] + +def busted(): + return [_job(result="busted")] -def _passed(label=LABEL): - return [FakeTask(label=label, result="passed")] +def running(): + return [_job(result=None, state="running")] -def _failed(label=LABEL): - return [FakeTask(label=label, result="failed")] +def retried(): + return [_job(result="retry")] -def _infra(label=LABEL): - return [FakeTask(label=label, state="exception", result="exception")] +def never_ran(): + return [] -def _running(label=LABEL): - return [FakeTask(label=label, state="running", result=None)] +class FakePush: + """A push whose only job is to yield the ancestor chain, as mozci does.""" -def _chain(*task_lists): - """Build a parent chain: task_lists[0] is the observed push, [1] its parent, ...""" + def __init__(self, rev, parent=None): + self.rev = rev + self._parent = parent + + @property + def parent(self): + if self._parent is None: + raise ParentPushNotFound("no parent", rev=self.rev, branch="autoland") + return self._parent + + +def _chain(*ancestors): + """Ancestor revs rev1..revN, and the Treeherder jobs recorded on each.""" push = None - for i, tasks in enumerate(reversed(task_lists)): - push = FakePush(rev=f"rev{len(task_lists) - 1 - i}", tasks=tasks, parent=push) - return push + for i in range(len(ancestors), 0, -1): + push = FakePush(f"rev{i}", parent=push) + jobs = {f"rev{i + 1}": a for i, a in enumerate(ancestors)} + return FakePush("head", parent=push), jobs -def _run(observed): - with ( - patch.object(regression, "Push", return_value=observed), - patch.object(regression.time, "sleep"), - ): - return regression.is_new_build_failure("autoland", observed.rev, LABEL) +def _run(chain, *, poll=None): + """Run the gate. `poll` supplies later job snapshots, one per poll attempt.""" + head, jobs = chain + snapshots = [jobs] + list(poll or []) + state = {"attempt": 0} + + def label_jobs(project, rev, label): + snapshot = snapshots[min(state["attempt"], len(snapshots) - 1)] + return [j for j in snapshot.get(rev, []) if label == LABEL] + def bump(_seconds): + state["attempt"] += 1 -def _run_polling(observed_sequence): - """Feed a fresh observed-push chain on each poll, simulating builds settling.""" with ( - patch.object(regression, "Push", side_effect=observed_sequence), - patch.object(regression.time, "sleep"), + patch.object(regression, "Push", return_value=head), + patch.object(regression.treeherder, "label_jobs", label_jobs), + patch.object(regression.time, "sleep", bump), ): - return regression.is_new_build_failure( - "autoland", observed_sequence[0].rev, LABEL - ) + return regression.is_new_build_failure("autoland", "head", LABEL) def test_parent_passed_is_new_failure(): - assert _run(_chain(_failed(), _passed())) is True + assert _run(_chain(passed())) is True def test_parent_failed_is_inherited(): - assert _run(_chain(_failed(), _failed())) is False + assert _run(_chain(failed())) is False + + +def test_parent_busted_is_inherited(): + assert _run(_chain(busted())) is False -def test_parent_intermittent_is_new_failure(): - parent_tasks = [ - FakeTask(result="failed"), - FakeTask(result="passed"), - ] - assert _run(_chain(_failed(), parent_tasks)) is True +def test_parent_with_one_green_retrigger_is_new_failure(): + # Any green run wins, which errs toward running the agent. + assert _run(_chain([_job(result="testfailed"), _job(result="success")])) is True def test_coalesced_parent_then_green_grandparent_is_new_failure(): - assert _run(_chain(_failed(), [], _passed())) is True + assert _run(_chain(never_ran(), passed())) is True def test_coalesced_parent_then_failed_grandparent_is_inherited(): - assert _run(_chain(_failed(), [], _failed())) is False + assert _run(_chain(never_ran(), failed())) is False -def test_infra_parent_is_waited_then_new_failure(): - # An exceptioned parent build is polled, not skipped; once its retry lands - # green the observed push introduced the failure. - assert ( - _run_polling([_chain(_failed(), _infra()), _chain(_failed(), _passed())]) - is True - ) +def test_running_parent_is_waited_then_inherited(): + # A queued or running ancestor is polled until it settles. + assert _run(_chain(running()), poll=[{"rev1": failed()}]) is False -def test_running_parent_is_waited_then_inherited(): - # A still-running parent is polled until it settles; a failure there means - # the observed push inherited it. - assert ( - _run_polling([_chain(_failed(), _running()), _chain(_failed(), _failed())]) - is False - ) +@pytest.mark.parametrize("result", ["retry", "exception", "unknown"]) +def test_unsettled_result_is_waited_not_skipped(result): + # These may still change outcome, so the ancestor must be waited for. Asserting + # the *inherited* verdict is what distinguishes waiting from treating the + # ancestor as non-decisive and walking past it. + chain = _chain([_job(result=result)]) + assert _run(chain, poll=[{"rev1": failed()}]) is False + + +def test_retried_parent_that_turns_green_is_a_new_failure(): + assert _run(_chain(retried()), poll=[{"rev1": passed()}]) is True def test_unsettled_parent_past_deadline_runs_agent(): - # An ancestor build that never settles fails open once the deadline passes. - observed = _chain(_failed(), _running()) + head, jobs = _chain(running()) with ( - patch.object(regression, "Push", return_value=observed), + patch.object(regression, "Push", return_value=head), + patch.object( + regression.treeherder, + "label_jobs", + lambda p, rev, label: jobs.get(rev, []), + ), patch.object(regression.time, "sleep"), patch.object( regression.time, @@ -127,40 +141,58 @@ def test_unsettled_parent_past_deadline_runs_agent(): side_effect=[0.0, regression.MAX_WAIT_SECONDS + 1], ), ): - assert regression.is_new_build_failure("autoland", observed.rev, LABEL) is True + assert regression.is_new_build_failure("autoland", "head", LABEL) is True def test_no_parent_runs_agent(): - assert _run(_chain(_failed())) is True + assert _run(_chain()) is True def test_no_decisive_ancestor_runs_agent(): - empties = [_failed()] + [[] for _ in range(regression.MAX_DEPTH + 2)] - assert _run(_chain(*empties)) is True + assert _run(_chain(*[never_ran() for _ in range(regression.MAX_DEPTH + 2)])) is True -def test_mozci_error_runs_agent(): - with patch.object(regression, "Push", side_effect=RuntimeError("boom")): - assert regression.is_new_build_failure("autoland", "rev", LABEL) is True +def test_other_label_on_parent_is_ignored(): + # Only our own label may decide: another build failing on the parent is not + # our failure being inherited. + head, _ = _chain(failed()) + with ( + patch.object(regression, "Push", return_value=head), + patch.object( + regression.treeherder, + "label_jobs", + lambda p, rev, label: failed() if label == OTHER_LABEL else [], + ), + patch.object(regression.time, "sleep"), + ): + assert regression.is_new_build_failure("autoland", "head", LABEL) is True -def test_other_label_on_parent_ignored(): - parent_tasks = _failed(label="build-macosx64/opt") # different build, not ours - assert _run(_chain(_failed(), parent_tasks)) is True +@pytest.mark.parametrize("boom", ["push", "jobs"]) +def test_lookup_error_runs_agent(boom): + def explode(*args, **kwargs): + raise RuntimeError("upstream down") + head, jobs = _chain(passed()) + with ( + patch.object( + regression, "Push", explode if boom == "push" else lambda *a, **k: head + ), + patch.object(regression.treeherder, "label_jobs", explode), + patch.object(regression.time, "sleep"), + ): + assert regression.is_new_build_failure("autoland", "head", LABEL) is True -def test_exception_parent_is_waited_not_failed(): - # A completed task with an infra `exception` result is unsettled, not a - # failure: we wait for the retry rather than declaring the parent failed. - exception_parent = [FakeTask(state="completed", result="exception")] - settled = _chain(_failed(), _passed()) - assert _run_polling([_chain(_failed(), exception_parent), settled]) is True +def test_pending_notice_is_logged_once_not_every_poll(caplog): + # A pending ancestor is re-polled for up to an hour; the notice must not repeat + # at INFO on every attempt. + chain = _chain(running()) + with caplog.at_level("DEBUG", logger="app.regression"): + assert _run(chain, poll=[{"rev1": running()}, {"rev1": failed()}]) is False -def test_treeherder_result_vocabulary(): - # success/busted are the Treeherder-source spellings of passed/failed. - assert _run(_chain(_failed(), [FakeTask(result="success")])) is True - assert _run(_chain(_failed(), [FakeTask(result="busted")])) is False + notices = [r for r in caplog.records if "not settled" in r.message] + assert [r.levelname for r in notices] == ["INFO", "DEBUG"] DAY = 24 * 60 * 60 diff --git a/services/hackbot-pulse-listener/tests/test_regression_test_failure.py b/services/hackbot-pulse-listener/tests/test_regression_test_failure.py new file mode 100644 index 0000000000..b41d42d5f2 --- /dev/null +++ b/services/hackbot-pulse-listener/tests/test_regression_test_failure.py @@ -0,0 +1,194 @@ +"""Tests for the test-failure regression gate (new_test_failures).""" + +from unittest.mock import patch + +import pytest +from app import regression +from mozci.errors import ParentPushNotFound + +GROUP = "dom/base/test/mochitest.ini" +OTHER_GROUP = "layout/test/mochitest.ini" +CONFIG = ("linux1804-64", "opt") +OTHER_CONFIG = ("windows11-64", "debug") + + +def _job(task_id="T1", result="testfailed", state="completed"): + return {"result": result, "state": state, "task_id": task_id} + + +class FakePush: + """A push whose only job is to yield the ancestor chain, as mozci does.""" + + def __init__(self, rev, parent=None): + self.rev = rev + self._parent = parent + + @property + def parent(self): + if self._parent is None: + raise ParentPushNotFound("no parent", rev=self.rev, branch="autoland") + return self._parent + + +def _chain(depth: int): + push = None + for i in range(depth, 0, -1): + push = FakePush(f"rev{i}", parent=push) + return FakePush("head", parent=push) + + +def _run(groups, jobs, results, *, depth=1, poll=None): + """jobs: {rev: {config: [job]}}; results: {rev: {task_id: {group: passed}}}.""" + snapshots = [(jobs, results)] + list(poll or []) + state = {"attempt": 0} + + def snapshot(): + return snapshots[min(state["attempt"], len(snapshots) - 1)] + + with ( + patch.object(regression, "Push", return_value=_chain(depth)), + patch.object( + regression.treeherder, + "config_jobs", + lambda p, rev, plat, opt: snapshot()[0].get(rev, {}).get((plat, opt), []), + ), + patch.object( + regression.treeherder, + "group_results", + lambda p, rev: snapshot()[1].get(rev, {}), + ), + patch.object( + regression.time, + "sleep", + lambda s: state.update(attempt=state["attempt"] + 1), + ), + ): + return regression.new_test_failures("autoland", "head", CONFIG, list(groups)) + + +def test_new_failure_when_ancestor_passed(): + assert _run( + [GROUP], + {"rev1": {CONFIG: [_job()]}}, + {"rev1": {"T1": {GROUP: True}}}, + ) == {GROUP} + + +def test_inherited_when_ancestor_failed(): + assert ( + _run([GROUP], {"rev1": {CONFIG: [_job()]}}, {"rev1": {"T1": {GROUP: False}}}) + == set() + ) + + +def test_retriggered_green_ancestor_counts_as_passed(): + jobs = {"rev1": {CONFIG: [_job(task_id="T1"), _job(task_id="T2")]}} + results = {"rev1": {"T1": {GROUP: False}, "T2": {GROUP: True}}} + assert _run([GROUP], jobs, results) == {GROUP} + + +def test_other_configuration_failure_does_not_mask_new_failure(): + # The manifest is already broken on another platform at the parent push; only + # this task's own label may decide. + jobs = { + "rev1": {OTHER_CONFIG: [_job(task_id="OTHER")]}, + "rev2": {CONFIG: [_job(task_id="T2")]}, + } + results = { + "rev1": {"OTHER": {GROUP: False}}, + "rev2": {"T2": {GROUP: True}}, + } + assert _run([GROUP], jobs, results, depth=2) == {GROUP} + + +def test_unfinished_ancestor_is_waited_then_inherited(): + jobs = {"rev1": {CONFIG: [_job(result=None, state="running")]}} + poll = [({"rev1": {CONFIG: [_job()]}}, {"rev1": {"T1": {GROUP: False}}})] + assert _run([GROUP], jobs, {}, poll=poll) == set() + + +def test_unsettled_sibling_defers_a_failed_group(): + # Same precedence as the build path: a run that may still turn green outranks + # an existing failure, so wait instead of calling it inherited. + jobs = {"rev1": {CONFIG: [_job(task_id="T1"), _job(task_id="T2", state="running")]}} + results = {"rev1": {"T1": {GROUP: False}}} + poll = [ + ( + {"rev1": {CONFIG: [_job(task_id="T1"), _job(task_id="T2")]}}, + {"rev1": {"T1": {GROUP: False}, "T2": {GROUP: True}}}, + ) + ] + assert _run([GROUP], jobs, results, poll=poll) == {GROUP} + + +def test_ancestor_that_never_ran_the_group_is_skipped(): + # Coalesced, or the manifest was chunked into another task: non-decisive. + jobs = { + "rev1": {CONFIG: [_job(task_id="T1")]}, + "rev2": {CONFIG: [_job(task_id="T2")]}, + } + results = { + "rev1": {"T1": {OTHER_GROUP: False}}, + "rev2": {"T2": {GROUP: True}}, + } + assert _run([GROUP], jobs, results, depth=2) == {GROUP} + + +def test_groups_are_judged_independently(): + jobs = {"rev1": {CONFIG: [_job()]}} + results = {"rev1": {"T1": {GROUP: True, OTHER_GROUP: False}}} + assert _run([GROUP, OTHER_GROUP], jobs, results) == {GROUP} + + +def test_no_ancestor_ran_the_group_fails_open(): + assert _run([GROUP], {}, {}) == {GROUP} + + +def test_lookup_error_fails_open(): + def explode(*args, **kwargs): + raise RuntimeError("treeherder down") + + with ( + patch.object(regression, "Push", return_value=_chain(1)), + patch.object(regression.treeherder, "config_jobs", explode), + patch.object(regression.time, "sleep"), + ): + assert regression.new_test_failures( + "autoland", "head", CONFIG, [GROUP, OTHER_GROUP] + ) == {GROUP, OTHER_GROUP} + + +def test_groups_of_one_task_share_one_wait_budget(): + # The budget is per call, not per group: two pending groups must not each get + # their own MAX_WAIT_SECONDS. + jobs = {"rev1": {CONFIG: [_job(result=None, state="running")]}} + + def no_sleep(_seconds): + pytest.fail("waited past a spent deadline") + + with ( + patch.object(regression, "Push", return_value=_chain(1)), + patch.object( + regression.treeherder, + "config_jobs", + lambda p, rev, plat, opt: jobs.get(rev, {}).get((plat, opt), []), + ), + patch.object(regression.treeherder, "group_results", lambda p, rev: {}), + patch.object(regression.time, "sleep", no_sleep), + patch.object( + regression.time, + "monotonic", + side_effect=[0.0, regression.MAX_WAIT_SECONDS + 1], + ), + ): + assert regression.new_test_failures( + "autoland", "head", CONFIG, [GROUP, OTHER_GROUP] + ) == {GROUP, OTHER_GROUP} + + +def test_missing_configuration_reports_every_group_as_new(): + # Without a configuration there is nothing to compare against, so nothing may + # be silently dropped. + assert regression.new_test_failures("autoland", "head", (None, None), [GROUP]) == { + GROUP + } diff --git a/services/hackbot-pulse-listener/tests/test_treeherder.py b/services/hackbot-pulse-listener/tests/test_treeherder.py new file mode 100644 index 0000000000..46d59eec2a --- /dev/null +++ b/services/hackbot-pulse-listener/tests/test_treeherder.py @@ -0,0 +1,365 @@ +"""Tests for the Treeherder classification gate.""" + +import httpx +import pytest +from app import treeherder + + +def _response(payload): + request = httpx.Request("GET", "https://treeherder.example/api") + return httpx.Response(200, json=payload, request=request) + + +def _job(classification): + return {"failure_classification_id": classification, "task_id": "TT"} + + +@pytest.fixture +def api(monkeypatch): + """Stub the job lookup; call .queue with successive return values.""" + calls = [] + + def set_results(*results): + it = iter(results) + + def fake(project, task_id): + calls.append((project, task_id)) + value = next(it) + if isinstance(value, Exception): + raise value + return value + + monkeypatch.setattr(treeherder, "_job", fake) + + monkeypatch.setattr(treeherder.time, "sleep", lambda s: None) + return type("Api", (), {"queue": staticmethod(set_results), "calls": calls}) + + +@pytest.mark.parametrize( + ("classification", "reason"), + [ + (2, "fixed by commit"), + (3, "expected fail"), + (4, "intermittent"), + (5, "infra"), + (7, "autoclassified intermittent"), + (8, "intermittent needs bugid"), + ], +) +def test_already_judged_failures_are_skipped(classification, reason): + assert treeherder.skip_reason(_job(classification)) == reason + + +@pytest.mark.parametrize("classification", [1, 6]) +def test_undecided_failures_are_investigated(classification): + # "not classified" and "new failure not classified" may still be real. + assert treeherder.skip_reason(_job(classification)) is None + + +def test_unknown_classification_is_investigated(): + # A classification we do not know about must not silently drop a regression. + assert treeherder.skip_reason(_job(99)) is None + + +def test_missing_job_is_investigated(): + assert treeherder.skip_reason(None) is None + + +def test_waits_for_ingestion_then_returns_the_job(api): + # Treeherder ingests a minute or so after the failure message arrives. + api.queue(None, None, _job(4)) + assert treeherder.job_for_task("autoland", "TT") == _job(4) + assert len(api.calls) == 3 + + +def test_never_ingested_fails_open(api, monkeypatch): + ticks = iter([0.0, treeherder.settings.treeherder_ingest_max_wait_seconds + 1]) + monkeypatch.setattr(treeherder.time, "monotonic", lambda: next(ticks)) + api.queue(None, None) + assert treeherder.job_for_task("autoland", "TT") is None + + +def test_lookup_error_fails_open(api): + api.queue(httpx.ConnectError("treeherder down")) + assert treeherder.job_for_task("autoland", "TT") is None + + +def test_lookup_uses_the_task_id_not_the_revision(monkeypatch): + # jobs/?revision= silently ignores the filter and returns unrelated jobs. + seen = {} + + def fake_get(url, **kwargs): + seen["url"] = url + return _response({"results": [_job(4)]}) + + monkeypatch.setattr(treeherder.httpx, "get", fake_get) + assert treeherder._job("autoland", "TASK123")["failure_classification_id"] == 4 + assert "task_id=TASK123" in seen["url"] + assert "revision=" not in seen["url"] + assert "/project/autoland/jobs/" in seen["url"] + + +def test_not_ingested_returns_none(monkeypatch): + monkeypatch.setattr( + treeherder.httpx, "get", lambda url, **k: _response({"results": []}) + ) + assert treeherder._job("autoland", "TT") is None + + +@pytest.fixture(autouse=True) +def _clear_group_cache(): + for cache in ( + treeherder._group_cache, + treeherder._jobs_cache, + treeherder._push_id_cache, + ): + cache.clear() + yield + for cache in ( + treeherder._group_cache, + treeherder._jobs_cache, + treeherder._push_id_cache, + ): + cache.clear() + + +def test_group_results_are_fetched_once_per_push(monkeypatch): + # A push emits many failing tasks; they must share one fetch. + calls = [] + + def fake_get(url, **kwargs): + calls.append(url) + return _response({"TASK1": {"a.ini": False}, "TASK2": {"b.ini": True}}) + + monkeypatch.setattr(treeherder.httpx, "get", fake_get) + first = treeherder.group_results("autoland", "abc" * 13 + "d") + second = treeherder.group_results("autoland", "abc" * 13 + "d") + assert first == second + assert len(calls) == 1 + assert "group_results/?revision=" in calls[0] + + +def test_group_results_refresh_bypasses_the_cache(monkeypatch): + calls = [] + + def fake_get(url, **kwargs): + calls.append(url) + return _response({"TASK1": {"a.ini": False}}) + + monkeypatch.setattr(treeherder.httpx, "get", fake_get) + rev = "abc" * 13 + "d" + treeherder.group_results("autoland", rev) + treeherder.group_results("autoland", rev, refresh=True) + assert len(calls) == 2 + + +def test_group_results_cached_per_project_and_revision(monkeypatch): + calls = [] + monkeypatch.setattr( + treeherder.httpx, + "get", + lambda url, **k: (calls.append(url), _response({}))[1], + ) + treeherder.group_results("autoland", "a" * 40) + treeherder.group_results("autoland", "b" * 40) + treeherder.group_results("mozilla-central", "a" * 40) + assert len(calls) == 3 + + +def test_label_jobs_filters_by_push_and_label(monkeypatch): + urls = [] + + def fake_get(url, **kwargs): + urls.append(url) + if "/push/?revision=" in url: + return _response({"results": [{"id": 4242}]}) + return _response( + {"results": [{"result": "success", "state": "completed", "task_id": "T1"}]} + ) + + monkeypatch.setattr(treeherder.httpx, "get", fake_get) + jobs = treeherder.label_jobs("autoland", "a" * 40, "test-linux/opt-mochitest-1") + assert [(j["result"], j["state"], j["task_id"]) for j in jobs] == [ + ("success", "completed", "T1") + ] + assert any("push_id=4242" in u for u in urls) + # The label is URL-encoded: it contains a slash. + assert any("job_type_name=test-linux%2Fopt-mochitest-1" in u for u in urls) + + +def test_label_jobs_without_a_push_is_empty(monkeypatch): + monkeypatch.setattr( + treeherder.httpx, "get", lambda url, **k: _response({"results": []}) + ) + assert treeherder.label_jobs("autoland", "a" * 40, "l") == [] + + +def test_label_jobs_cached_per_push_and_label(monkeypatch): + calls = [] + + def fake_get(url, **kwargs): + calls.append(url) + if "/push/?revision=" in url: + return _response({"results": [{"id": 1}]}) + return _response({"results": []}) + + monkeypatch.setattr(treeherder.httpx, "get", fake_get) + rev = "a" * 40 + treeherder.label_jobs("autoland", rev, "one") + treeherder.label_jobs("autoland", rev, "one") + treeherder.label_jobs("autoland", rev, "two") + # push id resolved once, jobs fetched once per distinct label. + assert sum("/push/?revision=" in u for u in calls) == 1 + assert sum("job_type_name=" in u for u in calls) == 2 + + +def test_absent_task_raises_rather_than_reporting_nothing(monkeypatch): + # A task-level failure (crash, timeout) records no per-manifest results, and so + # does a log still being parsed. Neither means "nothing failed". + monkeypatch.setattr( + treeherder, "group_results", lambda p, rev, refresh=False: {"OTHER": {}} + ) + with pytest.raises(treeherder.GroupResultsUnavailable): + treeherder.failing_groups("autoland", "a" * 40, "T1") + + +def test_task_present_with_no_failures_reports_nothing(monkeypatch): + # Distinct from the case above: Treeherder has results, none of them failing. + monkeypatch.setattr( + treeherder, + "group_results", + lambda p, rev, refresh=False: {"T1": {"a.ini": True, "b.ini": True}}, + ) + assert treeherder.failing_groups("autoland", "a" * 40, "T1") == [] + + +def test_recheck_reads_the_current_classification(monkeypatch): + monkeypatch.setattr(treeherder, "_job", lambda p, t: _job(4)) + assert treeherder.recheck_skip_reason("autoland", "T1") == "intermittent" + + +def test_recheck_does_not_wait_for_ingestion(monkeypatch): + monkeypatch.setattr(treeherder, "_job", lambda p, t: None) + monkeypatch.setattr( + treeherder.time, "sleep", lambda s: pytest.fail("re-check must not wait") + ) + assert treeherder.recheck_skip_reason("autoland", "T1") is None + + +def test_recheck_error_does_not_drop_the_failure(monkeypatch): + def boom(project, task_id): + raise RuntimeError("treeherder down") + + monkeypatch.setattr(treeherder, "_job", boom) + assert treeherder.recheck_skip_reason("autoland", "T1") is None + + +def test_failing_groups_returns_only_failures(monkeypatch): + monkeypatch.setattr( + treeherder, + "group_results", + lambda p, rev, refresh=False: { + "T1": {"a.ini": False, "b.ini": True, "/": False} + }, + ) + assert treeherder.failing_groups("autoland", "a" * 40, "T1") == ["a.ini"] + + +def test_failing_groups_refetches_when_task_absent(monkeypatch): + calls = [] + + def fake(project, rev, refresh=False): + calls.append(refresh) + return {"OTHER": {"a.ini": False}} if refresh else {} + + monkeypatch.setattr(treeherder, "group_results", fake) + with pytest.raises(treeherder.GroupResultsUnavailable): + treeherder.failing_groups("autoland", "a" * 40, "T1") + assert calls == [False, True] + + +def test_config_jobs_filters_platform_option_client_side(monkeypatch): + # Treeherder accepts platform_option but silently ignores it, so the query can + # only narrow by platform and the option must be filtered here. + urls = [] + + def fake_get(url, **kwargs): + urls.append(url) + if "/push/?revision=" in url: + return _response({"results": [{"id": 7}]}) + return _response( + { + "results": [ + { + "result": "success", + "state": "completed", + "task_id": "A", + "platform_option": "debug", + }, + { + "result": "testfailed", + "state": "completed", + "task_id": "B", + "platform_option": "opt", + }, + ] + } + ) + + monkeypatch.setattr(treeherder.httpx, "get", fake_get) + jobs = treeherder.config_jobs("autoland", "a" * 40, "linux2404-64", "debug") + assert [j["task_id"] for j in jobs] == ["A"] + assert any("platform=linux2404-64" in u for u in urls) + assert not any("platform_option=" in u for u in urls) + + +def test_config_jobs_shares_one_fetch_across_options(monkeypatch): + calls = [] + + def fake_get(url, **kwargs): + calls.append(url) + if "/push/?revision=" in url: + return _response({"results": [{"id": 7}]}) + return _response({"results": []}) + + monkeypatch.setattr(treeherder.httpx, "get", fake_get) + rev = "a" * 40 + treeherder.config_jobs("autoland", rev, "linux2404-64", "debug") + treeherder.config_jobs("autoland", rev, "linux2404-64", "opt") + assert sum("platform=" in u for u in calls) == 1 + + +def test_await_skip_reason_returns_a_late_verdict(monkeypatch): + verdicts = iter([_job(1), _job(4)]) + monkeypatch.setattr(treeherder, "_job", lambda p, t: next(verdicts)) + monkeypatch.setattr(treeherder.time, "sleep", lambda s: None) + assert treeherder.await_skip_reason("autoland", "T1", _job(1)) == "intermittent" + + +def test_await_skip_reason_uses_the_verdict_it_was_given(monkeypatch): + # Already classified at ingestion: no further request, and no waiting. + def fail(*_): + raise AssertionError("should not re-fetch an already classified job") + + monkeypatch.setattr(treeherder, "_job", fail) + monkeypatch.setattr(treeherder.time, "sleep", fail) + assert treeherder.await_skip_reason("autoland", "T1", _job(4)) == "intermittent" + + +def test_await_skip_reason_does_not_wait_without_a_job(monkeypatch): + # Treeherder never ingested the task, so there is no verdict coming; waiting the + # full window for one would stall the failure for nothing. + def fail(*_): + raise AssertionError("should not poll for a job Treeherder does not have") + + monkeypatch.setattr(treeherder, "_job", fail) + monkeypatch.setattr(treeherder.time, "sleep", fail) + assert treeherder.await_skip_reason("autoland", "T1", None) is None + + +def test_await_skip_reason_gives_up_and_investigates(monkeypatch): + monkeypatch.setattr(treeherder, "_job", lambda p, t: _job(6)) + monkeypatch.setattr(treeherder.time, "sleep", lambda s: None) + ticks = iter([0.0, treeherder.settings.treeherder_classification_wait_seconds + 1]) + monkeypatch.setattr(treeherder.time, "monotonic", lambda: next(ticks)) + assert treeherder.await_skip_reason("autoland", "T1", _job(6)) is None