chore(agentnetwork): address automated review feedback

Drain the peer update channels after the bootstrap in the realstack
test so its fan-out assertions can only be satisfied by the operation
under test; use assert with a length guard for diffMappings conditions
under test; drop the duplicate labelgen package comment and two stale
provider-bootstrap comments in the e2e suite; add context to the
exclusion test's account-ID assertion.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Brad Ison
2026-08-07 12:49:31 +02:00
parent 8143016a95
commit c70d9e2df4
6 changed files with 28 additions and 20 deletions

View File

@@ -113,7 +113,7 @@ func runPathRoutedGuardrailCase(t *testing.T, tc pathRoutedGuardrailCase) {
// Catch-all provider (no models) so the router forwards any model; a static
// bearer key means the router injects a static auth header instead of minting
// a GCP token. Bootstraps the cluster if it isn't already.
// a GCP token.
staticKey := "static-e2e-token"
prov, err := srv.CreateProvider(ctx, api.AgentNetworkProviderRequest{
Name: tc.name,

View File

@@ -61,7 +61,7 @@ func TestGuardrailMultiPolicyModelAllowlist(t *testing.T) {
}
// pRestricted declares the two guardrailed models so routing is deterministic
// (model -> provider). Created first, so it carries the bootstrap cluster.
// (model -> provider).
pRestricted, err := srv.CreateProvider(ctx, api.AgentNetworkProviderRequest{
Name: "restricted",
ProviderId: "openai_api",

View File

@@ -1,14 +1,12 @@
// Package labelgen produces DNS-safe Agent Network subdomain labels.
//
// The adjective pool below pairs with the noun pool in words.go to form
// `<adjective>-<noun>` labels. It is kept separate because words.go is almost
// entirely nouns — drawing both halves from it produced unreadable pairs like
// "millet-hammock". Entries are lowercase ASCII, 4-12 chars, free of hyphens
// and digits, screened for offensive/brand/region-specific terms, and disjoint
// from the noun pool (enforced by TestAdjectives_AreDisjointFromNouns).
package labelgen
// adjectives is the descriptor half of a generated label.
// adjectives is the descriptor half of a generated label. It pairs with the
// noun pool in words.go to form `<adjective>-<noun>` labels, and is kept
// separate because words.go is almost entirely nouns — drawing both halves
// from it produced unreadable pairs like "millet-hammock". Entries are
// lowercase ASCII, 4-12 chars, free of hyphens and digits, screened for
// offensive/brand/region-specific terms, and disjoint from the noun pool
// (enforced by TestAdjectives_AreDisjointFromNouns).
var adjectives = []string{
"able", "active", "adept", "agile", "airy", "alert", "amiable", "ample",
"ancient", "ardent", "artful", "astute", "balmy", "blithe", "bold", "bonny",

View File

@@ -234,10 +234,12 @@ func TestDiffMappings_ServingProxyChange(t *testing.T) {
creates, updates, deletes := diffMappings(previous, current)
require.Len(t, deletes, 1, "the old proxy must be told to drop the mapping")
assert.Equal(t, "proxy.example.com", deletes[0].cluster)
require.Len(t, creates, 1, "the new proxy must be told to add it")
assert.Equal(t, "brave-otter.gateway.example.com", creates[0].cluster)
if assert.Len(t, deletes, 1, "the old proxy must be told to drop the mapping") {
assert.Equal(t, "proxy.example.com", deletes[0].cluster)
}
if assert.Len(t, creates, 1, "the new proxy must be told to add it") {
assert.Equal(t, "brave-otter.gateway.example.com", creates[0].cluster)
}
assert.Empty(t, updates, "a serving-proxy move is a delete plus a create, not an update")
}
@@ -261,8 +263,9 @@ func TestDiffMappings_UnchangedClusterIsAnUpdate(t *testing.T) {
assert.Empty(t, creates)
assert.Empty(t, deletes)
require.Len(t, updates, 1)
assert.Equal(t, "proxy.example.com", updates[0].cluster)
if assert.Len(t, updates, 1) {
assert.Equal(t, "proxy.example.com", updates[0].cluster)
}
}
// TestDiffMappings_RemovedServiceIsDeletedOnItsOwnCluster — a service that has
@@ -280,6 +283,7 @@ func TestDiffMappings_RemovedServiceIsDeletedOnItsOwnCluster(t *testing.T) {
assert.Empty(t, creates)
assert.Empty(t, updates)
require.Len(t, deletes, 1)
assert.Equal(t, "brave-otter.gateway.example.com", deletes[0].cluster)
if assert.Len(t, deletes, 1) {
assert.Equal(t, "brave-otter.gateway.example.com", deletes[0].cluster)
}
}

View File

@@ -220,7 +220,8 @@ func TestGetClusterAllowList_DedicatedGatewayAddressExcluded(t *testing.T) {
}
st := &stubStore{
getAgentNetworkSettingsFunc: func(_ context.Context, accountID string) (*agentnetworkTypes.Settings, error) {
assert.Equal(t, "acc-123", accountID)
assert.Equal(t, "acc-123", accountID,
"the exclusion must look up the requesting account's own settings")
return &agentnetworkTypes.Settings{
AccountID: accountID,
Domain: "brave-otter.gateway.example.com",

View File

@@ -94,6 +94,11 @@ func TestAgentNetwork_ProviderCRUD_FansOutToProxyAndClientPeers(t *testing.T) {
_, err = agentMgr.CreateSettings(ctx, adminUserID, agenttypes.DefaultSettings(accountID), clusterAddr, "")
require.NoError(t, err, "CreateSettings must bootstrap the endpoint")
// The bootstrap itself reconciles and queues updates on both channels;
// drain them so the fan-out assertions below can only be satisfied by the
// operation under test, not by this leftover.
drain(clientCh)
drain(proxyCh)
provider, err := agentMgr.CreateProvider(ctx, adminUserID, &agenttypes.Provider{
AccountID: accountID,