Skip to content

Add focused txn_box comparison test - #13581

Open
bneradt wants to merge 1 commit into
apache:masterfrom
bneradt:fix-txn-box-tuple-comparison
Open

Add focused txn_box comparison test#13581
bneradt wants to merge 1 commit into
apache:masterfrom
bneradt:fix-txn-box-tuple-comparison

Conversation

@bneradt

@bneradt bneradt commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Txn_box's documented tuple example was reported as failing to load, but
its existing coverage is embedded in a broad static-file test and does
not isolate either reported comparison form.

This adds a replay-based regression test that loads the default txn_box
root as a global plugin and verifies heterogeneous tuple selection and
scalar status selection through response mutations.

Fixes: #12335

Txn_box's documented tuple example was reported as failing to load, but
its existing coverage is embedded in a broad static-file test and does
not isolate either reported comparison form.

This adds a replay-based regression test that loads the default txn_box
root as a global plugin and verifies heterogeneous tuple selection and
scalar status selection through response mutations.

Fixes: apache#12335
@bneradt bneradt added this to the 11.0.0 milestone Aug 21, 2026
Copilot AI lite review requested due to automatic review settings August 21, 2026 20:39
@bneradt bneradt self-assigned this Aug 21, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@bneradt bneradt added Plugins TxnBox TxnBox plugin labels Aug 21, 2026
@bryancall
bryancall self-requested a review August 24, 2026 22:08

@bryancall bryancall left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving. I built this locally with experimental plugins on and ran the test, and
confirmed it's doing real work rather than passing vacuously: it runs (Skipped: 0,
so the PluginExists gate isn't quietly skipping it), and it fails as expected both
when the tuple is made unmatchable (match: "example" -> "NOMATCH") and when the
config is given a parse error.

One thing that might let us close #12335 on firmer ground. The tuple form here was
already covered and passing at the 10.2.0 tag itself --
tests/gold_tests/pluginTest/txn_box/static_file/static_file.replay.yaml has the
identical with: [ upstream-rsp-status , proxy-req-path ] + as-tuple block, and
that's the snippet doc/admin-guide/plugins/txn_box/examples.en.rst literal-includes
as the documented example. No comparison-parsing changes have landed in txn_box since
10.2.0 either (that window is Coverity/ASAN/OpenSSL cleanups). So instead of "it works
on master," I think we can tell the reporter something stronger: the documented form
wasn't broken on the version they reported either, so the difference has to be in
their actual config or environment -- and we'd need the real file to get further.
Might be worth reopening just long enough to ask for it.

Two smaller notes:

  • The description says the existing coverage "does not isolate either reported
    comparison form." The tuple form is isolated (and doc-linked) in static_file. What's
    genuinely new here is worth calling out instead: this loads a standalone
    txn_box.yaml with the default txn_box: root key via plugin.config, whereas
    static_file goes through TxnBoxTestAndRun with config_key="meta.txn_box.global".
    The standalone path is what an operator actually uses, and only
    prod/mTLS.txnbox.yaml otherwise touches it -- that's a good argument for this test
    existing. The scalar select case is new too.

  • The reporter's second snippet had no do: under - eq: 200:

    - with: upstream-rsp-status
      select:
      - eq: 200

    This test adds one, so that literal form is still unexercised. Probably not the
    cause, but worth knowing it's untested if the issue comes back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

txn_box plugin error in example

3 participants