Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
152 changes: 152 additions & 0 deletions cmd/skillget/registry_edge_responses_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
package main

import (
"context"
"net/http"
"net/http/httptest"
"strings"
"testing"

skillgetmanager "github.com/getskillpack/skillget-manager"
)

// Stub-registry tests for HTTP status handling (401 / 404 / 410) per REGISTRY_CLIENT_CONTRACT.
// Always run under go test -short; no live network.

Copilot AI Mar 29, 2026

Copy link

Choose a reason for hiding this comment

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

The file header comment says these tests “Always run under go test -short”, but there’s no Short() gating here (these tests will also run in non--short runs). Consider rewording to “Safe to run under -short; no live network” or add an explicit skip when !testing.Short() if you truly want them restricted to short mode.

Suggested change
// Always run under go test -short; no live network.
// Safe to run under -short; no live network.

Copilot uses AI. Check for mistakes.

func swapRegistryHTTPClient(t *testing.T, c *http.Client) func() {
t.Helper()
prev := skillgetmanager.HTTPClient
skillgetmanager.HTTPClient = c
return func() { skillgetmanager.HTTPClient = prev }
}

func TestRegistrySearchSkills_401IncludesReadTokenHint(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodGet || r.URL.Path != "/api/v1/skills" {
http.NotFound(w, r)
return
}
http.Error(w, `{"error":"unauthorized"}`, http.StatusUnauthorized)
}))
defer srv.Close()
defer swapRegistryHTTPClient(t, srv.Client())()

t.Setenv("SKILLGET_REGISTRY_URL", srv.URL+"/api/v1")
t.Setenv("SKILLGET_REGISTRY_READ_TOKEN", "")
t.Setenv("SKILLGET_REGISTRY_TOKEN", "")
t.Setenv("SKILLGET_TOKEN", "")

_, err := skillgetmanager.SearchSkills(context.Background(), skillgetmanager.SearchSkillsOptions{Limit: 3})
if err == nil {
t.Fatal("expected error")
}
s := err.Error()
if !strings.Contains(s, "401") {
t.Fatalf("expected status in error, got: %s", s)
}
if !strings.Contains(s, "SKILLGET_REGISTRY_READ_TOKEN") {
t.Fatalf("expected 401 hint for reads, got: %s", s)
}
}

func TestRegistrySearchSkills_404IncludesSkillHint(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodGet || r.URL.Path != "/api/v1/skills" {
http.NotFound(w, r)
return
}
http.NotFound(w, r)
}))
defer srv.Close()
defer swapRegistryHTTPClient(t, srv.Client())()

t.Setenv("SKILLGET_REGISTRY_URL", srv.URL+"/api/v1")

_, err := skillgetmanager.SearchSkills(context.Background(), skillgetmanager.SearchSkillsOptions{Limit: 3})
if err == nil {
t.Fatal("expected error")
}
s := err.Error()
if !strings.Contains(s, "404") {
t.Fatalf("expected status in error, got: %s", s)
}
if !strings.Contains(s, "SKILLGET_REGISTRY_URL") {
t.Fatalf("expected 404 hint, got: %s", s)
}
Comment on lines +52 to +75

Copilot AI Mar 29, 2026

Copy link

Choose a reason for hiding this comment

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

Test name says it “IncludesSkillHint”, but the assertion checks for the registry URL env var hint (SKILLGET_REGISTRY_URL). Rename the test to match what’s being asserted (or update the assertion to check for the intended “skill name” wording) to avoid confusion when these fail.

Copilot uses AI. Check for mistakes.
}

func TestRegistryResolveInstallTarget_Skill404(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method == http.MethodGet && r.URL.Path == "/api/v1/skills/nope-such-skill" {
http.NotFound(w, r)
return
}
http.NotFound(w, r)
}))
defer srv.Close()
defer swapRegistryHTTPClient(t, srv.Client())()

t.Setenv("SKILLGET_REGISTRY_URL", srv.URL+"/api/v1")

_, err := skillgetmanager.ResolveInstallTarget(context.Background(), "nope-such-skill")
if err == nil {
t.Fatal("expected error")
}
s := err.Error()
if !strings.Contains(s, "404") {
t.Fatalf("expected status in error, got: %s", s)
}
}

func TestRegistryResolveInstallTarget_Version410YankedHint(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method == http.MethodGet && r.URL.Path == "/api/v1/skills/yanked-pkg/versions/1.0.0" {
http.Error(w, `{"error":"gone"}`, http.StatusGone)
return
}
http.NotFound(w, r)
}))
defer srv.Close()
defer swapRegistryHTTPClient(t, srv.Client())()

t.Setenv("SKILLGET_REGISTRY_URL", srv.URL+"/api/v1")

_, err := skillgetmanager.ResolveInstallTarget(context.Background(), "yanked-pkg@1.0.0")
if err == nil {
t.Fatal("expected error")
}
s := err.Error()
if !strings.Contains(s, "410") {
t.Fatalf("expected status in error, got: %s", s)
}
if !strings.Contains(s, "yanked") {
t.Fatalf("expected yanked hint, got: %s", s)
}
}

func TestRegistryPublishSkill_401IncludesTokenHint(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodPost || r.URL.Path != "/api/v1/skills" {
http.NotFound(w, r)
return
}
http.Error(w, `{"error":"unauthorized"}`, http.StatusUnauthorized)
}))
defer srv.Close()
defer swapRegistryHTTPClient(t, srv.Client())()

t.Setenv("SKILLGET_REGISTRY_URL", srv.URL+"/api/v1")
t.Setenv("SKILLGET_REGISTRY_TOKEN", "test-token-for-publish")

err := skillgetmanager.PublishSkill(context.Background(), []byte(`{}`), []byte("fake"))
if err == nil {
t.Fatal("expected error")
}
s := err.Error()
if !strings.Contains(s, "401") {
t.Fatalf("expected status in error, got: %s", s)
}
if !strings.Contains(s, "SKILLGET_REGISTRY_READ_TOKEN") {
t.Fatalf("expected 401 registry hint, got: %s", s)
Comment on lines +149 to +150

Copilot AI Mar 29, 2026

Copy link

Choose a reason for hiding this comment

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

TestRegistryPublishSkill_401IncludesTokenHint currently asserts the 401 hint contains SKILLGET_REGISTRY_READ_TOKEN, but publish auth uses the write token (SKILLGET_REGISTRY_TOKEN / SKILLGET_TOKEN). To make the test reflect the publish contract and be less brittle if the shared 401 hint text changes, assert for SKILLGET_REGISTRY_TOKEN (and/or SKILLGET_TOKEN) instead.

Suggested change
if !strings.Contains(s, "SKILLGET_REGISTRY_READ_TOKEN") {
t.Fatalf("expected 401 registry hint, got: %s", s)
if !strings.Contains(s, "SKILLGET_REGISTRY_TOKEN") && !strings.Contains(s, "SKILLGET_TOKEN") {
t.Fatalf("expected 401 registry hint to mention SKILLGET_REGISTRY_TOKEN or SKILLGET_TOKEN, got: %s", s)

Copilot uses AI. Check for mistakes.
}
}
Loading