-
Notifications
You must be signed in to change notification settings - Fork 40
Feat: Declare plugin pipeline directions and complete ConfigSchema coverage #847
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,96 @@ | ||
| package pipeline | ||
|
|
||
| import ( | ||
| "reflect" | ||
| "testing" | ||
| ) | ||
|
|
||
| // A plugin that declares nothing is unconstrained: Supports must answer | ||
| // "no objection" for every direction, which is what keeps the field | ||
| // advisory and backward-compatible with out-of-tree plugins. | ||
| func TestSupportsUnconstrained(t *testing.T) { | ||
| var caps PluginCapabilities | ||
| for _, d := range []Direction{Inbound, Outbound} { | ||
| if !caps.Supports(d) { | ||
| t.Errorf("nil Directions should support %s", d) | ||
| } | ||
| } | ||
| // An explicitly-empty slice behaves the same as nil. | ||
| caps.Directions = []Direction{} | ||
| for _, d := range []Direction{Inbound, Outbound} { | ||
| if !caps.Supports(d) { | ||
| t.Errorf("empty Directions should support %s", d) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| func TestSupports(t *testing.T) { | ||
| cases := []struct { | ||
| name string | ||
| declared []Direction | ||
| wantIn bool | ||
| wantOutbnd bool | ||
| }{ | ||
| {"inbound only", []Direction{Inbound}, true, false}, | ||
| {"outbound only", []Direction{Outbound}, false, true}, | ||
| {"both", []Direction{Inbound, Outbound}, true, true}, | ||
| } | ||
| for _, c := range cases { | ||
| t.Run(c.name, func(t *testing.T) { | ||
| caps := PluginCapabilities{Directions: c.declared} | ||
| if got := caps.Supports(Inbound); got != c.wantIn { | ||
| t.Errorf("Supports(Inbound) = %v, want %v", got, c.wantIn) | ||
| } | ||
| if got := caps.Supports(Outbound); got != c.wantOutbnd { | ||
| t.Errorf("Supports(Outbound) = %v, want %v", got, c.wantOutbnd) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| // Normalize canonicalizes Directions so two literals describing the same | ||
| // plugin can't produce two different cached/wire representations. | ||
| func TestNormalizeDirections(t *testing.T) { | ||
| cases := []struct { | ||
| name string | ||
| in []Direction | ||
| want []Direction | ||
| }{ | ||
| {"nil stays nil", nil, nil}, | ||
| {"empty becomes nil", []Direction{}, nil}, | ||
| {"sorts", []Direction{Outbound, Inbound}, []Direction{Inbound, Outbound}}, | ||
| {"dedups", []Direction{Inbound, Inbound}, []Direction{Inbound}}, | ||
| {"dedups and sorts", []Direction{Outbound, Inbound, Outbound}, []Direction{Inbound, Outbound}}, | ||
| {"already canonical", []Direction{Inbound, Outbound}, []Direction{Inbound, Outbound}}, | ||
| } | ||
| for _, c := range cases { | ||
| t.Run(c.name, func(t *testing.T) { | ||
| got := PluginCapabilities{Directions: c.in}.Normalize().Directions | ||
| if !reflect.DeepEqual(got, c.want) { | ||
| t.Errorf("Normalize().Directions = %v, want %v", got, c.want) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| // Normalize must not reorder the caller's slice: a plugin returning a | ||
| // package-level slice from Capabilities() would otherwise have it | ||
| // permuted underneath it by whoever normalized first. | ||
| func TestNormalizeDoesNotMutateInput(t *testing.T) { | ||
| orig := []Direction{Outbound, Inbound} | ||
| caps := PluginCapabilities{Directions: orig} | ||
| _ = caps.Normalize() | ||
| if orig[0] != Outbound || orig[1] != Inbound { | ||
| t.Fatalf("Normalize mutated the input slice: %v", orig) | ||
| } | ||
| } | ||
|
|
||
| // Normalize is idempotent — the catalog normalizes on read, so applying | ||
| // it twice must not change the answer. | ||
| func TestNormalizeDirectionsIdempotent(t *testing.T) { | ||
| once := PluginCapabilities{Directions: []Direction{Outbound, Inbound, Inbound}}.Normalize() | ||
| twice := once.Normalize() | ||
| if !reflect.DeepEqual(once.Directions, twice.Directions) { | ||
| t.Errorf("not idempotent: %v then %v", once.Directions, twice.Directions) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -54,7 +54,10 @@ type FieldSchema struct { | |
|
|
||
| // Type is a coarse-grained category sufficient to render templates | ||
| // and pick value placeholders. One of: | ||
| // "string", "int", "bool", "[]string", "object", "unknown". | ||
| // "string", "int", "number", "bool", "[]string", "object", "unknown". | ||
| // "number" is a float (per-token costs, budgets); "int" stays | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion — the stated rationale is contradicted by the only consumer, inside this same PR. I agree with the code, not the comment — the two |
||
| // reserved for integral kinds so a template can emit "0" vs "0.0" | ||
| // appropriately. | ||
| // "object" indicates a nested struct whose fields populate Fields. | ||
| // "unknown" covers shapes the helper hasn't been taught (maps, | ||
| // slice-of-struct, etc.); the field still renders but without a | ||
|
|
@@ -174,6 +177,8 @@ func kindOf(t reflect.Type) string { | |
| case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, | ||
| reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: | ||
| return "int" | ||
| case reflect.Float32, reflect.Float64: | ||
| return "number" | ||
| case reflect.Slice: | ||
| // Only []string gets a typed tag; other slices are "unknown" | ||
| // (slice-of-struct, slice-of-map, etc. are rare in plugin | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
must-fix — this function will not compile against
main.WritesBodyno longer exists there:6ca3fbb("Rename WritesBody to WritesRequestBody") and6440927("Split body-write capability by direction") both landed 2026-09-02, the day this PR was opened, andgrep -c WritesBodyonmain'splugin.gois now 0.main's version reads:So the resolution is to keep
main's condition and add your one line to it:The
Directionsfield itself,canonicalDirections, andSupportsall apply cleanly — the collision is only with theWritesBodyline and the doc comment above it (which also namesWritesBody, at:96). Flagging it explicitly because a rebase that resolves this hunk by taking your side compiles nowhere, and one that takesmain's side silently drops thecanonicalDirectionscall — losing the de-duplication and sort that the rest of the change depends on.