Files
Enterprise-Knowledge-Base/cli/cmd/api/api_test.go
T
nullkey 5adcedf170 refactor(cli): v0.3 cross-cutting cleanup
Cross-cutting findings surfaced by the branch-completion review.

Perf bug:
- Factory.Client closure was not memoized. Factory.ResolveKB internally
  calls f.Client() to resolve --kb name → id, then the command's RunE
  calls f.Client() again. Two SDK clients, two keyring reads, two
  AuthRetryTransport allocations per name-resolved invocation, with
  *independent* token state (a refresh in one was invisible to the
  other). Switched to sync.Once like Secrets already does.

Silent bug bait:
- cmdutil.NormalizeHost docstring claimed CodeInputMissingFlag for the
  empty case; code returned CodeInputInvalidArgument. Aligned doc to
  code (present-but-empty is a bad value, not a missing flag).

Agent contract gaps:
- Five user-facing subcommands lacked SetAgentHelp: auth login /
  logout / list / status and chat. Added concise strings with error-
  code call-outs so agents can branch without parsing human strings.

Helper extraction (≥3 callers):
- text.KnowledgeDisplayName(fileName, title, id) — byte-identical
  formatter that was in both cmd/doc/list.go and cmd/search/docs.go.
  Takes fields directly so internal/text stays SDK-free.
- cmdutil.WrapHTTP(cause, fmt, args...) *Error — replaces the
  `Wrapf(ClassifyHTTPError(err), err, ...)` pattern across 24 SDK
  call sites. Sed-driven migration; off-pattern shapes in chat.go
  (used streamErr) and cmdutil/kb.go (in-package) hand-edited.
  Contract test gains a comment update: post-migration the dominant
  pattern is WrapHTTP which the AST scanner skips entirely (only
  NewError/Wrapf selectors inspected); ClassifyHTTPErrorOutputs()
  bridge still covers the dynamic codes those paths can yield.

UX consistency:
- cmd/doc/list.go --page-size help now reads "Items per page
  (1..1000)" matching cmd/session/list.go. The bounds validation
  already enforced 1..1000; the help text was the last drift.

Comment-discipline sweep:
- Deleted the WHAT-only "*Options captures `weknora ...` flag state"
  docstring across 23 files (context, kb, auth, doc, session, search,
  chat, doctor, link). Where the line carried a real WHY clause
  (kb/delete, doc/delete, session/delete, kb/edit), kept the WHY and
  dropped only the leading WHAT phrase.
- Stripped third-party project-name attribution from inline comments
  and one user-visible flag-help string across ~40 files in cli/cmd
  and cli/internal (plus 4 test-file comments). Removed phrases like
  "Mirrors `gh X`", "borrowed from lark-cli", "kubectl-style",
  "gcloud `--project`", "Stripe pattern", and the embedded GitHub
  URLs pointing at those projects. Behavioral descriptions and the
  WHY behind each comment are preserved; only the upstream-name
  attribution is gone. Inspiration / north-star references belong in
  cli/AGENTS.md (the design doc) and commit messages, not scattered
  through every file.

  Triggered by an audit round that surfaced several false / fragile
  parity claims (e.g. "Mirrors `gh repo edit`" — gh repo edit has no
  --name flag; "matches gcloud `--project` id-or-name" — gcloud's
  --project accepts ID only). Rather than fix them one by one, the
  whole category of in-comment external-project references was
  stripped uniformly.
2026-05-14 10:57:17 +08:00

291 lines
9.4 KiB
Go

package api
import (
"context"
"encoding/json"
"io"
"net/http"
"net/http/httptest"
"os"
"path/filepath"
"strings"
"testing"
"github.com/spf13/cobra"
"github.com/Tencent/WeKnora/cli/internal/cmdutil"
"github.com/Tencent/WeKnora/cli/internal/iostreams"
"github.com/Tencent/WeKnora/cli/internal/prompt"
sdk "github.com/Tencent/WeKnora/client"
)
// newTestClient stands up an httptest server with the supplied handler and
// returns an *sdk.Client targeting it plus a teardown closure. The real SDK is
// used so we exercise the same Raw() code path as production (header
// injection, JSON marshalling, etc.).
func newTestClient(t *testing.T, h http.HandlerFunc) (*sdk.Client, func()) {
t.Helper()
srv := httptest.NewServer(h)
return sdk.NewClient(srv.URL), srv.Close
}
func TestAPI_GetSuccess(t *testing.T) {
out, _ := iostreams.SetForTest(t)
cli, stop := newTestClient(t, func(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodGet || r.URL.Path != "/api/v1/foo" {
t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path)
}
w.Header().Set("Content-Type", "application/json")
_, _ = w.Write([]byte(`{"hello":"world"}`))
})
defer stop()
if err := runAPI(context.Background(), &Options{}, cli, "GET", "/api/v1/foo"); err != nil {
t.Fatalf("runAPI: %v", err)
}
got := out.String()
if !strings.Contains(got, `"hello":"world"`) {
t.Errorf("expected raw JSON body in stdout, got %q", got)
}
if !strings.HasSuffix(got, "\n") {
t.Errorf("expected trailing newline appended, got %q", got)
}
}
func TestAPI_GetSuccess_JSON(t *testing.T) {
out, _ := iostreams.SetForTest(t)
cli, stop := newTestClient(t, func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
w.Header().Set("X-Request-Id", "req-123")
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte(`{"value":42}`))
})
defer stop()
if err := runAPI(context.Background(), &Options{JSONOut: true}, cli, "GET", "/api/v1/foo"); err != nil {
t.Fatalf("runAPI: %v", err)
}
var env struct {
OK bool `json:"ok"`
Data struct {
Status int `json:"status"`
Headers map[string]string `json:"headers"`
Body map[string]any `json:"body"`
} `json:"data"`
}
if err := json.Unmarshal(out.Bytes(), &env); err != nil {
t.Fatalf("decode envelope: %v\n%s", err, out.String())
}
if !env.OK {
t.Errorf("expected ok:true, got %s", out.String())
}
if env.Data.Status != 200 {
t.Errorf("status: want 200, got %d", env.Data.Status)
}
if env.Data.Headers["Content-Type"] != "application/json" {
t.Errorf("Content-Type header missing: %v", env.Data.Headers)
}
if got, ok := env.Data.Body["value"]; !ok || got.(float64) != 42 {
t.Errorf("body.value: want 42, got %v", env.Data.Body)
}
}
func TestAPI_PostWithData(t *testing.T) {
_, _ = iostreams.SetForTest(t)
var seenBody []byte
var seenMethod, seenPath string
cli, stop := newTestClient(t, func(w http.ResponseWriter, r *http.Request) {
seenMethod = r.Method
seenPath = r.URL.Path
seenBody, _ = io.ReadAll(r.Body)
w.Header().Set("Content-Type", "application/json")
_, _ = w.Write([]byte(`{"id":"new"}`))
})
defer stop()
opts := &Options{Data: `{"name":"foo"}`}
if err := runAPI(context.Background(), opts, cli, "POST", "/api/v1/things"); err != nil {
t.Fatalf("runAPI: %v", err)
}
if seenMethod != http.MethodPost || seenPath != "/api/v1/things" {
t.Errorf("server saw %s %s, want POST /api/v1/things", seenMethod, seenPath)
}
// SDK marshals body via json.Marshal; json.RawMessage round-trips
// verbatim so the bytes server-side equal the --data argument.
if string(seenBody) != `{"name":"foo"}` {
t.Errorf("server received body %q, want %q", seenBody, `{"name":"foo"}`)
}
}
// TestAPI_InputFile verifies --input <file> reads the request body from disk.
func TestAPI_InputFile(t *testing.T) {
_, _ = iostreams.SetForTest(t)
tmp := filepath.Join(t.TempDir(), "body.json")
payload := `{"k":"from-file"}`
if err := os.WriteFile(tmp, []byte(payload), 0o600); err != nil {
t.Fatalf("write temp: %v", err)
}
var seenBody []byte
cli, stop := newTestClient(t, func(w http.ResponseWriter, r *http.Request) {
seenBody, _ = io.ReadAll(r.Body)
_, _ = w.Write([]byte(`{}`))
})
defer stop()
opts := &Options{Input: tmp}
if err := runAPI(context.Background(), opts, cli, "POST", "/api/v1/x"); err != nil {
t.Fatalf("runAPI: %v", err)
}
if string(seenBody) != payload {
t.Errorf("body from --input: got %q, want %q", seenBody, payload)
}
}
// TestAPI_InputDash_Stdin verifies the "--input -" form: the payload comes
// from opts.StdinReader (production-default iostreams.IO.In).
func TestAPI_InputDash_Stdin(t *testing.T) {
_, _ = iostreams.SetForTest(t)
var seenBody []byte
cli, stop := newTestClient(t, func(w http.ResponseWriter, r *http.Request) {
seenBody, _ = io.ReadAll(r.Body)
_, _ = w.Write([]byte(`{}`))
})
defer stop()
payload := `{"k":"from-stdin"}`
opts := &Options{Input: "-", StdinReader: strings.NewReader(payload)}
if err := runAPI(context.Background(), opts, cli, "POST", "/api/v1/x"); err != nil {
t.Fatalf("runAPI: %v", err)
}
if string(seenBody) != payload {
t.Errorf("body from --input -: got %q, want %q", seenBody, payload)
}
}
func TestAPI_NotFound(t *testing.T) {
_, _ = iostreams.SetForTest(t)
cli, stop := newTestClient(t, func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusNotFound)
_, _ = w.Write([]byte(`{"error":"missing"}`))
})
defer stop()
err := runAPI(context.Background(), &Options{}, cli, "GET", "/api/v1/missing")
if err == nil {
t.Fatal("expected error for 404")
}
if !cmdutil.IsNotFound(err) {
t.Errorf("expected resource.not_found, got %v", err)
}
}
func TestAPI_InvalidMethod(t *testing.T) {
_, _ = iostreams.SetForTest(t)
// No server needed: validation should fail before dispatch.
err := runAPI(context.Background(), &Options{}, nil, "FOO", "/api/v1/things")
if err == nil {
t.Fatal("expected error for unsupported method")
}
var ce *cmdutil.Error
if !asTypedError(err, &ce) || ce.Code != cmdutil.CodeInputInvalidArgument {
t.Errorf("expected input.invalid_argument, got %v", err)
}
}
func TestAPI_PathWithoutSlash(t *testing.T) {
_, _ = iostreams.SetForTest(t)
err := runAPI(context.Background(), &Options{}, nil, "GET", "api/v1/things")
if err == nil {
t.Fatal("expected error for missing leading slash")
}
var ce *cmdutil.Error
if !asTypedError(err, &ce) || ce.Code != cmdutil.CodeInputInvalidArgument {
t.Errorf("expected input.invalid_argument, got %v", err)
}
}
// withRootHarness wraps `weknora api ...` under a synthetic root cmd that
// registers the global `-y/--yes` persistent flag (mirrors addGlobalFlags in
// cmd/root.go). Required because api's NewCmd doesn't register --yes itself
// — it inherits from root in production.
func withRootHarness(api *cobra.Command, args ...string) *cobra.Command {
root := &cobra.Command{Use: "weknora"}
root.PersistentFlags().BoolP("yes", "y", false, "")
root.PersistentFlags().Bool("dry-run", false, "")
root.AddCommand(api)
root.SetArgs(append([]string{"api"}, args...))
root.SetContext(context.Background())
root.SilenceErrors = true
root.SilenceUsage = true
return root
}
// TestAPI_DELETE_RequiresConfirmation pins the exit-10 protocol on the
// escape-hatch DELETE path: agent invokes `weknora api DELETE /...` without
// -y/--yes, must get input.confirmation_required + exit 10. Confirmation is
// enforced in NewCmd.RunE (not runAPI), so the test drives the cobra cmd.
func TestAPI_DELETE_RequiresConfirmation(t *testing.T) {
iostreams.SetForTest(t) // non-TTY
f := &cmdutil.Factory{
Client: func() (*sdk.Client, error) { return nil, nil },
Prompter: func() prompt.Prompter { return prompt.AgentPrompter{} },
}
root := withRootHarness(NewCmd(f), "/api/v1/knowledge-bases/kb_xxx", "-X", "DELETE")
err := root.Execute()
if err == nil {
t.Fatal("expected confirmation_required error for DELETE without -y")
}
var ce *cmdutil.Error
if !asTypedError(err, &ce) || ce.Code != cmdutil.CodeInputConfirmationRequired {
t.Errorf("want input.confirmation_required, got %v", err)
}
if got := cmdutil.ExitCode(err); got != 10 {
t.Errorf("exit code = %d, want 10", got)
}
}
// TestAPI_DELETE_WithYes_Proceeds: -y/--yes opt-in skips confirmation and
// dispatches to the SDK. Server returns 200 to verify the happy-path lands
// on the response body emit.
func TestAPI_DELETE_WithYes_Proceeds(t *testing.T) {
iostreams.SetForTest(t)
called := false
cli, stop := newTestClient(t, func(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodDelete {
t.Errorf("expected DELETE, got %s", r.Method)
}
called = true
w.WriteHeader(http.StatusOK)
})
defer stop()
f := &cmdutil.Factory{
Client: func() (*sdk.Client, error) { return cli, nil },
Prompter: func() prompt.Prompter { return prompt.AgentPrompter{} },
}
root := withRootHarness(NewCmd(f), "/api/v1/knowledge-bases/kb_xxx", "-X", "DELETE", "-y")
if err := root.Execute(); err != nil {
t.Fatalf("execute: %v", err)
}
if !called {
t.Error("DELETE handler not called — confirmation may have blocked")
}
}
// asTypedError is a tiny wrapper around errors.As that keeps the call sites
// concise. Returns true on success, populating dst.
func asTypedError(err error, dst **cmdutil.Error) bool {
for e := err; e != nil; {
if t, ok := e.(*cmdutil.Error); ok {
*dst = t
return true
}
type unwrapper interface{ Unwrap() error }
u, ok := e.(unwrapper)
if !ok {
return false
}
e = u.Unwrap()
}
return false
}