Skip to content

Commit e9926db

Browse files
ksamorayKobi Samorayclaude
authored
Expand unit test coverage for the cache layer and realization polling (#2316)
* Expand unit test coverage for low-coverage resources and data sources Raises overall unit test coverage on ./nsxt/... from 68.8% to 73.8%, building on the prior "near-zero-coverage data sources" pass: - 8 resources whose existing mock tests only exercised error/guard paths now cover Create/Read/Update/Delete success paths too: cluster_security_config (20% -> ~97%), ldap_identity_source (20% -> ~97%), l7_access_profile (24% -> ~89%), idps_cluster_config (25% -> ~97%), segment_port_profile_bindings (33% -> ~86%), host_transport_node (36% -> ~88%), ip_pool_block_subnet (37% -> ~90%), ipsec_vpn_session (41% -> ~74%). Adds minimal package-level client-constructor seams (mirroring the existing cli*Client convention used throughout the provider) to cluster_security_config and idps_cluster_config so their SDK clients can be mocked; reuses existing seams elsewhere. - 9 near-identical LB monitor/application profile resources gain success-path tests via their already-shared mock helpers, raising each from ~46-50% to 84-100%. - 13 data sources that previously had no mock test at all now have one, nearly all reaching 100%: 8 trivial generic-search wrappers, plus provider_info, host_upgrade_group, edge_cluster, gateway_interface, and proxy_config (the last reusing an existing seam from its sibling resource file instead of constructing its SDK client inline). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Expand unit test coverage for the cache layer and realization polling cache_test.go's tests predated the unittest tag convention and were named outside the TestMock*Nsxt*/TestUnitNsxt_* pattern that `make test-unit` (and CI) filters on via -run, so they never actually executed as part of the tracked unit test suite. Tag the file `//go:build unittest` and rename its tests to TestUnitNsxt_* so they're finally counted. Add nsxt/cache_unit_test.go covering the cache.go helpers that were still at 0%: query-string/query-key construction, the per-type/per-query cache buckets (hit, miss-and-populate, and post-write-bypass paths) including the composite gateway-policy/security-policy + rule merge, tag stripping and provider-managed-tag patching, and TryCacheRead/CacheAwareResourceRead/ cacheAwareDataSourceReadByID end to end using the existing cliQueryClient stub pattern from policy_search_unit_test.go. Also add coverage for nsxtPolicyWaitForRealizationStateConf (policy_utils.go), reusing the existing realization-info mock helper to exercise its Refresh closure (found, not-yet-realized, and error cases). Package coverage under `go test ./nsxt -tags=unittest -run='^(TestMock.*Nsxt.*|TestUnitNsxt_.*)' -coverpkg=./nsxt` moves from 74.0% to 74.8%, with cache.go itself going from mostly-0% to 75-100% coverage across nearly every function. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Kobi Samoray <kobis@Kobis-MacBook-Air.local> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 8efb254 commit e9926db

3 files changed

Lines changed: 778 additions & 15 deletions

File tree

nsxt/cache_test.go

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
//go:build unittest
2+
3+
// © Broadcom. All Rights Reserved.
4+
// The term "Broadcom" refers to Broadcom Inc. and/or its subsidiaries.
5+
// SPDX-License-Identifier: MPL-2.0
6+
17
package nsxt
28

39
import (
@@ -11,7 +17,7 @@ import (
1117
"github.com/vmware/vsphere-automation-sdk-go/services/nsxt/model"
1218
)
1319

14-
func TestProviderManagedTagsSearchQuery(t *testing.T) {
20+
func TestUnitNsxt_providerManagedTagsSearchQuery(t *testing.T) {
1521
if got := providerManagedTagsSearchQuery(""); got != "" {
1622
t.Fatalf("empty runID: want empty, got %q", got)
1723
}
@@ -27,7 +33,7 @@ func TestProviderManagedTagsSearchQuery(t *testing.T) {
2733
}
2834
}
2935

30-
func TestBuildTagQuery(t *testing.T) {
36+
func TestUnitNsxt_buildTagQuery(t *testing.T) {
3137
tagSchema := map[string]*schema.Schema{
3238
"tag": {
3339
Type: schema.TypeSet,
@@ -101,7 +107,7 @@ func TestBuildTagQuery(t *testing.T) {
101107
})
102108
}
103109

104-
func TestAttachRulesByParentPathSecurityPolicy(t *testing.T) {
110+
func TestUnitNsxt_attachRulesByParentPathSecurityPolicy(t *testing.T) {
105111
policyPathA := "/infra/domains/default/security-policies/pol-a"
106112
policyPathB := "/infra/domains/default/security-policies/pol-b"
107113

@@ -139,7 +145,7 @@ func TestAttachRulesByParentPathSecurityPolicy(t *testing.T) {
139145
})
140146
}
141147

142-
func TestEnsureProviderManagedTagsWithPatchFunc(t *testing.T) {
148+
func TestUnitNsxt_ensureProviderManagedTagsWithPatchFunc(t *testing.T) {
143149
type testTagObj struct {
144150
Tags []model.Tag
145151
}
@@ -218,7 +224,7 @@ func int64Ptr(v int64) *int64 {
218224
return &v
219225
}
220226

221-
func TestGroupRulesByValidParentPath(t *testing.T) {
227+
func TestUnitNsxt_groupRulesByValidParentPath(t *testing.T) {
222228
pathA := "/policies/a"
223229
pathB := "/policies/b"
224230
valid := map[string]struct{}{pathA: {}, pathB: {}}
@@ -286,7 +292,7 @@ func attachRulesToSecurityPoliciesForTest(parents []model.SecurityPolicy, rules
286292
)
287293
}
288294

289-
func TestAttachRulesByParentPathGatewayPolicy(t *testing.T) {
295+
func TestUnitNsxt_attachRulesByParentPathGatewayPolicy(t *testing.T) {
290296
policyPathA := "/orgs/p/proj/vpcs/vpc/gateway-policies/pol-a"
291297
policyPathB := "/orgs/p/proj/vpcs/vpc/gateway-policies/pol-b"
292298

@@ -391,7 +397,7 @@ func TestAttachRulesByParentPathGatewayPolicy(t *testing.T) {
391397
})
392398
}
393399

394-
func TestGetQueryStringVPCScopedToProjectNotVPC(t *testing.T) {
400+
func TestUnitNsxt_getQueryStringVPCScopedToProjectNotVPC(t *testing.T) {
395401
// VPCID must be omitted from the cache bucket key: NSX policy paths/IDs are unique
396402
// within a project across all VPCs, and narrowing the key (and the underlying search)
397403
// to a single VPC caused a fresh cache bucket per VPC, regressing cache mode below
@@ -414,7 +420,7 @@ func TestGetQueryStringVPCScopedToProjectNotVPC(t *testing.T) {
414420
}
415421
}
416422

417-
func TestProjectScopedSearchContextStripsVPCID(t *testing.T) {
423+
func TestUnitNsxt_projectScopedSearchContextStripsVPCID(t *testing.T) {
418424
for _, clientType := range []utl.ClientType{utl.VPC, utl.Multitenancy} {
419425
in := utl.SessionContext{ClientType: clientType, ProjectID: "proj-1", VPCID: "vpc-1"}
420426
out := projectScopedSearchContext(in)
@@ -433,7 +439,7 @@ func TestProjectScopedSearchContextStripsVPCID(t *testing.T) {
433439
}
434440
}
435441

436-
func TestShouldIndexByPathForVPCScopedTypes(t *testing.T) {
442+
func TestUnitNsxt_shouldIndexByPathForVPCScopedTypes(t *testing.T) {
437443
// VPC-scoped types must key by path, not short id: NSX ids for these types (often
438444
// user-chosen via nsx_id) are only guaranteed unique within their own VPC, but the cache
439445
// populate search/bucket for these types is now shared across all VPCs in a project.
@@ -456,7 +462,7 @@ func TestShouldIndexByPathForVPCScopedTypes(t *testing.T) {
456462
}
457463
}
458464

459-
func TestConverListToMapByTypeVpcScopedResourcesIndexedByPath(t *testing.T) {
465+
func TestUnitNsxt_converListToMapByTypeVpcScopedResourcesIndexedByPath(t *testing.T) {
460466
// Two different VPCs in the same project can legitimately have a VpcSubnet with the same
461467
// user-chosen short id (getOrGenerateID2 only checks uniqueness within the current VPC).
462468
// Since the cache bucket for VPC-scoped types is now shared project-wide, both objects
@@ -484,7 +490,7 @@ func TestConverListToMapByTypeVpcScopedResourcesIndexedByPath(t *testing.T) {
484490
}
485491
}
486492

487-
func TestErrCacheUseBackendDirect(t *testing.T) {
493+
func TestUnitNsxt_errCacheUseBackendDirect(t *testing.T) {
488494
if !errors.Is(errCacheUseBackendDirect, errCacheUseBackendDirect) {
489495
t.Fatal("errors.Is should match sentinel to itself")
490496
}
@@ -494,7 +500,7 @@ func TestErrCacheUseBackendDirect(t *testing.T) {
494500
}
495501
}
496502

497-
func TestReflectStringField(t *testing.T) {
503+
func TestUnitNsxt_reflectStringField(t *testing.T) {
498504
t.Run("returns-pointer-value", func(t *testing.T) {
499505
obj := &model.Group{DisplayName: strPtr("g1")}
500506
got := reflectStringField(obj, "DisplayName")
@@ -532,7 +538,7 @@ func TestReflectStringField(t *testing.T) {
532538
})
533539
}
534540

535-
func TestCacheAwareDataSourceReadByIDBypassesCacheForShortIDOnPathIndexedTypes(t *testing.T) {
541+
func TestUnitNsxt_cacheAwareDataSourceReadByIDBypassesCacheForShortIDOnPathIndexedTypes(t *testing.T) {
536542
dsSchema := map[string]*schema.Schema{
537543
"id": getDataSourceIDSchema(),
538544
"display_name": getDataSourceExtendedDisplayNameSchema(),
@@ -568,7 +574,7 @@ func TestCacheAwareDataSourceReadByIDBypassesCacheForShortIDOnPathIndexedTypes(t
568574
})
569575
}
570576

571-
func TestCacheAwareResourceReadBypassesCacheForShortIDOnPathIndexedTypes(t *testing.T) {
577+
func TestUnitNsxt_cacheAwareResourceReadBypassesCacheForShortIDOnPathIndexedTypes(t *testing.T) {
572578
// CacheAwareResourceRead's resourceID is a short id (not yet path) during the Create-then-Read
573579
// sequence (path isn't set on d until the Read populates it from the live object) and after
574580
// terraform import (importers call d.SetId(shortID) without setting path). Without this bypass,
@@ -633,7 +639,7 @@ func TestCacheAwareResourceReadBypassesCacheForShortIDOnPathIndexedTypes(t *test
633639
})
634640
}
635641

636-
func TestTryCacheReadBypassesCacheForShortIDOnPathIndexedTypes(t *testing.T) {
642+
func TestUnitNsxt_tryCacheReadBypassesCacheForShortIDOnPathIndexedTypes(t *testing.T) {
637643
rSchema := map[string]*schema.Schema{
638644
"path": getPathSchema(),
639645
}

0 commit comments

Comments
 (0)