Claude/test coverage analysis s33ri0

cmd/mcpcurl/coverage_test.go

@@ -0,0 +1,199 @@
+package main
+
+import (
+	"encoding/json"
+	"io"
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/spf13/cobra"
+	"github.com/stretchr/testify/require"
+)
+
+// sampleTool returns a Tool exercising every property type handled by
+// addCommandFromTool / buildArgumentsMap.
+func sampleTool() *Tool {
+	return &Tool{
+		Name:        "sample_tool",
+		Description: "A sample tool",
+		InputSchema: InputSchema{
+			Type: "object",
+			Properties: map[string]Property{
+				"name":   {Type: "string", Description: "the name"},
+				"status": {Type: "string", Description: "state", Enum: []string{"open", "closed"}},
+				"count":  {Type: "integer", Description: "how many"},
+				"ratio":  {Type: "number", Description: "a ratio"},
+				"active": {Type: "boolean", Description: "is active"},
+				"labels": {Type: "array", Description: "labels", Items: &PropertyItem{Type: "string"}},
+				"items":  {Type: "array", Description: "items", Items: &PropertyItem{Type: "object"}},
+			},
+			Required: []string{"name"},
+		},
+	}
+}
+
+// newToolCommand builds the cobra subcommand for a tool via addCommandFromTool
+// and returns it, so tests can set flags and call the pure helpers.
+func newToolCommand(t *testing.T, tool *Tool) *cobra.Command {
+	t.Helper()
+	parent := &cobra.Command{Use: "tools"}
+	addCommandFromTool(parent, tool, false)
+	cmds := parent.Commands()
+	require.Len(t, cmds, 1)
+	return cmds[0]
+}
+
+func TestAddCommandFromTool_RegistersFlags(t *testing.T) {
+	cmd := newToolCommand(t, sampleTool())
+
+	require.Equal(t, "sample_tool", cmd.Use)
+	require.Equal(t, "A sample tool", cmd.Short)
+
+	// A flag is registered for each scalar/array property.
+	for _, name := range []string{"name", "status", "count", "ratio", "active", "labels"} {
+		require.NotNil(t, cmd.Flags().Lookup(name), "expected flag %q", name)
+	}
+	// Object-array properties get a "<name>-json" flag.
+	require.NotNil(t, cmd.Flags().Lookup("items-json"))
+
+	// Optional flags advertise themselves as optional in the usage text.
+	require.Contains(t, cmd.Flags().Lookup("status").Usage, "(optional)")
+	require.NotContains(t, cmd.Flags().Lookup("name").Usage, "(optional)")
+}
+
+func TestAddCommandFromTool_EnumValidation(t *testing.T) {
+	cmd := newToolCommand(t, sampleTool())
+	require.NotNil(t, cmd.PreRunE, "enum property should install a PreRunE validator")
+
+	require.NoError(t, cmd.Flags().Set("status", "open"))
+	require.NoError(t, cmd.PreRunE(cmd, nil))
+
+	require.NoError(t, cmd.Flags().Set("status", "bogus"))
+	err := cmd.PreRunE(cmd, nil)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "status must be one of")
+}
+
+func TestBuildArgumentsMap_AllTypes(t *testing.T) {
+	tool := sampleTool()
+	cmd := newToolCommand(t, tool)
+
+	require.NoError(t, cmd.Flags().Set("name", "hello"))
+	require.NoError(t, cmd.Flags().Set("count", "5"))
+	require.NoError(t, cmd.Flags().Set("ratio", "1.5"))
+	require.NoError(t, cmd.Flags().Set("active", "true"))
+	require.NoError(t, cmd.Flags().Set("labels", "a,b"))
+	require.NoError(t, cmd.Flags().Set("items-json", `[{"x":1}]`))
+
+	args, err := buildArgumentsMap(cmd, tool)
+	require.NoError(t, err)
+
+	require.Equal(t, "hello", args["name"])
+	require.Equal(t, int64(5), args["count"])
+	require.InEpsilon(t, 1.5, args["ratio"], 1e-9)
+	require.Equal(t, true, args["active"])
+	require.Equal(t, []string{"a", "b"}, args["labels"])
+	require.Equal(t, []any{map[string]any{"x": float64(1)}}, args["items"])
+
+	// Unset, zero-valued, and empty fields are omitted.
+	_, ok := args["status"]
+	require.False(t, ok, "unset string should be omitted")
+}
+
+func TestBuildArgumentsMap_OmitsZeroAndUnset(t *testing.T) {
+	tool := sampleTool()
+	cmd := newToolCommand(t, tool)
+
+	// Only set the boolean to false explicitly; it must still be included
+	// because Changed() is true, while zero-valued numbers stay omitted.
+	require.NoError(t, cmd.Flags().Set("active", "false"))
+
+	args, err := buildArgumentsMap(cmd, tool)
+	require.NoError(t, err)
+
+	require.Equal(t, false, args["active"])
+	_, hasCount := args["count"]
+	require.False(t, hasCount, "zero integer should be omitted")
+	_, hasName := args["name"]
+	require.False(t, hasName, "unset string should be omitted")
+}
+
+func TestBuildArgumentsMap_InvalidObjectJSON(t *testing.T) {
+	tool := sampleTool()
+	cmd := newToolCommand(t, tool)
+
+	require.NoError(t, cmd.Flags().Set("items-json", "{not valid json"))
+
+	_, err := buildArgumentsMap(cmd, tool)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "error parsing JSON for items")
+}
+
+func TestBuildJSONRPCRequest(t *testing.T) {
+	out, err := buildJSONRPCRequest("tools/call", "my_tool", map[string]any{"a": "b"})
+	require.NoError(t, err)
+
+	var req JSONRPCRequest
+	require.NoError(t, json.Unmarshal([]byte(out), &req))
+	require.Equal(t, "2.0", req.JSONRPC)
+	require.Equal(t, "tools/call", req.Method)
+	require.Equal(t, "my_tool", req.Params.Name)
+	require.Equal(t, "b", req.Params.Arguments["a"])
+}
+
+// captureStdout runs fn and returns everything it wrote to os.Stdout.
+func captureStdout(t *testing.T, fn func()) string {
+	t.Helper()
+	orig := os.Stdout
+	r, w, err := os.Pipe()
+	require.NoError(t, err)
+	os.Stdout = w
+	defer func() { os.Stdout = orig }()
+
+	fn()
+	require.NoError(t, w.Close())
+	out, err := io.ReadAll(r)
+	require.NoError(t, err)
+	return string(out)
+}
+
+func TestPrintResponse_NoPrettyPrint(t *testing.T) {
+	raw := `{"jsonrpc":"2.0","id":1,"result":{}}`
+	out := captureStdout(t, func() {
+		require.NoError(t, printResponse(raw, false))
+	})
+	require.Equal(t, raw, strings.TrimSpace(out))
+}
+
+func TestPrintResponse_PrettyObjectContent(t *testing.T) {
+	// content.text holds a JSON object string that should be re-indented.
+	resp := `{"jsonrpc":"2.0","id":1,"result":{"content":[{"type":"text","text":"{\"hello\":\"world\"}"}]}}`
+	out := captureStdout(t, func() {
+		require.NoError(t, printResponse(resp, true))
+	})
+	require.Contains(t, out, "\"hello\": \"world\"")
+}
+
+func TestPrintResponse_PrettyArrayContent(t *testing.T) {
+	// content.text holds a JSON array string -> JSONL fallback path.
+	resp := `{"jsonrpc":"2.0","id":1,"result":{"content":[{"type":"text","text":"[{\"n\":1}]"}]}}`
+	out := captureStdout(t, func() {
+		require.NoError(t, printResponse(resp, true))
+	})
+	require.Contains(t, out, "\"n\": 1")
+}
+
+func TestPrintResponse_PrettyEmptyContent(t *testing.T) {
+	resp := `{"jsonrpc":"2.0","id":1,"result":{"content":[]}}`
+	out := captureStdout(t, func() {
+		require.NoError(t, printResponse(resp, true))
+	})
+	require.Contains(t, out, resp)
+}
+
+func TestPrintResponse_PrettyInvalidJSON(t *testing.T) {
+	err := printResponse("not json", true)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "failed to parse JSON")
+}

docs/PENDIENTES_DE_EMPEZAR.md

@@ -0,0 +1,29 @@
+# Pendientes de empezar (sin comenzar)
+
+> Fecha: junio 2026. Cobertura global actual: **69,3 %** (sobre `main` = upstream).
+
+## Paquetes con menor cobertura (objetivos)
+
+| Paquete | Cobertura | Notas |
+|---|---:|---|
+| `internal/ghmcp` | **0,0 %** | Arranque del servidor, hosts GHES/GHEC, transporte HTTP. Mayor hueco e impacto. |
+| `cmd/mcpcurl` | ~55 % | **En progreso** (este trabajo). Resto sin cubrir: rutas con subproceso (`executeServerCommand`, `Run`). |
+| `internal/githubv4mock` | 18,1 % | Utilidad de test; cobertura indirecta. |
+| `pkg/http` | 37,4 % | Capa del servidor HTTP remoto. |
+| `pkg/http/transport` | 46,7 % | Transporte HTTP. |
+| `pkg/log` | 47,4 % | Logging E/S. |
+| `pkg/http/middleware` | 50,6 % | Middleware del servidor. |
+| `pkg/utils` | 54,0 % | Helpers de resultados. |
+
+## Tareas concretas sin empezar
+1. **`internal/ghmcp`**: tests de derivación de hosts GHES/GHEC, wrappers de
+   transporte/`RoundTrip`, middleware. Mayor impacto en el total.
+2. **`pkg/http` y submódulos** (`transport`, `middleware`, `oauth`): rutas del
+   servidor remoto.
+3. **`cmd/mcpcurl`** (rematar): cubrir `executeServerCommand` y el `Run` de los
+   comandos generados (requiere un binario/servidor de prueba por stdio).
+
+## Recomendación de orden
+Confirmar primero la estabilidad vs upstream (ver PENDIENTES_DE_RESOLVER). Si se
+mantiene: `internal/ghmcp` → `pkg/http*`. Re-medir la cobertura del paquete antes
+de cada tarea, porque el repo cambia con frecuencia.

docs/PENDIENTES_DE_RESOLVER.md

@@ -0,0 +1,18 @@
+# Pendientes de resolver (en curso / decisión pendiente)
+
+> Fecha: junio 2026.
+
+## 1. ¿Abrir PR con los tests de `cmd/mcpcurl`?
+- Estado: el fichero `cmd/mcpcurl/coverage_test.go` está hecho y verde en local,
+  commiteado en la rama. Falta decidir si se abre PR contra `main`.
+
+## 2. Estabilidad del repo frente al upstream — RIESGO
+- `main` ya se ha reseteado al upstream una vez, borrando trabajo previo.
+- Mientras no se aclare la política del fork (¿se sigue sincronizando con
+  `github/github-mcp-server`?), **cualquier test nuevo puede volver a perderse**.
+- Pendiente: confirmar con el responsable si el fork va a divergir del upstream
+  o seguir sincronizándose, para decidir cuánto esfuerzo invertir.
+
+## 3. Documentos de seguimiento
+- Recuperados en `docs/` y puestos al día (este conjunto). Pendiente: mantenerlos
+  vivos según avance el trabajo.

docs/TRABAJO_REALIZADO.md

@@ -0,0 +1,34 @@
+# Trabajo realizado
+
+> Proyecto: **github-mcp-server** (servidor MCP en Go). Fecha: junio 2026.
+> Rama de trabajo: `claude/test-coverage-analysis-s33ri0`.
+
+## Nota de contexto importante
+El `main` del fork se **sincronizó/reseteó con el upstream oficial
+`github/github-mcp-server`**. Eso **eliminó de `main`** todo el trabajo previo
+del fork (tests de `pkg/lockdown` del PR #5, documentos de estado, los arreglos
+de workflows, etc.). El historial actual de `main` es el del upstream. Por eso
+este documento se ha reescrito para reflejar la realidad actual.
+
+## Histórico (referencia)
+- Análisis inicial de cobertura del repo.
+- **PR #5** (fusionado en su momento): tests de `pkg/lockdown` → perdidos en el reset.
+- **PR #10** (cerrado sin fusionar): arreglo de CI mezclado con docs.
+- **PR #12** (cerrado): reintento de arreglo de CI; quedó obsoleto porque el
+  reset a upstream eliminó los workflows rotos (`python-package.yml`,
+  `go-build.yml`) que pretendía borrar.
+- Diagnósticos de CI hechos por el camino (colisión de símbolos por refactor
+  paralelo, crash de golangci-lint por Go 1.26 vs golangci 1.25, workflow de
+  Python sobre repo Go).
+
+## Trabajo vigente sobre el `main` nuevo (upstream)
+- **Re-medición de cobertura** desde cero: total **69,3 %**.
+- **Tests para `cmd/mcpcurl`**: subido de **8,9 % → ~55 %** añadiendo
+  `cmd/mcpcurl/coverage_test.go` (sin tocar el test existente). Cubre:
+  - `buildArgumentsMap` → 100 % (todos los tipos: string/number/integer/boolean/
+    array de strings/array de objetos vía `-json`, y error de JSON inválido).
+  - `printResponse` → 88,5 % (sin pretty, objeto, array/JSONL, vacío, JSON inválido).
+  - `buildJSONRPCRequest` → 75 %.
+  - `addCommandFromTool` → 63,5 % (registro de flags, flags requeridos, validación
+    de `enum` vía `PreRunE`).
+- Verificado en local: `gofmt`, `go vet ./...`, `go test ./...` → **todo verde**.