Apply Task 1 review fixes
- Rename cmd/pipewiregen/internal/pipewiregen to normalize for package consistency - Wrap validation errors in parser.go with 'validation failed:' context - Add validation tests for empty library name, soname, group name, symbol name, and signature - Align README.md Go version (1.23+) with go.mod (1.23)
- commit
- 9f72b603c7644f8d698eebe24e147d9572660d0b
- author
- brice · · 2026-04-09 05:18 UTC
- parent
- 5b5bc74
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
9 files · +112 -2
M README.md +1 -1
| @@ -27,5 +27,5 @@ Mocks are generated by [mockery](https://github.com/vektra/mockery): | |||
| 27 | 27 | ||
| 28 | 28 | ## Requirements | |
| 29 | 29 | ||
| 30 | - | - Go 1.24+ | |
| 30 | + | - Go 1.23+ | |
| 31 | 31 | - Linux with PipeWire 0.3 runtime libraries | |
| @@ -27,5 +27,5 @@ Mocks are generated by [mockery](https://github.com/vektra/mockery): | |||||
| 27 | 27 | ||||
| 28 | ## Requirements | 28 | ## Requirements | ||
| 29 | 29 | ||||
| 30 | - | - Go 1.24+ | 30 | + | - Go 1.23+ |
| 31 | - Linux with PipeWire 0.3 runtime libraries | 31 | - Linux with PipeWire 0.3 runtime libraries | ||
R cmd/pipewiregen/internal/pipewiregen/normalize.go → cmd/pipewiregen/internal/normalize/normalize.go +0 -0 renamed, 100% similar
renamed, 100% similar
M cmd/pipewiregen/internal/parser/parser.go +1 -1
| @@ -19,7 +19,7 @@ func Load(path string) (*model.Model, error) { | |||
| 19 | 19 | return nil, fmt.Errorf("failed to unmarshal JSON from %s: %w", path, err) | |
| 20 | 20 | } | |
| 21 | 21 | if err := validate(&m); err != nil { | |
| 22 | - | return nil, err | |
| 22 | + | return nil, fmt.Errorf("validation failed: %w", err) | |
| 23 | 23 | } | |
| 24 | 24 | return &m, nil | |
| 25 | 25 | } | |
| @@ -19,7 +19,7 @@ func Load(path string) (*model.Model, error) { | |||||
| 19 | return nil, fmt.Errorf("failed to unmarshal JSON from %s: %w", path, err) | 19 | return nil, fmt.Errorf("failed to unmarshal JSON from %s: %w", path, err) | ||
| 20 | } | 20 | } | ||
| 21 | if err := validate(&m); err != nil { | 21 | if err := validate(&m); err != nil { | ||
| 22 | - | return nil, err | 22 | + | return nil, fmt.Errorf("validation failed: %w", err) |
| 23 | } | 23 | } | ||
| 24 | return &m, nil | 24 | return &m, nil | ||
| 25 | } | 25 | } | ||
M cmd/pipewiregen/internal/parser/parser_test.go +65 -0
| @@ -47,3 +47,68 @@ func TestLoadSymbolWithUnknownLibraryReturnsError(t *testing.T) { | |||
| 47 | 47 | t.Fatalf("error message should contain 'unknown library', got: %v", err) | |
| 48 | 48 | } | |
| 49 | 49 | } | |
| 50 | + | ||
| 51 | + | func TestLoadEmptyLibraryNameReturnsError(t *testing.T) { | |
| 52 | + | _, err := Load("testdata/empty_lib_name.json") | |
| 53 | + | if err == nil { | |
| 54 | + | t.Fatal("expected error for empty library name, got nil") | |
| 55 | + | } | |
| 56 | + | if !strings.Contains(err.Error(), "validation failed") { | |
| 57 | + | t.Fatalf("error message should contain 'validation failed', got: %v", err) | |
| 58 | + | } | |
| 59 | + | if !strings.Contains(err.Error(), "empty name") { | |
| 60 | + | t.Fatalf("error message should contain 'empty name', got: %v", err) | |
| 61 | + | } | |
| 62 | + | } | |
| 63 | + | ||
| 64 | + | func TestLoadEmptyLibrarySONameReturnsError(t *testing.T) { | |
| 65 | + | _, err := Load("testdata/empty_lib_soname.json") | |
| 66 | + | if err == nil { | |
| 67 | + | t.Fatal("expected error for empty library soname, got nil") | |
| 68 | + | } | |
| 69 | + | if !strings.Contains(err.Error(), "validation failed") { | |
| 70 | + | t.Fatalf("error message should contain 'validation failed', got: %v", err) | |
| 71 | + | } | |
| 72 | + | if !strings.Contains(err.Error(), "empty soname") { | |
| 73 | + | t.Fatalf("error message should contain 'empty soname', got: %v", err) | |
| 74 | + | } | |
| 75 | + | } | |
| 76 | + | ||
| 77 | + | func TestLoadEmptyGroupNameReturnsError(t *testing.T) { | |
| 78 | + | _, err := Load("testdata/empty_group_name.json") | |
| 79 | + | if err == nil { | |
| 80 | + | t.Fatal("expected error for empty group name, got nil") | |
| 81 | + | } | |
| 82 | + | if !strings.Contains(err.Error(), "validation failed") { | |
| 83 | + | t.Fatalf("error message should contain 'validation failed', got: %v", err) | |
| 84 | + | } | |
| 85 | + | if !strings.Contains(err.Error(), "empty name") { | |
| 86 | + | t.Fatalf("error message should contain 'empty name', got: %v", err) | |
| 87 | + | } | |
| 88 | + | } | |
| 89 | + | ||
| 90 | + | func TestLoadEmptySymbolNameReturnsError(t *testing.T) { | |
| 91 | + | _, err := Load("testdata/empty_symbol_name.json") | |
| 92 | + | if err == nil { | |
| 93 | + | t.Fatal("expected error for empty symbol name, got nil") | |
| 94 | + | } | |
| 95 | + | if !strings.Contains(err.Error(), "validation failed") { | |
| 96 | + | t.Fatalf("error message should contain 'validation failed', got: %v", err) | |
| 97 | + | } | |
| 98 | + | if !strings.Contains(err.Error(), "empty name") { | |
| 99 | + | t.Fatalf("error message should contain 'empty name', got: %v", err) | |
| 100 | + | } | |
| 101 | + | } | |
| 102 | + | ||
| 103 | + | func TestLoadEmptySymbolSignatureReturnsError(t *testing.T) { | |
| 104 | + | _, err := Load("testdata/empty_symbol_sig.json") | |
| 105 | + | if err == nil { | |
| 106 | + | t.Fatal("expected error for empty symbol signature, got nil") | |
| 107 | + | } | |
| 108 | + | if !strings.Contains(err.Error(), "validation failed") { | |
| 109 | + | t.Fatalf("error message should contain 'validation failed', got: %v", err) | |
| 110 | + | } | |
| 111 | + | if !strings.Contains(err.Error(), "empty signature") { | |
| 112 | + | t.Fatalf("error message should contain 'empty signature', got: %v", err) | |
| 113 | + | } | |
| 114 | + | } | |
| @@ -47,3 +47,68 @@ func TestLoadSymbolWithUnknownLibraryReturnsError(t *testing.T) { | |||||
| 47 | t.Fatalf("error message should contain 'unknown library', got: %v", err) | 47 | t.Fatalf("error message should contain 'unknown library', got: %v", err) | ||
| 48 | } | 48 | } | ||
| 49 | } | 49 | } | ||
| 50 | + | ||||
| 51 | + | func TestLoadEmptyLibraryNameReturnsError(t *testing.T) { | |||
| 52 | + | _, err := Load("testdata/empty_lib_name.json") | |||
| 53 | + | if err == nil { | |||
| 54 | + | t.Fatal("expected error for empty library name, got nil") | |||
| 55 | + | } | |||
| 56 | + | if !strings.Contains(err.Error(), "validation failed") { | |||
| 57 | + | t.Fatalf("error message should contain 'validation failed', got: %v", err) | |||
| 58 | + | } | |||
| 59 | + | if !strings.Contains(err.Error(), "empty name") { | |||
| 60 | + | t.Fatalf("error message should contain 'empty name', got: %v", err) | |||
| 61 | + | } | |||
| 62 | + | } | |||
| 63 | + | ||||
| 64 | + | func TestLoadEmptyLibrarySONameReturnsError(t *testing.T) { | |||
| 65 | + | _, err := Load("testdata/empty_lib_soname.json") | |||
| 66 | + | if err == nil { | |||
| 67 | + | t.Fatal("expected error for empty library soname, got nil") | |||
| 68 | + | } | |||
| 69 | + | if !strings.Contains(err.Error(), "validation failed") { | |||
| 70 | + | t.Fatalf("error message should contain 'validation failed', got: %v", err) | |||
| 71 | + | } | |||
| 72 | + | if !strings.Contains(err.Error(), "empty soname") { | |||
| 73 | + | t.Fatalf("error message should contain 'empty soname', got: %v", err) | |||
| 74 | + | } | |||
| 75 | + | } | |||
| 76 | + | ||||
| 77 | + | func TestLoadEmptyGroupNameReturnsError(t *testing.T) { | |||
| 78 | + | _, err := Load("testdata/empty_group_name.json") | |||
| 79 | + | if err == nil { | |||
| 80 | + | t.Fatal("expected error for empty group name, got nil") | |||
| 81 | + | } | |||
| 82 | + | if !strings.Contains(err.Error(), "validation failed") { | |||
| 83 | + | t.Fatalf("error message should contain 'validation failed', got: %v", err) | |||
| 84 | + | } | |||
| 85 | + | if !strings.Contains(err.Error(), "empty name") { | |||
| 86 | + | t.Fatalf("error message should contain 'empty name', got: %v", err) | |||
| 87 | + | } | |||
| 88 | + | } | |||
| 89 | + | ||||
| 90 | + | func TestLoadEmptySymbolNameReturnsError(t *testing.T) { | |||
| 91 | + | _, err := Load("testdata/empty_symbol_name.json") | |||
| 92 | + | if err == nil { | |||
| 93 | + | t.Fatal("expected error for empty symbol name, got nil") | |||
| 94 | + | } | |||
| 95 | + | if !strings.Contains(err.Error(), "validation failed") { | |||
| 96 | + | t.Fatalf("error message should contain 'validation failed', got: %v", err) | |||
| 97 | + | } | |||
| 98 | + | if !strings.Contains(err.Error(), "empty name") { | |||
| 99 | + | t.Fatalf("error message should contain 'empty name', got: %v", err) | |||
| 100 | + | } | |||
| 101 | + | } | |||
| 102 | + | ||||
| 103 | + | func TestLoadEmptySymbolSignatureReturnsError(t *testing.T) { | |||
| 104 | + | _, err := Load("testdata/empty_symbol_sig.json") | |||
| 105 | + | if err == nil { | |||
| 106 | + | t.Fatal("expected error for empty symbol signature, got nil") | |||
| 107 | + | } | |||
| 108 | + | if !strings.Contains(err.Error(), "validation failed") { | |||
| 109 | + | t.Fatalf("error message should contain 'validation failed', got: %v", err) | |||
| 110 | + | } | |||
| 111 | + | if !strings.Contains(err.Error(), "empty signature") { | |||
| 112 | + | t.Fatalf("error message should contain 'empty signature', got: %v", err) | |||
| 113 | + | } | |||
| 114 | + | } | |||
A cmd/pipewiregen/internal/parser/testdata/empty_group_name.json +9 -0
| @@ -0,0 +1,9 @@ | |||
| 1 | + | { | |
| 2 | + | "libraries": [ | |
| 3 | + | {"name": "pipewire", "soname": "libpipewire-0.3.so.0"} | |
| 4 | + | ], | |
| 5 | + | "groups": [ | |
| 6 | + | {"name": "", "symbols": []} | |
| 7 | + | ], | |
| 8 | + | "symbols": [] | |
| 9 | + | } | |
| @@ -0,0 +1,9 @@ | |||||
| 1 | + | { | |||
| 2 | + | "libraries": [ | |||
| 3 | + | {"name": "pipewire", "soname": "libpipewire-0.3.so.0"} | |||
| 4 | + | ], | |||
| 5 | + | "groups": [ | |||
| 6 | + | {"name": "", "symbols": []} | |||
| 7 | + | ], | |||
| 8 | + | "symbols": [] | |||
| 9 | + | } | |||
A cmd/pipewiregen/internal/parser/testdata/empty_lib_name.json +7 -0
| @@ -0,0 +1,7 @@ | |||
| 1 | + | { | |
| 2 | + | "libraries": [ | |
| 3 | + | {"name": "", "soname": "libpipewire-0.3.so.0"} | |
| 4 | + | ], | |
| 5 | + | "groups": [], | |
| 6 | + | "symbols": [] | |
| 7 | + | } | |
| @@ -0,0 +1,7 @@ | |||||
| 1 | + | { | |||
| 2 | + | "libraries": [ | |||
| 3 | + | {"name": "", "soname": "libpipewire-0.3.so.0"} | |||
| 4 | + | ], | |||
| 5 | + | "groups": [], | |||
| 6 | + | "symbols": [] | |||
| 7 | + | } | |||
A cmd/pipewiregen/internal/parser/testdata/empty_lib_soname.json +7 -0
| @@ -0,0 +1,7 @@ | |||
| 1 | + | { | |
| 2 | + | "libraries": [ | |
| 3 | + | {"name": "pipewire", "soname": ""} | |
| 4 | + | ], | |
| 5 | + | "groups": [], | |
| 6 | + | "symbols": [] | |
| 7 | + | } | |
| @@ -0,0 +1,7 @@ | |||||
| 1 | + | { | |||
| 2 | + | "libraries": [ | |||
| 3 | + | {"name": "pipewire", "soname": ""} | |||
| 4 | + | ], | |||
| 5 | + | "groups": [], | |||
| 6 | + | "symbols": [] | |||
| 7 | + | } | |||
A cmd/pipewiregen/internal/parser/testdata/empty_symbol_name.json +11 -0
| @@ -0,0 +1,11 @@ | |||
| 1 | + | { | |
| 2 | + | "libraries": [ | |
| 3 | + | {"name": "pipewire", "soname": "libpipewire-0.3.so.0"} | |
| 4 | + | ], | |
| 5 | + | "groups": [ | |
| 6 | + | {"name": "init", "symbols": [""]} | |
| 7 | + | ], | |
| 8 | + | "symbols": [ | |
| 9 | + | {"name": "", "library": "pipewire", "signature": "func()"} | |
| 10 | + | ] | |
| 11 | + | } | |
| @@ -0,0 +1,11 @@ | |||||
| 1 | + | { | |||
| 2 | + | "libraries": [ | |||
| 3 | + | {"name": "pipewire", "soname": "libpipewire-0.3.so.0"} | |||
| 4 | + | ], | |||
| 5 | + | "groups": [ | |||
| 6 | + | {"name": "init", "symbols": [""]} | |||
| 7 | + | ], | |||
| 8 | + | "symbols": [ | |||
| 9 | + | {"name": "", "library": "pipewire", "signature": "func()"} | |||
| 10 | + | ] | |||
| 11 | + | } | |||
A cmd/pipewiregen/internal/parser/testdata/empty_symbol_sig.json +11 -0
| @@ -0,0 +1,11 @@ | |||
| 1 | + | { | |
| 2 | + | "libraries": [ | |
| 3 | + | {"name": "pipewire", "soname": "libpipewire-0.3.so.0"} | |
| 4 | + | ], | |
| 5 | + | "groups": [ | |
| 6 | + | {"name": "init", "symbols": ["pw_init"]} | |
| 7 | + | ], | |
| 8 | + | "symbols": [ | |
| 9 | + | {"name": "pw_init", "library": "pipewire", "signature": ""} | |
| 10 | + | ] | |
| 11 | + | } | |
| @@ -0,0 +1,11 @@ | |||||
| 1 | + | { | |||
| 2 | + | "libraries": [ | |||
| 3 | + | {"name": "pipewire", "soname": "libpipewire-0.3.so.0"} | |||
| 4 | + | ], | |||
| 5 | + | "groups": [ | |||
| 6 | + | {"name": "init", "symbols": ["pw_init"]} | |||
| 7 | + | ], | |||
| 8 | + | "symbols": [ | |||
| 9 | + | {"name": "pw_init", "library": "pipewire", "signature": ""} | |||
| 10 | + | ] | |||
| 11 | + | } | |||