Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion .aspect/config.axl
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
load("@aspect//feature/artifacts.axl", "ArtifactUpload")
load("@aspect//feature/buildkite_annotations.axl", "BuildkiteAnnotations")
load("@aspect//feature/buildkite_annotations_test.axl", "bk_annotation_snapshot_tests")
load("@aspect//feature/buildkite_annotations_test.axl", "bk_annotation_decision_tests", "bk_annotation_snapshot_tests")
load("@aspect//feature/github_status_checks.axl", "GithubStatusChecks")
load("@aspect//feature/github_status_comments_test.axl", "pr_comment_snapshot_tests")
load("@aspect//lib/bazel_results_test.axl", "template_snapshot_tests")
Expand Down Expand Up @@ -123,6 +123,13 @@ def config(ctx: ConfigContext):
# Run with: aspect dev test-bk-annotation-snapshots
ctx.tasks.add(bk_annotation_snapshot_tests)

# Buildkite annotations — unit tests for the kind/context_id
# renderability decision. Locks in the contract that empty-kind
# phase markers (setup_phase / preflight_phase) skip silently
# without emitting "no renderer for kind=" trace noise.
# Run with: aspect dev test-bk-annotation-decision
ctx.tasks.add(bk_annotation_decision_tests)

# GitHub PR comment — snapshots the aggregate-status PR comment
# body across mixed task kinds, statuses, link availability, and
# phase / progress shapes.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,40 @@ def format_body(out, kind, status, task_label = "", final = False):
"""Public test hook; see `_format_body` for behavior."""
return _format_body(out, kind, status, task_label = task_label, final = final)

def should_render_update(update, context_id):
"""Decide whether a `TaskUpdate` is renderable by the BK annotation surface.

Returns `("render", "")` when the update should be rendered;
`("skip", reason)` otherwise. The `reason` is a short tag that
callers can log at trace level or assert against in tests.

Skip reasons:
`"no_kind"` — empty `update.kind`. The documented
"no template" contract (see
`lib/lifecycle.axl`): phase-marker
updates from `setup_phase` /
`preflight_phase` emit so non-template
observers (telemetry) see every state
change, but have no per-kind data to
render. Silent skip; not a bug.
`"no_context_id"` — kind is non-empty but `task_started` hasn't
populated the BK annotation context yet.
Renderable update fired too early; the
handler can't post the annotation.
`"unknown_kind"` — kind is non-empty and routed past the
empty-kind skip, but no entry exists in
`RENDERERS`. Indicates a misconfig (new
task kind added without registering a
renderer).
"""
if not update.kind:
return ("skip", "no_kind")
if not context_id:
return ("skip", "no_context_id")
if renderer_for_kind(update.kind) == None:
return ("skip", "unknown_kind")
return ("render", "")

# ─── Body formatting ──────────────────────────────────────────────────────────

_LOG = feature_logger("Buildkite annotations")
Expand Down Expand Up @@ -294,21 +328,26 @@ def _buildkite_annotations(ctx: FeatureContext):
final = update.final

# Keep the most recent phase label on bare refreshes (empty
# progress styles). BK renders the narrative `body` form.
# progress styles). BK renders the narrative `body` form. Done
# before the renderability check below so phase-marker updates
# (`kind=""` from `setup_phase` / `preflight_phase`) still latch
# their progress text for the next renderable kind to pick up.
progress_text = update.progress.body
if progress_text:
_state["_progress"] = progress_text

context_id = _state.get("_context_id", "")
if not context_id:
_LOG.trace("task_update fired but no context_id; ignoring (kind=%s status=%s)" %
(update.kind, status))
decision, reason = should_render_update(update, context_id)
if decision == "skip":
# `no_kind` is the documented phase-marker contract — silent.
# `no_context_id` / `unknown_kind` are routing/config bugs:
# trace so they surface under `ASPECT_DEBUG=1`.
if reason != "no_kind":
_LOG.trace("ignoring task_update (reason=%s kind=%r status=%s)" %
(reason, update.kind, status))
return

renderer = renderer_for_kind(update.kind)
if renderer == None:
_LOG.trace("no renderer for kind=%r; ignoring" % update.kind)
return
data = update.data if update.data else renderer.init()

# Bypass throttle on first-failing (so the red stripe lands
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ Coverage:
all-skipped, delivery all-failed, soft-format passing with files.
"""

load("./buildkite_annotations.axl", "format_body")
load("./buildkite_annotations.axl", "format_body", "should_render_update")
load("../lib/lifecycle.axl", "TaskUpdate")

# Shared fixture factories — also consumed by github_status_comments_test.axl.
load(
Expand Down Expand Up @@ -179,3 +180,73 @@ bk_annotation_snapshot_tests = task(
implementation = _test_impl,
args = {},
)

# ─── should_render_update unit tests ──────────────────────────────────────────

def _expect_render_decision(label, update, context_id, want_decision, want_reason):
got_decision, got_reason = should_render_update(update, context_id)
if got_decision != want_decision or got_reason != want_reason:
fail("%s: got (%r, %r), want (%r, %r)" %
(label, got_decision, got_reason, want_decision, want_reason))

def _decision_test_impl(ctx):
# Phase-marker updates (setup_phase / preflight_phase) emit
# `kind=""` per the lifecycle docstring contract. The BK surface
# MUST skip them silently — logging would spam every phase
# boundary. This case is the regression guard for the
# `no renderer for kind=""` log noise.
_expect_render_decision(
"empty kind, no context_id (setup_phase ordering)",
TaskUpdate(kind = "", status = "running"),
"",
"skip",
"no_kind",
)
_expect_render_decision(
"empty kind, context_id set (preflight_phase ordering)",
TaskUpdate(kind = "", status = "running"),
"aspect-lint-bk",
"skip",
"no_kind",
)

# Renderable kind but no context yet — `task_started` hasn't run.
# Trace-worthy because a renderer-mapped update arrived before
# the BK annotation context was prepared.
_expect_render_decision(
"renderable kind, missing context_id",
TaskUpdate(kind = "lint_results", status = "running"),
"",
"skip",
"no_context_id",
)

# Kind present but not in RENDERERS — misconfig (new task kind
# added without a renderer registration).
_expect_render_decision(
"unknown kind",
TaskUpdate(kind = "wat_results", status = "running"),
"aspect-lint-bk",
"skip",
"unknown_kind",
)

# Happy path — known kind + populated context.
for kind in ["bazel_results", "lint_results", "delivery_results", "format_results", "gazelle_results"]:
_expect_render_decision(
"renderable: " + kind,
TaskUpdate(kind = kind, status = "running"),
"aspect-lint-bk",
"render",
"",
)

print("should_render_update: OK (9 scenarios)")
return 0

bk_annotation_decision_tests = task(
name = "test-bk-annotation-decision",
group = ["dev"],
implementation = _decision_test_impl,
args = {},
)
Loading