test: migrate the event listener and cover Listen and Dispatch

[ahmed3mar]

📑 Description

Companion to goravel/framework#1541, which merges QueueListener into event.Listener and gives Handle the canonical event name. This repository implements the old signature, so its Test In Example jobs on that PR fail until this lands.

It also answers the review ask on #1541 — full tests for the new Listen / Dispatch API.

Migration

event.Listener.Handle takes a leading eventName string. A listener receives only the event name, never the event object, so it behaves the same in process and when it comes back off the queue, where only scalar arguments survive.

// before
func (receiver *SendShipmentNotification) Handle(args ...any) error

// after
func (receiver *SendShipmentNotification) Handle(eventName string, args ...any) error

Signature() and Queue() are unchanged, and the payload keeps its positions.

Tests

tests/feature/event_test.go now covers the new dispatcher:

• a string event registered with Listen and fired with Dispatch
• several events and several listeners registered in one call
• a wildcard pattern, asserting the listener receives the matched name rather than the pattern
• both closure forms: func(evt any, args ...any) error, and the typed func(evt *SomeEvent) error whose event is inferred from its parameter
• Dispatch running every listener and collecting each error, where the deprecated Job stops at the first
• a queued listener registered through Listen, asserting the event name leads the queued payload
• the registrations Listen rejects: a non-listener, a nil pointer listener, an empty signature, a typed closure on an event it does not name, and an empty event name
• Dispatch rejecting a second payload
• the deprecated Job reaching listeners registered through Listen
• a panicking listener surfacing on the Result while the listener behind it still runs
• a queued wildcard listener, asserting the matched name leads the payload
• the bootstrapped OrderShipped event fired through Dispatch rather than the deprecated Job
• the canonical event name a listener receives for an object event, goravel/tests/feature.listenEvent, which is also the queue wire format

Two behaviour changes the existing tests had to absorb

• Listeners are resolved by event name, derived from the type. The suite reused one integrationEvent type across every scenario, which made TestDispatchUnregisteredEvent find the listeners another scenario had registered under that same name. Each scenario now declares its own event type.
• GetEvents returns a copy, so the registry snapshot the suite used to unregister between tests was a no-op against the new framework. It was removed.

Nothing unregisters an event or a queue signature, and app.events, app.listeners and app.registered grow for the life of the process. The suite therefore derives its event names and listener signatures from a process-wide counter, so a second run in the same process (-count=2) does not collide with its own registrations.

CI and merge order

Neither repository can go green on its own. go.mod pins framework v1.18.0, which does not have this interface, and the framework's Test In Example job checks out example master. The order is: merge goravel/framework#1541, bump this repository's framework version to the merged commit, then this PR goes green.

Verified locally against that branch with a replace directive: go build ./... and go vet ./... both pass. The suite itself was not executed here — it needs Docker for Postgres and Redis.

[goravel-coder]

🤖 Automated review — please double-check

This is an AI-generated code review. Please double-check each finding before acting — in particular M1 (the exact require/replace resolution against the current framework master), since the pinned commit and merge order are the parts most likely to have shifted.

---

🔴 Must Fix

M1. go.mod doesn't pin a framework with Listen/Dispatch — the branch won't compile

The PR changes SendShipmentNotification.Handle to Handle(eventName string, args ...any) error and calls facades.Event().Listen(...) / .Dispatch(...) plus six new frameworkerrors.* constants, none of which exist in the pinned framework.

• PR branch (go.mod:11 + :260): require github.com/goravel/framework v1.18.0 with a no-op replace => v1.18.0. v1.18.0 has the old Listener.Handle(args ...any) error, no Listen/Dispatch, no event.Result, and only EventListenerNotBind in errors/list.go.
• origin/master (go.mod:11 require v1.18.1-0.20260819065651, :261 replace => v1.18.1-0.20260830012104-f18e0811c413): that replace resolves to framework commit f18e0811c413 = "fix: harden Orm.Connection() connection cache against concurrent cold-path races" (2026-08-30), a side-branch commit that does not contain the event feature (no application_dispatch.go/application_listen.go/result.go; Listener.Handle(args ...any) error). It isn't even reachable from framework master.
• The feature lands at framework commit dc98961c ("feat: [#730] add Listen and Dispatch to the event module (#1541)", merged 2026-09-02).

Fix: point require/replace at the latest framework commit (>= dc98961c, or a v1.18.1 release containing #1541), then rebase.

Staleness note: the PR forks from e2ab7d7f (go 1.25.0) while master has advanced (go 1.26.1) through #137–#148 — including #140, which added TestCommandMakeEventBroadcast/TestCommandMakeEventBroadcastNow to the same tests/feature/event_test.go. The branch's copy doesn't have them, and the file was substantially rewritten here, so the rebase will conflict and must preserve those tests.

M2. TestListenRejectsInvalidRegistrations "NotAListener" expects the wrong error string

tests/feature/event_test.go:492:

listen:      func() error { return facades.Event().Listen(eventName, "not a listener") },
expectedErr: frameworkerrors.EventInvalidListener.Args("string"),

The framework's newListener falls through to EventInvalidListener.Args(typeName(reflect.TypeOf(l))), and typeName builds PkgPath() + "." + Name(). A builtin string has an empty PkgPath, so the message is invalid listener .string, ..., not invalid listener string, .... Confirmed against the framework's own test (event/application_listen_test.go uses errors.EventInvalidListener.Args(".string") for the identical input).

Fix: frameworkerrors.EventInvalidListener.Args(".string"). (The other four sub-cases — nil pointer, empty signature, typed-closure mismatch, empty event name — are correct.)

---

🟡 Should Fix

S1. Data race on listeners.TestResultOfSendShipmentNotification

app/listeners/send_shipment_notification.go:31 appends to a package-level []string with no synchronization, while the test goroutine resets it (event_test.go:35), polls len(...) (:52, :459), and reads it (:58, :461). The bootstrapped SendShipmentNotification is queued (Queue().Enable=true), so on a real queue (redis/database + worker) Handle runs on a worker goroutine — a genuine memory-model violation that go test -race will flag. Masked in CI only because the default driver is sync (inline). The new listenerCapture-based tests do this correctly with a mutex; this global is the one exception, and the PR extends the pattern by adding TestDispatchBootstrappedEventThroughDispatch. Fix with a mutex-guarded accessor (or mirror listenerCapture).

S2. Coverage gaps vs. the claim to cover "the registrations Listen rejects"

• EventQueueDuplicateSignature — two queued listeners sharing one Signature() via Listen. A first-class Listen rejection (claimQueueJob returns it); the most on-point gap.
• EventHandlePanic — a panic inside an event's own Handle (the existing panic test only panics a listener, i.e. EventListenerPanic).
• EventInvalidEvent for non-string/non-struct events (Listen(42, ...)) and unsupported slice kinds (Listen([]int{...}, ...)) — only the empty-string case is covered.
• []any as the events argument — string/[]string/[]event.Event are covered, []any isn't.
• (EventListenerTypeMismatch and EventQueueMissingEvent are effectively unreachable from the public Listen/Dispatch surface, so I don't count those against the PR.)

S3. Panic and collect-error assertions are weaker than the rest of the suite

• TestDispatchRecoversFromAPanickingListener (:399) asserts ErrorContains(result.Error(), "panicked") instead of s.Equal(frameworkerrors.EventListenerPanic.Args(...), result.Error()), unlike every other error-path test.
• TestDispatchCollectsEveryListenerError (:379-382) checks Errors() only by count plus two ErrorContains, which would pass if the two errors were duplicated/swapped. s.ElementsMatch([]error{first, second}, result.Errors()) pins it exactly.

S4. waitUntil reinvents require.Eventually

event_test.go:709-721 hand-rolls a time.Sleep poll loop, but the repo already uses s.Require().Eventually(...) (tests/feature/disabled_runners_test.go:80). s.True(waitUntil(...)) soft-asserts and then fails later on the s.Equal with a misleading empty-capture diff; s.Require().Eventually fails fast at the real timeout with a real message.

S5. listenEvent type is shared across two tests, contradicting the isolation rationale

listenEvent is registered in both TestListenEventValuesAndListener (:297) and TestListenClosures (:345) under the same canonical name goravel/tests/feature.listenEvent. Passes today (each test asserts only on its own capture, and alphabetical order is favorable), but it contradicts the SetupTest comment's "each scenario declares its own event type" rule and breaks under -shuffle. Give TestListenClosures its own type (as was done for jobSeesListenEvent).

S6. Example inconsistency: bootstrapped wiring stays on the deprecated Register flow

bootstrap/app.go:47-56 (WithEvents) and app/services/mock_examples.go:54 still use the deprecated Job/Register while the suite teaches Listen/Dispatch. Still works (the PR's own tests fire the bootstrapped listeners through both paths), but for the example package it reads as half-migrated. Either migrate the boot path or add a comment stating the deprecated path is retained deliberately to demonstrate backward compatibility.

---

🟢 Nits

• castString (:699) duplicates github.com/spf13/cast.ToString, already a dependency — drop the helper.
• uniqueName's receiver s is now unused after moving the counter to the package var — could be a free function (:538).
• Names() uses append([]string(nil), ...) while Handled() uses copyAnySlice — pick one idiom (:678 vs :665).
• eventTestCounter is declared after the method that uses it (:542).
• Queue() appends to queueArgs on every call, so QueueCallCount() would over-count if the framework ever calls Queue() at both register and dispatch time (currently only asserted for a sync listener called once — latent fragility, not flaky).

---

Coverage map

Framework surface	Covered
string event / []string / event.Event / []event.Event	✅
wildcard → listener receives the matched name (sync + queued)	✅
func(evt any, args ...any) error closure	✅
typed func(evt *SomeEvent) error closure (event inferred from param)	✅
Dispatch runs every listener & collects all errors vs. Job stops-at-first	✅
queued listener via Listen: event name leads payload; GetJob(signature) resolvable	✅
deprecated Job reaches listeners registered via Listen	✅
panicking listener → error on Result, survivors still run	✅
bootstrapped event fired via Dispatch (not Job)	✅
canonical object-event name (goravel/tests/feature.listenEvent)	✅
Listen rejects: non-listener / nil-pointer / empty-signature / typed-closure-mismatch / empty-event-name	✅ (with the M2 fix)
Dispatch rejects a second payload	✅
EventQueueDuplicateSignature, EventHandlePanic, EventInvalidEvent (non-string/slice-kind), []any events	❌ missing

Coverage verdict: sufficient and high-quality for the feature's core; the error-rejection table just needs one more case (EventQueueDuplicateSignature) and ideally a panicking-event case to fully match its own "registrations Listen rejects" claim.

---

Order of operations to land this

1. Bump the example's require/replace to the latest framework commit (>= dc98961c, or a v1.18.1 release containing #1541).
2. Rebase onto master and reconcile the event_test.go conflict with #140's broadcast tests.
3. Fix the ".string" expectation in M2.
4. Then the Should Fix items above (S1–S6).