fix(dx): audit coding standards and add tests for untested paths
- CLAUDE.md: document coreerr.E() error handling and go-io exclusion - server_test.go: replace fmt.Errorf with coreerr.E() in test fixtures - gguf_test.go: add tests for v2 format, skipValue (all type branches), readTypedValue uint64 path, unsupported version, truncated file - discover_test.go: add test for corrupt GGUF file skipping - vram_test.go: add tests for invalid/empty sysfs content Coverage: 65.8% → 79.2% (+13.4%) Co-Authored-By: Virgil <virgil@lethean.io>
This commit is contained in:
parent
5dc79971e2
commit
9aaa404397
5 changed files with 245 additions and 4 deletions
|
|
@ -68,6 +68,8 @@ sudo cp build/bin/llama-server /usr/local/bin/llama-server
|
||||||
- UK English
|
- UK English
|
||||||
- Tests: testify assert/require
|
- Tests: testify assert/require
|
||||||
- Build tags: `linux && amd64` for GPU code, `rocm` for integration tests
|
- Build tags: `linux && amd64` for GPU code, `rocm` for integration tests
|
||||||
|
- Errors: `coreerr.E("pkg.Func", "what failed", err)` via `go-log`, never `fmt.Errorf` or `errors.New`
|
||||||
|
- File I/O: `os` package used directly — `go-io` not imported (its transitive deps are too heavy for a GPU inference module)
|
||||||
- Conventional commits
|
- Conventional commits
|
||||||
- Co-Author: `Co-Authored-By: Virgil <virgil@lethean.io>`
|
- Co-Author: `Co-Authored-By: Virgil <virgil@lethean.io>`
|
||||||
- Licence: EUPL-1.2
|
- Licence: EUPL-1.2
|
||||||
|
|
|
||||||
|
|
@ -127,3 +127,23 @@ func TestDiscoverModels_NotFound(t *testing.T) {
|
||||||
require.NoError(t, err)
|
require.NoError(t, err)
|
||||||
assert.Empty(t, models)
|
assert.Empty(t, models)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestDiscoverModels_SkipsCorruptFile(t *testing.T) {
|
||||||
|
dir := t.TempDir()
|
||||||
|
|
||||||
|
// Create a valid GGUF file.
|
||||||
|
writeDiscoverTestGGUF(t, dir, "valid.gguf", [][2]any{
|
||||||
|
{"general.architecture", "llama"},
|
||||||
|
{"general.name", "Valid Model"},
|
||||||
|
{"general.file_type", uint32(15)},
|
||||||
|
})
|
||||||
|
|
||||||
|
// Create a corrupt .gguf file (not valid GGUF binary).
|
||||||
|
require.NoError(t, os.WriteFile(filepath.Join(dir, "corrupt.gguf"), []byte("not gguf data"), 0644))
|
||||||
|
|
||||||
|
models, err := DiscoverModels(dir)
|
||||||
|
require.NoError(t, err)
|
||||||
|
// Only the valid model should be returned; corrupt one is silently skipped.
|
||||||
|
require.Len(t, models, 1)
|
||||||
|
assert.Equal(t, "Valid Model", models[0].Name)
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -59,11 +59,56 @@ func writeKV(t *testing.T, f *os.File, key string, val any) {
|
||||||
// Type: 4 (uint32)
|
// Type: 4 (uint32)
|
||||||
require.NoError(t, binary.Write(f, binary.LittleEndian, uint32(4)))
|
require.NoError(t, binary.Write(f, binary.LittleEndian, uint32(4)))
|
||||||
require.NoError(t, binary.Write(f, binary.LittleEndian, v))
|
require.NoError(t, binary.Write(f, binary.LittleEndian, v))
|
||||||
|
case uint64:
|
||||||
|
// Type: 10 (uint64)
|
||||||
|
require.NoError(t, binary.Write(f, binary.LittleEndian, uint32(10)))
|
||||||
|
require.NoError(t, binary.Write(f, binary.LittleEndian, v))
|
||||||
default:
|
default:
|
||||||
t.Fatalf("writeKV: unsupported value type %T", val)
|
t.Fatalf("writeKV: unsupported value type %T", val)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// writeRawKV writes a key with a specific GGUF type and raw byte payload.
|
||||||
|
// Used to test skipValue for types not used in interesting keys.
|
||||||
|
func writeRawKV(t *testing.T, f *os.File, key string, valType uint32, rawVal []byte) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
require.NoError(t, binary.Write(f, binary.LittleEndian, uint64(len(key))))
|
||||||
|
_, err := f.Write([]byte(key))
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.NoError(t, binary.Write(f, binary.LittleEndian, valType))
|
||||||
|
_, err = f.Write(rawVal)
|
||||||
|
require.NoError(t, err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// writeTestGGUFV2 creates a synthetic GGUF v2 file (uint32 tensor/kv counts).
|
||||||
|
func writeTestGGUFV2(t *testing.T, kvs [][2]any) string {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
dir := t.TempDir()
|
||||||
|
path := filepath.Join(dir, "test_v2.gguf")
|
||||||
|
|
||||||
|
f, err := os.Create(path)
|
||||||
|
require.NoError(t, err)
|
||||||
|
defer f.Close()
|
||||||
|
|
||||||
|
// Magic
|
||||||
|
require.NoError(t, binary.Write(f, binary.LittleEndian, uint32(0x46554747)))
|
||||||
|
// Version 2
|
||||||
|
require.NoError(t, binary.Write(f, binary.LittleEndian, uint32(2)))
|
||||||
|
// Tensor count (uint32 for v2): 0
|
||||||
|
require.NoError(t, binary.Write(f, binary.LittleEndian, uint32(0)))
|
||||||
|
// KV count (uint32 for v2)
|
||||||
|
require.NoError(t, binary.Write(f, binary.LittleEndian, uint32(len(kvs))))
|
||||||
|
|
||||||
|
for _, kv := range kvs {
|
||||||
|
key := kv[0].(string)
|
||||||
|
writeKV(t, f, key, kv[1])
|
||||||
|
}
|
||||||
|
|
||||||
|
return path
|
||||||
|
}
|
||||||
|
|
||||||
func TestReadMetadata_Gemma3(t *testing.T) {
|
func TestReadMetadata_Gemma3(t *testing.T) {
|
||||||
path := writeTestGGUFOrdered(t, [][2]any{
|
path := writeTestGGUFOrdered(t, [][2]any{
|
||||||
{"general.architecture", "gemma3"},
|
{"general.architecture", "gemma3"},
|
||||||
|
|
@ -153,3 +198,159 @@ func TestFileTypeName(t *testing.T) {
|
||||||
assert.Equal(t, "F16", FileTypeName(1))
|
assert.Equal(t, "F16", FileTypeName(1))
|
||||||
assert.Equal(t, "type_999", FileTypeName(999))
|
assert.Equal(t, "type_999", FileTypeName(999))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestReadMetadata_V2(t *testing.T) {
|
||||||
|
// GGUF v2 uses uint32 for tensor and KV counts (instead of uint64 in v3).
|
||||||
|
path := writeTestGGUFV2(t, [][2]any{
|
||||||
|
{"general.architecture", "llama"},
|
||||||
|
{"general.name", "V2 Model"},
|
||||||
|
{"general.file_type", uint32(15)},
|
||||||
|
{"llama.context_length", uint32(2048)},
|
||||||
|
{"llama.block_count", uint32(16)},
|
||||||
|
})
|
||||||
|
|
||||||
|
m, err := ReadMetadata(path)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
assert.Equal(t, "llama", m.Architecture)
|
||||||
|
assert.Equal(t, "V2 Model", m.Name)
|
||||||
|
assert.Equal(t, uint32(15), m.FileType)
|
||||||
|
assert.Equal(t, uint32(2048), m.ContextLength)
|
||||||
|
assert.Equal(t, uint32(16), m.BlockCount)
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestReadMetadata_UnsupportedVersion(t *testing.T) {
|
||||||
|
dir := t.TempDir()
|
||||||
|
path := filepath.Join(dir, "bad_version.gguf")
|
||||||
|
|
||||||
|
f, err := os.Create(path)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
require.NoError(t, binary.Write(f, binary.LittleEndian, uint32(0x46554747))) // magic
|
||||||
|
require.NoError(t, binary.Write(f, binary.LittleEndian, uint32(99))) // invalid version
|
||||||
|
f.Close()
|
||||||
|
|
||||||
|
_, err = ReadMetadata(path)
|
||||||
|
require.Error(t, err)
|
||||||
|
assert.Contains(t, err.Error(), "unsupported GGUF version")
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestReadMetadata_SkipsUnknownValueTypes(t *testing.T) {
|
||||||
|
// Tests skipValue for uint8, int16, float32, uint64, bool, and array types.
|
||||||
|
// These are stored under uninteresting keys so ReadMetadata skips them.
|
||||||
|
dir := t.TempDir()
|
||||||
|
path := filepath.Join(dir, "skip_types.gguf")
|
||||||
|
|
||||||
|
f, err := os.Create(path)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
// Header: magic, v3, 0 tensors
|
||||||
|
require.NoError(t, binary.Write(f, binary.LittleEndian, uint32(0x46554747)))
|
||||||
|
require.NoError(t, binary.Write(f, binary.LittleEndian, uint32(3)))
|
||||||
|
require.NoError(t, binary.Write(f, binary.LittleEndian, uint64(0)))
|
||||||
|
// 8 KV pairs: 6 skip types + 2 interesting keys
|
||||||
|
require.NoError(t, binary.Write(f, binary.LittleEndian, uint64(8)))
|
||||||
|
|
||||||
|
// 1. uint8 (type 0) — 1 byte
|
||||||
|
raw := make([]byte, 1)
|
||||||
|
raw[0] = 42
|
||||||
|
writeRawKV(t, f, "custom.uint8_val", 0, raw)
|
||||||
|
|
||||||
|
// 2. bool (type 7) — 1 byte
|
||||||
|
raw = []byte{1}
|
||||||
|
writeRawKV(t, f, "custom.bool_val", 7, raw)
|
||||||
|
|
||||||
|
// 3. int16 (type 3) — 2 bytes
|
||||||
|
raw = make([]byte, 2)
|
||||||
|
binary.LittleEndian.PutUint16(raw, 1234)
|
||||||
|
writeRawKV(t, f, "custom.int16_val", 3, raw)
|
||||||
|
|
||||||
|
// 4. float32 (type 6) — 4 bytes
|
||||||
|
raw = make([]byte, 4)
|
||||||
|
binary.LittleEndian.PutUint32(raw, 0x3F800000) // 1.0
|
||||||
|
writeRawKV(t, f, "custom.float32_val", 6, raw)
|
||||||
|
|
||||||
|
// 5. uint64 (type 10) — 8 bytes
|
||||||
|
raw = make([]byte, 8)
|
||||||
|
binary.LittleEndian.PutUint64(raw, 9999)
|
||||||
|
writeRawKV(t, f, "custom.uint64_val", 10, raw)
|
||||||
|
|
||||||
|
// 6. array of uint8 (type 9, element type 0, count 3)
|
||||||
|
var arrBuf []byte
|
||||||
|
b4 := make([]byte, 4)
|
||||||
|
binary.LittleEndian.PutUint32(b4, 0) // element type: uint8
|
||||||
|
arrBuf = append(arrBuf, b4...)
|
||||||
|
b8 := make([]byte, 8)
|
||||||
|
binary.LittleEndian.PutUint64(b8, 3) // count: 3
|
||||||
|
arrBuf = append(arrBuf, b8...)
|
||||||
|
arrBuf = append(arrBuf, 10, 20, 30) // 3 uint8 values
|
||||||
|
writeRawKV(t, f, "custom.array_val", 9, arrBuf)
|
||||||
|
|
||||||
|
// 7-8. Interesting keys to verify parsing continued correctly.
|
||||||
|
writeKV(t, f, "general.architecture", "llama")
|
||||||
|
writeKV(t, f, "general.name", "Skip Test Model")
|
||||||
|
|
||||||
|
f.Close()
|
||||||
|
|
||||||
|
m, err := ReadMetadata(path)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
assert.Equal(t, "llama", m.Architecture)
|
||||||
|
assert.Equal(t, "Skip Test Model", m.Name)
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestReadMetadata_Uint64ContextLength(t *testing.T) {
|
||||||
|
// context_length stored as uint64 that fits in uint32 — readTypedValue
|
||||||
|
// should downcast it to uint32.
|
||||||
|
path := writeTestGGUFOrdered(t, [][2]any{
|
||||||
|
{"general.architecture", "llama"},
|
||||||
|
{"llama.context_length", uint64(8192)},
|
||||||
|
{"llama.block_count", uint64(32)},
|
||||||
|
})
|
||||||
|
|
||||||
|
m, err := ReadMetadata(path)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
assert.Equal(t, uint32(8192), m.ContextLength)
|
||||||
|
assert.Equal(t, uint32(32), m.BlockCount)
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestReadMetadata_TruncatedFile(t *testing.T) {
|
||||||
|
dir := t.TempDir()
|
||||||
|
path := filepath.Join(dir, "truncated.gguf")
|
||||||
|
|
||||||
|
// Write only the magic — no version or counts.
|
||||||
|
f, err := os.Create(path)
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.NoError(t, binary.Write(f, binary.LittleEndian, uint32(0x46554747)))
|
||||||
|
f.Close()
|
||||||
|
|
||||||
|
_, err = ReadMetadata(path)
|
||||||
|
require.Error(t, err)
|
||||||
|
assert.Contains(t, err.Error(), "reading version")
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestReadMetadata_SkipsStringValue(t *testing.T) {
|
||||||
|
// Tests skipValue for string type (type 8) on an uninteresting key.
|
||||||
|
dir := t.TempDir()
|
||||||
|
path := filepath.Join(dir, "skip_string.gguf")
|
||||||
|
|
||||||
|
f, err := os.Create(path)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
require.NoError(t, binary.Write(f, binary.LittleEndian, uint32(0x46554747)))
|
||||||
|
require.NoError(t, binary.Write(f, binary.LittleEndian, uint32(3)))
|
||||||
|
require.NoError(t, binary.Write(f, binary.LittleEndian, uint64(0)))
|
||||||
|
require.NoError(t, binary.Write(f, binary.LittleEndian, uint64(2)))
|
||||||
|
|
||||||
|
// Uninteresting string key (exercises skipValue for typeString).
|
||||||
|
writeKV(t, f, "custom.description", "a long description value")
|
||||||
|
// Interesting key to confirm parsing continued.
|
||||||
|
writeKV(t, f, "general.architecture", "gemma3")
|
||||||
|
|
||||||
|
f.Close()
|
||||||
|
|
||||||
|
m, err := ReadMetadata(path)
|
||||||
|
require.NoError(t, err)
|
||||||
|
assert.Equal(t, "gemma3", m.Architecture)
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -4,12 +4,12 @@ package rocm
|
||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
"fmt"
|
|
||||||
"os"
|
"os"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
"forge.lthn.ai/core/go-inference"
|
"forge.lthn.ai/core/go-inference"
|
||||||
|
coreerr "forge.lthn.ai/core/go-log"
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
"github.com/stretchr/testify/require"
|
"github.com/stretchr/testify/require"
|
||||||
)
|
)
|
||||||
|
|
@ -90,7 +90,7 @@ func TestServerAlive_Running(t *testing.T) {
|
||||||
func TestServerAlive_Exited(t *testing.T) {
|
func TestServerAlive_Exited(t *testing.T) {
|
||||||
exited := make(chan struct{})
|
exited := make(chan struct{})
|
||||||
close(exited)
|
close(exited)
|
||||||
s := &server{exited: exited, exitErr: fmt.Errorf("process killed")}
|
s := &server{exited: exited, exitErr: coreerr.E("test", "process killed", nil)}
|
||||||
assert.False(t, s.alive())
|
assert.False(t, s.alive())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -99,7 +99,7 @@ func TestGenerate_ServerDead(t *testing.T) {
|
||||||
close(exited)
|
close(exited)
|
||||||
s := &server{
|
s := &server{
|
||||||
exited: exited,
|
exited: exited,
|
||||||
exitErr: fmt.Errorf("process killed"),
|
exitErr: coreerr.E("test", "process killed", nil),
|
||||||
}
|
}
|
||||||
m := &rocmModel{srv: s}
|
m := &rocmModel{srv: s}
|
||||||
|
|
||||||
|
|
@ -124,7 +124,7 @@ func TestChat_ServerDead(t *testing.T) {
|
||||||
close(exited)
|
close(exited)
|
||||||
s := &server{
|
s := &server{
|
||||||
exited: exited,
|
exited: exited,
|
||||||
exitErr: fmt.Errorf("process killed"),
|
exitErr: coreerr.E("test", "process killed", nil),
|
||||||
}
|
}
|
||||||
m := &rocmModel{srv: s}
|
m := &rocmModel{srv: s}
|
||||||
|
|
||||||
|
|
|
||||||
18
vram_test.go
18
vram_test.go
|
|
@ -26,6 +26,24 @@ func TestReadSysfsUint64_NotFound(t *testing.T) {
|
||||||
assert.Error(t, err)
|
assert.Error(t, err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestReadSysfsUint64_InvalidContent(t *testing.T) {
|
||||||
|
dir := t.TempDir()
|
||||||
|
path := filepath.Join(dir, "bad_value")
|
||||||
|
require.NoError(t, os.WriteFile(path, []byte("not-a-number\n"), 0644))
|
||||||
|
|
||||||
|
_, err := readSysfsUint64(path)
|
||||||
|
assert.Error(t, err)
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestReadSysfsUint64_EmptyFile(t *testing.T) {
|
||||||
|
dir := t.TempDir()
|
||||||
|
path := filepath.Join(dir, "empty_value")
|
||||||
|
require.NoError(t, os.WriteFile(path, []byte(""), 0644))
|
||||||
|
|
||||||
|
_, err := readSysfsUint64(path)
|
||||||
|
assert.Error(t, err)
|
||||||
|
}
|
||||||
|
|
||||||
func TestGetVRAMInfo(t *testing.T) {
|
func TestGetVRAMInfo(t *testing.T) {
|
||||||
info, err := GetVRAMInfo()
|
info, err := GetVRAMInfo()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue