Skip to content

Commit 707f330

Browse files
committed
Address brixbench review feedback
Signed-off-by: Misun Park <misuneeh@gmail.com>
1 parent 49a39f7 commit 707f330

6 files changed

Lines changed: 66 additions & 10 deletions

File tree

brixbench/benchmark/runner_fallback_test.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@ limitations under the License.
1717
package benchmark
1818

1919
import (
20+
"context"
2021
"errors"
22+
"strings"
2123
"testing"
2224

2325
"github.com/vllm-project/aibrix/brixbench/internal/resolver"
@@ -142,3 +144,25 @@ func TestShouldRunDynamoStaleCleanupRequiresResetEnabled(t *testing.T) {
142144
})
143145
}
144146
}
147+
148+
func TestSetupAndRunDeploymentRejectsLLMdProvider(t *testing.T) {
149+
provider := "llmd"
150+
testCase := &resolver.Test{
151+
Name: "manual-llmd",
152+
Provider: &provider,
153+
}
154+
155+
deployer, gatewayURL, err := setupAndRunDeployment(context.Background(), t, t.TempDir(), testCase, "benchmark", t.TempDir())
156+
if err == nil {
157+
t.Fatalf("expected llmd not implemented error")
158+
}
159+
if deployer != nil {
160+
t.Fatalf("expected nil deployer, got %T", deployer)
161+
}
162+
if gatewayURL != "" {
163+
t.Fatalf("expected empty gateway URL, got %q", gatewayURL)
164+
}
165+
if !strings.Contains(err.Error(), "provider llmd is not implemented") {
166+
t.Fatalf("expected llmd not implemented error, got %v", err)
167+
}
168+
}

brixbench/benchmark/runner_test.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,8 +200,7 @@ func setupAndRunDeployment(ctx context.Context, t *testing.T, projectRoot string
200200
deployer = deployers.NewAIBrixDeployer()
201201
t.Log("Using AIBrix deployer")
202202
case "llmd":
203-
t.Log("Using LLM-d deployer")
204-
// return nil, "", fmt.Errorf("LLM-d deployer not implemented")
203+
return nil, "", fmt.Errorf("provider llmd is not implemented")
205204
case "dynamo":
206205
deployer = deployers.NewDynamoDeployer()
207206
t.Log("Using Dynamo deployer")

brixbench/benchmark/testdata/deployments/dynamo/qwen3-32b-round-robin-4p8d-vke.yaml

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -195,8 +195,7 @@ spec:
195195
--gpu-memory-utilization $GPU_MEM_UTIL \
196196
--tensor-parallel-size $TP \
197197
--disaggregation-mode decode \
198-
--kv-transfer-config '{"kv_connector":"NixlConnector","kv_role":"kv_both"}' \
199-
--enforce-eager
198+
--kv-transfer-config '{"kv_connector":"NixlConnector","kv_role":"kv_both"}'
200199
env:
201200
- name: DYN_LOG
202201
value: "info"
@@ -298,8 +297,7 @@ spec:
298297
--tensor-parallel-size $TP \
299298
--disaggregation-mode prefill \
300299
--kv-transfer-config '{"kv_connector":"NixlConnector","kv_role":"kv_both"}' \
301-
--kv-events-config '{"publisher":"zmq","topic":"kv-events","endpoint":"tcp://*:20080","enable_kv_cache_events":true}' \
302-
--enforce-eager
300+
--kv-events-config '{"publisher":"zmq","topic":"kv-events","endpoint":"tcp://*:20080","enable_kv_cache_events":true}'
303301
env:
304302
- name: DYN_LOG
305303
value: "info"

brixbench/benchmark/testdata/deployments/dynamo/qwen3-8b-round-robin-1p1d-tp2-vke.yaml

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,7 @@ spec:
169169
--gpu-memory-utilization $GPU_MEM_UTIL \
170170
--tensor-parallel-size $TP \
171171
--disaggregation-mode decode \
172-
--kv-transfer-config '{"kv_connector":"NixlConnector","kv_role":"kv_both"}' \
173-
--enforce-eager
172+
--kv-transfer-config '{"kv_connector":"NixlConnector","kv_role":"kv_both"}'
174173
env:
175174
- name: DYN_LOG
176175
value: "info"
@@ -255,8 +254,7 @@ spec:
255254
--tensor-parallel-size $TP \
256255
--disaggregation-mode prefill \
257256
--kv-transfer-config '{"kv_connector":"NixlConnector","kv_role":"kv_both"}' \
258-
--kv-events-config '{"publisher":"zmq","topic":"kv-events","endpoint":"tcp://*:20080","enable_kv_cache_events":true}' \
259-
--enforce-eager
257+
--kv-events-config '{"publisher":"zmq","topic":"kv-events","endpoint":"tcp://*:20080","enable_kv_cache_events":true}'
260258
env:
261259
- name: DYN_LOG
262260
value: "info"

brixbench/internal/resolver/provider_inputs.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ func validateProviderInputs(test *Test) error {
2121
}
2222

2323
func validateNullProviderSourceSelection(test *Test) error {
24+
if strings.TrimSpace(test.Version) != "" || strings.TrimSpace(test.Commit) != "" || len(test.ControlPlane) > 0 || strings.TrimSpace(test.Platform.ValuesFile) != "" {
25+
return fmt.Errorf("provider null does not support version, commit, controlplane, or platform inputs for %s", test.Name)
26+
}
2427
if strings.TrimSpace(test.LocalPath) != "" {
2528
return fmt.Errorf("localPath is only supported for provider aibrix in %s", test.Name)
2629
}

brixbench/internal/resolver/provider_inputs_test.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,40 @@ func TestValidateProviderInputsAcceptsNullProvider(t *testing.T) {
1313
}
1414
}
1515

16+
func TestValidateProviderInputsRejectsNullProviderUnsupportedInputs(t *testing.T) {
17+
for _, tc := range []struct {
18+
name string
19+
test Test
20+
}{
21+
{
22+
name: "version",
23+
test: Test{Name: "baseline", Version: "v0.6.0"},
24+
},
25+
{
26+
name: "commit",
27+
test: Test{Name: "baseline", Commit: "abcdef0"},
28+
},
29+
{
30+
name: "controlplane",
31+
test: Test{Name: "baseline", ControlPlane: []string{"controlplane.yaml"}},
32+
},
33+
{
34+
name: "platform values",
35+
test: Test{Name: "baseline", Platform: Platform{ValuesFile: "platform.yaml"}},
36+
},
37+
} {
38+
t.Run(tc.name, func(t *testing.T) {
39+
err := validateProviderInputs(&tc.test)
40+
if err == nil {
41+
t.Fatalf("expected provider null unsupported input error")
42+
}
43+
if !strings.Contains(err.Error(), "provider null does not support version, commit, controlplane, or platform inputs") {
44+
t.Fatalf("expected provider null unsupported input error, got %v", err)
45+
}
46+
})
47+
}
48+
}
49+
1650
func TestValidateProviderInputsRejectsLLMdProvider(t *testing.T) {
1751
provider := "llmd"
1852
test := Test{Name: "llmd", Provider: &provider}

0 commit comments

Comments
 (0)