Add Opencode::Instrumentation.notify; bump to 0.0.1.alpha2
Code review consensus from Tobi+Sandi: the empty-block call
'Opencode::Instrumentation.instrument(name, payload) { }' at fire-
and-forget call sites in opencode-rails is API smell. Tobi: 'two
named verbs are clearer than one verb with a vestigial block.'
Sandi: 'a method with a block parameter that's optional but expected
empty in some call sites is doing two things.'
Two emission shapes now:
.instrument(name, payload) { ... } # block; duration measured
.notify(name, payload) # fire-and-forget; no block
Both flow through the same adapter. The adapter still always
receives a block argument (some adapters key on it, e.g. AS::
Notifications.instrument requires a block) — .notify passes an
empty {}. Adapter return value is ignored for .notify (it returns
nil); .instrument continues to pass through the block's return.
Three new tests in smoke_test.rb:
- no-op when no adapter set
- forwards to adapter + verifies block presence + verifies that
.notify returns nil (not the adapter's return)
- works without a block at the call site
Also: switched gemspec metadata URLs from Gitea to GitHub. The gem
will eventually publish from github.com/ajaynomics/opencode-ruby —
the metadata now reflects that. (No actual GitHub remote push yet;
that's the user's manual step.)
15 tests pass, 32 assertions, 0 failures.
This commit is contained in:
@@ -166,4 +166,43 @@ class SmokeTest < Minitest::Test
|
||||
Opencode::Instrumentation.adapter = nil
|
||||
assert_equal 42, Opencode::Instrumentation.instrument("x") { 42 }
|
||||
end
|
||||
|
||||
def test_Instrumentation_notify_no_op_without_adapter
|
||||
Opencode::Instrumentation.adapter = nil
|
||||
# Must not raise; must return nil.
|
||||
assert_nil Opencode::Instrumentation.notify("x", foo: 1)
|
||||
end
|
||||
|
||||
def test_Instrumentation_notify_forwards_to_adapter_fire_and_forget
|
||||
events = []
|
||||
Opencode::Instrumentation.adapter = ->(name, payload, &block) {
|
||||
# block_given? is misleading inside a lambda — check the captured
|
||||
# &block instead. AS::Notifications-shaped adapters always
|
||||
# expect a block (it's what marks "event finished").
|
||||
events << [ name, payload, !block.nil? ]
|
||||
block.call if block
|
||||
:adapter_return_ignored
|
||||
}
|
||||
|
||||
result = Opencode::Instrumentation.notify("opencode.session.recreated", session_id: "ses_1")
|
||||
|
||||
# notify is fire-and-forget — it returns nil, NOT the adapter's
|
||||
# return value (that's what .instrument does).
|
||||
assert_nil result
|
||||
assert_equal 1, events.size
|
||||
name, payload, had_block = events.first
|
||||
assert_equal "opencode.session.recreated", name
|
||||
assert_equal({ session_id: "ses_1" }, payload)
|
||||
assert had_block,
|
||||
"notify must still pass an empty block — AS::Notifications-shaped " \
|
||||
"adapters always expect one"
|
||||
end
|
||||
|
||||
def test_Instrumentation_notify_does_not_require_block
|
||||
Opencode::Instrumentation.adapter = ->(_name, _payload, &_block) { }
|
||||
# Call site has no block — that's the whole point of notify.
|
||||
Opencode::Instrumentation.notify("opencode.test", k: "v")
|
||||
# If we got here without raising, the API is fire-and-forget as designed.
|
||||
assert true
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user