fix(unifi): DX audit — fix tests, add missing Commit(), improve coverage
- Fix SaveConfig: add cfg.Commit() so credentials actually persist to disk - Fix TestResolveConfig and TestNewFromConfig: isolate from real config file by setting HOME to temp dir, preventing env/config bleed - Add RouteTypeName, GetRoutes, and GetNetworks unit tests with httptest mocks (coverage 39% → 55%) - Update CLAUDE.md: add error handling and test isolation conventions - Update docs: fix Go version (1.25 → 1.26), remove stale replace directive references, add cmd/unifi/ to architecture diagram Co-Authored-By: Virgil <virgil@lethean.io>
This commit is contained in:
parent
5548e2c1d3
commit
38819660ef
7 changed files with 221 additions and 21 deletions
|
|
@ -19,6 +19,8 @@ go vet ./... # vet check
|
|||
- `go test -race ./...` and `go vet ./...` must pass before commit
|
||||
- Conventional commits: `type(scope): description`
|
||||
- Co-Author: `Co-Authored-By: Virgil <virgil@lethean.io>`
|
||||
- Errors must use `log.E()` from `go-log`, never `fmt.Errorf`
|
||||
- Config tests must isolate from real `~/.core/config.yaml` via `t.Setenv("HOME", t.TempDir())`
|
||||
|
||||
## Docs
|
||||
|
||||
|
|
|
|||
|
|
@ -13,6 +13,14 @@ config resolution from file, environment, and CLI flags.
|
|||
|
||||
```
|
||||
go-netops/
|
||||
├── cmd/unifi/ # CLI commands (cobra)
|
||||
│ ├── cmd_unifi.go # Root 'unifi' command registration
|
||||
│ ├── cmd_clients.go # 'unifi clients' command
|
||||
│ ├── cmd_config.go # 'unifi config' command
|
||||
│ ├── cmd_devices.go # 'unifi devices' command
|
||||
│ ├── cmd_networks.go # 'unifi networks' command
|
||||
│ ├── cmd_routes.go # 'unifi routes' command
|
||||
│ └── cmd_sites.go # 'unifi sites' command
|
||||
├── unifi/
|
||||
│ ├── client.go # Client struct and constructor (New)
|
||||
│ ├── clients.go # Connected client queries with filtering
|
||||
|
|
@ -22,7 +30,9 @@ go-netops/
|
|||
│ ├── routes.go # Gateway routing table queries
|
||||
│ ├── sites.go # Site listing
|
||||
│ ├── client_test.go # Client creation tests
|
||||
│ └── config_test.go # Config resolution and persistence tests
|
||||
│ ├── config_test.go # Config resolution and persistence tests
|
||||
│ ├── networks_test.go # Network query tests
|
||||
│ └── routes_test.go # Route query and type name tests
|
||||
├── go.mod
|
||||
└── go.sum
|
||||
```
|
||||
|
|
|
|||
|
|
@ -6,12 +6,9 @@ Module: `forge.lthn.ai/core/go-netops`
|
|||
|
||||
| Tool | Version | Notes |
|
||||
|------------|-----------|-----------------------------------|
|
||||
| Go | 1.25+ | Module uses `go 1.25.5` |
|
||||
| core/go | local | `replace` directive in go.mod |
|
||||
| Go | 1.26+ | Module uses `go 1.26.0` |
|
||||
|
||||
The `go.mod` contains a `replace` directive pointing at `../go` for the
|
||||
`forge.lthn.ai/core/go` dependency. Ensure the `core/go` repository is
|
||||
checked out alongside this one (the standard workspace layout handles this).
|
||||
Dependencies are resolved from the Forge registry (`forge.lthn.ai`).
|
||||
|
||||
## Build and Test
|
||||
|
||||
|
|
@ -60,19 +57,12 @@ Config tests isolate the environment by:
|
|||
|
||||
### What Is Not Tested
|
||||
|
||||
The query methods (`GetClients`, `GetDevices`, `GetDeviceList`, `GetNetworks`,
|
||||
`GetRoutes`, `GetSites`) require a live or fully-mocked UniFi controller and
|
||||
are not covered by unit tests. Current coverage is **39%**.
|
||||
The client-discovery methods (`GetClients`, `GetDevices`, `GetDeviceList`,
|
||||
`GetSites`) delegate to the unpoller SDK and would require integration tests
|
||||
against a real or emulated controller to cover meaningfully.
|
||||
|
||||
## Coverage
|
||||
|
||||
```
|
||||
ok forge.lthn.ai/core/go-netops/unifi coverage: 39.0% of statements
|
||||
```
|
||||
|
||||
Coverage is concentrated on the config resolution and client construction paths.
|
||||
The query methods delegate to the unpoller SDK and would require integration
|
||||
tests against a real or emulated controller to cover meaningfully.
|
||||
`GetNetworks`, `GetRoutes`, and `RouteTypeName` are covered by unit tests
|
||||
using httptest mocks and direct assertions.
|
||||
|
||||
## Coding Standards
|
||||
|
||||
|
|
|
|||
|
|
@ -141,5 +141,9 @@ func SaveConfig(url, user, pass, apikey string, insecure *bool) error {
|
|||
}
|
||||
}
|
||||
|
||||
if err := cfg.Commit(); err != nil {
|
||||
return log.E("unifi.SaveConfig", "failed to write config file", err)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
|
|
|||
|
|
@ -11,7 +11,8 @@ import (
|
|||
)
|
||||
|
||||
func TestResolveConfig(t *testing.T) {
|
||||
// Clear environment variables to start clean
|
||||
// Isolate from real config file and environment
|
||||
t.Setenv("HOME", t.TempDir())
|
||||
os.Unsetenv("UNIFI_URL")
|
||||
os.Unsetenv("UNIFI_USER")
|
||||
os.Unsetenv("UNIFI_PASS")
|
||||
|
|
@ -74,6 +75,14 @@ func TestResolveConfig(t *testing.T) {
|
|||
}
|
||||
|
||||
func TestNewFromConfig(t *testing.T) {
|
||||
// Isolate from real config file and environment
|
||||
t.Setenv("HOME", t.TempDir())
|
||||
os.Unsetenv("UNIFI_URL")
|
||||
os.Unsetenv("UNIFI_USER")
|
||||
os.Unsetenv("UNIFI_PASS")
|
||||
os.Unsetenv("UNIFI_APIKEY")
|
||||
os.Unsetenv("UNIFI_INSECURE")
|
||||
|
||||
// Mock UniFi controller
|
||||
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
|
|
@ -88,8 +97,6 @@ func TestNewFromConfig(t *testing.T) {
|
|||
assert.Equal(t, ts.URL, client.URL())
|
||||
|
||||
// 2. Error case: No credentials
|
||||
os.Unsetenv("UNIFI_USER")
|
||||
os.Unsetenv("UNIFI_APIKEY")
|
||||
client, err = NewFromConfig("", "", "", "", nil)
|
||||
assert.Error(t, err)
|
||||
assert.Nil(t, client)
|
||||
|
|
|
|||
95
unifi/networks_test.go
Normal file
95
unifi/networks_test.go
Normal file
|
|
@ -0,0 +1,95 @@
|
|||
package unifi
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func TestGetNetworks(t *testing.T) {
|
||||
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
// New-style controller: paths are prefixed with /proxy/network
|
||||
if r.URL.Path == "/proxy/network/api/s/default/rest/networkconf" {
|
||||
fmt.Fprintln(w, `{"data":[{"_id":"abc123","name":"LAN","purpose":"corporate","ip_subnet":"10.69.1.0/24","vlan":0,"vlan_enabled":false,"enabled":true,"networkgroup":"LAN","dhcpd_enabled":true,"dhcpd_start":"10.69.1.100","dhcpd_stop":"10.69.1.254"}]}`)
|
||||
return
|
||||
}
|
||||
fmt.Fprintln(w, `{"meta":{"rc":"ok"}, "data": []}`)
|
||||
}))
|
||||
defer ts.Close()
|
||||
|
||||
client, err := New(ts.URL, "user", "pass", "", true)
|
||||
assert.NoError(t, err)
|
||||
|
||||
networks, err := client.GetNetworks("")
|
||||
assert.NoError(t, err)
|
||||
if assert.Len(t, networks, 1) {
|
||||
assert.Equal(t, "abc123", networks[0].ID)
|
||||
assert.Equal(t, "LAN", networks[0].Name)
|
||||
assert.Equal(t, "corporate", networks[0].Purpose)
|
||||
assert.Equal(t, "10.69.1.0/24", networks[0].IPSubnet)
|
||||
assert.True(t, networks[0].Enabled)
|
||||
assert.True(t, networks[0].DHCPEnabled)
|
||||
assert.Equal(t, "10.69.1.100", networks[0].DHCPStart)
|
||||
assert.Equal(t, "10.69.1.254", networks[0].DHCPStop)
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetNetworks_CustomSite(t *testing.T) {
|
||||
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
if r.URL.Path == "/proxy/network/api/s/office/rest/networkconf" {
|
||||
fmt.Fprintln(w, `{"data":[{"_id":"def456","name":"Office LAN","purpose":"corporate","ip_subnet":"172.16.0.0/24"}]}`)
|
||||
return
|
||||
}
|
||||
fmt.Fprintln(w, `{"meta":{"rc":"ok"}, "data": []}`)
|
||||
}))
|
||||
defer ts.Close()
|
||||
|
||||
client, err := New(ts.URL, "user", "pass", "", true)
|
||||
assert.NoError(t, err)
|
||||
|
||||
networks, err := client.GetNetworks("office")
|
||||
assert.NoError(t, err)
|
||||
if assert.Len(t, networks, 1) {
|
||||
assert.Equal(t, "Office LAN", networks[0].Name)
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetNetworks_Empty(t *testing.T) {
|
||||
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
fmt.Fprintln(w, `{"meta":{"rc":"ok"}, "data": []}`)
|
||||
}))
|
||||
defer ts.Close()
|
||||
|
||||
client, err := New(ts.URL, "user", "pass", "", true)
|
||||
assert.NoError(t, err)
|
||||
|
||||
networks, err := client.GetNetworks("")
|
||||
assert.NoError(t, err)
|
||||
assert.Empty(t, networks)
|
||||
}
|
||||
|
||||
func TestGetNetworks_InvalidJSON(t *testing.T) {
|
||||
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
if r.URL.Path == "/proxy/network/api/s/default/rest/networkconf" {
|
||||
fmt.Fprintln(w, `not valid json`)
|
||||
return
|
||||
}
|
||||
fmt.Fprintln(w, `{"meta":{"rc":"ok"}, "data": []}`)
|
||||
}))
|
||||
defer ts.Close()
|
||||
|
||||
client, err := New(ts.URL, "user", "pass", "", true)
|
||||
assert.NoError(t, err)
|
||||
|
||||
networks, err := client.GetNetworks("")
|
||||
assert.Error(t, err)
|
||||
assert.Nil(t, networks)
|
||||
assert.Contains(t, err.Error(), "failed to parse networks")
|
||||
}
|
||||
92
unifi/routes_test.go
Normal file
92
unifi/routes_test.go
Normal file
|
|
@ -0,0 +1,92 @@
|
|||
package unifi
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func TestRouteTypeName(t *testing.T) {
|
||||
tests := []struct {
|
||||
code string
|
||||
expected string
|
||||
}{
|
||||
{"S", "static"},
|
||||
{"C", "connected"},
|
||||
{"K", "kernel"},
|
||||
{"B", "bgp"},
|
||||
{"O", "ospf"},
|
||||
{"X", "X"},
|
||||
{"", ""},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
assert.Equal(t, tt.expected, RouteTypeName(tt.code))
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetRoutes(t *testing.T) {
|
||||
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
// New-style controller: paths are prefixed with /proxy/network
|
||||
if r.URL.Path == "/proxy/network/api/s/default/stat/routing" {
|
||||
fmt.Fprintln(w, `{"data":[{"pfx":"10.0.0.0/24","nh":"10.0.0.1","intf":"br0","type":"C","distance":0,"metric":0,"uptime":3600,"fib":true}]}`)
|
||||
return
|
||||
}
|
||||
fmt.Fprintln(w, `{"meta":{"rc":"ok"}, "data": []}`)
|
||||
}))
|
||||
defer ts.Close()
|
||||
|
||||
client, err := New(ts.URL, "user", "pass", "", true)
|
||||
assert.NoError(t, err)
|
||||
|
||||
routes, err := client.GetRoutes("")
|
||||
assert.NoError(t, err)
|
||||
if assert.Len(t, routes, 1) {
|
||||
assert.Equal(t, "10.0.0.0/24", routes[0].Network)
|
||||
assert.Equal(t, "10.0.0.1", routes[0].NextHop)
|
||||
assert.Equal(t, "br0", routes[0].Interface)
|
||||
assert.Equal(t, "C", routes[0].Type)
|
||||
assert.True(t, routes[0].Selected)
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetRoutes_CustomSite(t *testing.T) {
|
||||
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
if r.URL.Path == "/proxy/network/api/s/mysite/stat/routing" {
|
||||
fmt.Fprintln(w, `{"data":[{"pfx":"192.168.1.0/24","nh":"192.168.1.1","intf":"eth0","type":"S","distance":1,"metric":0,"uptime":0,"fib":true}]}`)
|
||||
return
|
||||
}
|
||||
fmt.Fprintln(w, `{"meta":{"rc":"ok"}, "data": []}`)
|
||||
}))
|
||||
defer ts.Close()
|
||||
|
||||
client, err := New(ts.URL, "user", "pass", "", true)
|
||||
assert.NoError(t, err)
|
||||
|
||||
routes, err := client.GetRoutes("mysite")
|
||||
assert.NoError(t, err)
|
||||
if assert.Len(t, routes, 1) {
|
||||
assert.Equal(t, "192.168.1.0/24", routes[0].Network)
|
||||
assert.Equal(t, "S", routes[0].Type)
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetRoutes_Empty(t *testing.T) {
|
||||
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
fmt.Fprintln(w, `{"meta":{"rc":"ok"}, "data": []}`)
|
||||
}))
|
||||
defer ts.Close()
|
||||
|
||||
client, err := New(ts.URL, "user", "pass", "", true)
|
||||
assert.NoError(t, err)
|
||||
|
||||
routes, err := client.GetRoutes("")
|
||||
assert.NoError(t, err)
|
||||
assert.Empty(t, routes)
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue