Skip to content

Remove the e2e permit-accounting test's #[ignore], or record the ADR-0046 exception #462

Closed

Description

@FlorentinDUBOIS

Summary

crates/magnetar/tests/e2e_shared_subscription_permit_accounting.rs carries the only #[ignore] attribute in the e2e suite, added in #460. ADR-0046 §7 forbids that outright:

#[ignore] is forbidden for end-to-end tests. The "environment-dependency" carve-out in ADR-0021 §1 is removed for e2e.

and §Consequences states the intended end state as "there are no #[ignore]s on e2e tests, full stop". The deviation is deliberate and documented in the test's doc comment and in docs/testing.md, but it is still a deviation, and the ADR layer does not record it.

Why the exception was taken

Every other e2e test asserts something about magnetar. This one asserts that a broker defect is absent — apache/pulsar#26416's deferred-flow permit-accounting leak. No client behaviour can make it pass or fail, and no generally-available image carries the fix: Docker Hub's apachepulsar/pulsar stops at 4.0.13 and 4.2.4, while the fixed 4.0.14 / 4.2.5 are unpublished. Left running on the latest default it would be permanently red in CI on a defect this repository cannot repair — the exact failure mode where a check stops being read.

Measured 2026-09-18: 4.0.4 red 3/3 runs (min −10 to −14), 4.2.4 (== latest) red 8/8 (min −4 to −18), 5.0.0-M2 green in 11 of 12 with the twelfth at min=Some(-2) negatives=1. So apache/pulsar#26289 + #26422 shrink the leak by roughly two orders of magnitude without closing it.

What to do

Two decisions, and they are independent:

  1. Remove the #[ignore] once a published image is reliably green. The attribute keeps the latest default precisely so this becomes a one-line deletion the day such an image ships. Watch for 4.0.14 / 4.2.5 reaching Docker Hub. Note the 11/12 result above: 5.0.0-M2 alone is not yet a green cell, so shipping images may not be either — re-measure rather than assuming.
  2. Reconcile the ADR layer, whichever way the project prefers:
    • amend ADR-0046 with a narrow carve-out for "asserts a defect in an external component that no available version has fixed", recorded in its Status line the way ADR-0101 and ADR-0103 record theirs; or
    • keep §7 absolute and treat this test as owing the project a removal, tracked by this issue.

Doing neither leaves an accepted ADR saying "full stop" while the tree contains a counterexample, which is the state this issue exists to close.

References

Activity

  1. FlorentinDUBOIS commented on Nov 2, 2026

    @FlorentinDUBOIS
    CollaboratorAuthor

    Status update: decision 2 is delivered, decision 1 is still owed, and this issue stays open to own it.

    Decision 2 — the ADR layer is reconciled

    PR #895 adds ADR-0126, amending ADR-0046 §7 narrowly rather than leaving an accepted ADR reading "full stop" against a standing counterexample. The attribute is authorized only when all five conditions hold: the assertion is about an external component; no available version passes it repeatably by recorded measurement; the reason string carries the measured counter-evidence and the reproduce command; the deviation is written down per image; and an open issue owes the removal. Review date 2026-12-15.

    It does not restore ADR-0021 §1's general environment carve-out, does not cover an intermittent test of our own (that is ADR-0123's register), and records that register as the better long-term home if the project wants the aggregate measured continuously.

    Why decision 1 is not taken, with the measurement

    This is worth recording precisely, because the tempting argument is wrong. Verified on 2026-11-02 from each tag's own commit range, apache/pulsar#26289 and #26422 ship in all three images this matrix runs — 4.0.14, 4.2.5 and 5.0.0 — together with the later permit-leak fixes #26576, #26606 and #26661. 4.0.13 and 4.2.4 carry neither.

    Carrying the patches is still not enough:

    image 2026-10-12 2026-10-16
    4.0.14 (nightly axis only) green 5/5 green 3/3
    4.2.5 (Broker::Default, per-PR) green 7/10, residual −2 run 1 green, run 2 at −2
    5.0.0 (per-PR) green 7/10, residual −2 not run

    The test selects Broker::Default, so its per-PR axes are 4.2.5 and 5.0.0 — the two cells still reproducing a negative aggregate in roughly three runs of ten (PR #881, PR #885). Removing the #[ignore] on a version-number argument would put e2e intermittently red on a defect this repository cannot repair, which is the failure mode the attribute exists to prevent. ADR-0126's condition 2 exists to refuse exactly that inference.

    What would qualify for removal

    Both per-PR axes — the Broker::Default image and 5.0.0 — passing the unmodified test repeatedly on their published images, with each run's observation count and minimum recorded, and that evidence in the removal PR's description. A new release that merely contains an upstream patch does not qualify; neither does a single green run. The historical 5.0.0-M2 cell was green in only 11 of 12 runs, so the 5.x axis is the one to watch.

    Note on sequencing

    ADR-0126 landed before #885, which carries the per-image evidence row and the reason-string rewrite satisfying its own conditions 3 and 4. The ADR says so plainly: those two surfaces are owed by the tree until #885 merges, and the carve-out does not excuse a reason string that still names an unpublished image.

  2. added a commit that references this issue on Nov 3, 2026
    626738c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions