Kieran Klukas created
Remove the auto-publish from DiscoverWithStates so that
discoverSkills in the coordinator is the only code path that
emits skill discovery events. This eliminates the double-publish
where DiscoverWithStates would emit user-only states, followed
by the coordinator emitting the merged builtin+user states.
Update the Discover tests to call DiscoverWithStates directly
and assert on the returned states slice, rather than subscribing
to the pubsub broker. This also lets those tests run in parallel.internal/skills/skills.go | 2 --
internal/skills/skills_test.go | 36 +++++++++---------------------------
2 files changed, 9 insertions(+), 29 deletions(-)
@@ -289,8 +289,6 @@ func DiscoverWithStates(paths []string) ([]*Skill, []*SkillState) {
return strings.Compare(strings.ToLower(a.Name), strings.ToLower(b.Name))
})
- // Publish states as-is; the coordinator will merge and re-sort them later.
- broker.Publish(pubsub.UpdatedEvent, Event{States: states})
return skills, states
}
@@ -1,7 +1,6 @@
package skills
import (
- "context"
"os"
"path/filepath"
"strings"
@@ -216,7 +215,8 @@ func TestSkillValidate(t *testing.T) {
}
func TestDiscover(t *testing.T) {
- // Not parallel: shares global broker with other Discover tests.
+ t.Parallel()
+
tmpDir := t.TempDir()
// Create valid skill 1.
@@ -248,14 +248,8 @@ description: Name doesn't match directory.
---
`), 0o644))
- ctx, cancel := context.WithCancel(context.Background())
- defer cancel()
- ch := SubscribeEvents(ctx)
-
- skills := Discover([]string{tmpDir})
+ skills, states := DiscoverWithStates([]string{tmpDir})
- evt := <-ch
- states := evt.Payload.States
var normalCount int
var errorCount int
var hasInvalidDir bool
@@ -285,32 +279,20 @@ description: Name doesn't match directory.
}
func TestDiscoverEmptyDir(t *testing.T) {
- // Not parallel: shares global broker with other Discover tests.
+ t.Parallel()
tmpDir := t.TempDir()
- ctx, cancel := context.WithCancel(context.Background())
- defer cancel()
- ch := SubscribeEvents(ctx)
-
- skills := Discover([]string{tmpDir})
-
- evt := <-ch
- require.Empty(t, evt.Payload.States)
+ skills, states := DiscoverWithStates([]string{tmpDir})
+ require.Empty(t, states)
require.Empty(t, skills)
}
func TestDiscoverMissingPath(t *testing.T) {
- // Not parallel: shares global broker with other Discover tests.
-
- ctx, cancel := context.WithCancel(context.Background())
- defer cancel()
- ch := SubscribeEvents(ctx)
-
- skills := Discover([]string{filepath.Join(t.TempDir(), "missing")})
+ t.Parallel()
- evt := <-ch
- require.Empty(t, evt.Payload.States)
+ skills, states := DiscoverWithStates([]string{filepath.Join(t.TempDir(), "missing")})
+ require.Empty(t, states)
require.Empty(t, skills)
}