Add unit tests for the api/versioned generated clientset - #2698
Draft
abrarshivani wants to merge 7 commits into
Draft
Add unit tests for the api/versioned generated clientset#2698abrarshivani wants to merge 7 commits into
abrarshivani wants to merge 7 commits into
Conversation
abrarshivani
force-pushed
the
unit-test-api-versioned
branch
2 times, most recently
from
July 31, 2026 03:56
29508ae to
c3802d0
Compare
Cover the generated clientset scheme registration: all six nvidia kinds
(ClusterPolicy, NVIDIADriver, GPUCluster and their List types) resolve in
both directions via ObjectKinds and Scheme.New, with negative cases for
unknown groups and cross-version kinds. Also covers the metav1 types added
for GroupVersion{Version:"v1"}, Status being registered as unversioned,
Codecs JSON round-trips (asserting encoding does not mutate the input),
missing/unknown kind decode errors, ParameterCodec encode/decode of
ListOptions including the *int64 timeout, and AddToScheme composing into a
fresh scheme idempotently.
The codec round-trip matrix covers every registered kind, GPUCluster and
GPUClusterList included, so it matches its stated purpose. AddToScheme is
asserted through its public result rather than by indexing the generated
localSchemeBuilder: the number of registration functions client-gen emits and
the order it emits them in are not a contract, so adding a group or reordering
the slice should only fail this test if the resulting scheme is wrong.
Signed-off-by: Abrar Shivani <ashivani@nvidia.com>
Cover the generated NvidiaV1 typed client and the ClusterPolicy resource client. setConfigDefaults is checked for GroupVersion, APIPath, a non-nil NegotiatedSerializer and UserAgent defaulting, along with the constructor error paths, NewForConfigOrDie panicking, New wiring the supplied rest.Interface, and the nil-receiver branch of RESTClient. The ClusterPolicy tests drive a real httptest server through NewForConfig and assert both the outgoing request and the decoded result for Get, List, Create, Update, UpdateStatus, Delete, DeleteCollection, Patch and Watch: paths, ParameterCodec-encoded query params, per-PatchType Content-Type and verbatim body, the subresources variadic producing /status, and error mapping for 404 and 500 Status bodies. Every path assertion also confirms the resource is cluster-scoped by rejecting a /namespaces/ segment. No assertion runs on the httptest server's goroutine. testify's require calls t.FailNow, which the testing package only permits on the goroutine running the test; a failure there could kill the handler goroutine without stopping the test. writeJSON therefore takes no *testing.T and surfaces a marshal failure as a 500, responses are built on the test goroutine, and request bodies are asserted afterwards from the recorder. Error paths assert stable typed errors such as *os.PathError and fs.ErrNotExist rather than message text owned by client-go, the standard library or the OS, which can change without any behaviour change here. Waits are bounded by a single 2s eventTimeout through a shared receiveEvent helper, and request contexts derive from t.Context(). Signed-off-by: Abrar Shivani <ashivani@nvidia.com>
Cover the generated fake NvidiaV1 client and fake ClusterPolicy client using a bare testing.Fake wired to an ObjectTracker and ObjectReaction, so the tests exercise the generated wiring without depending on the top-level fake clientset. Includes the full CRUD/List/Watch/Patch surface, IsAlreadyExists and IsNotFound boundaries, label-selector filtering, recorded action verbs, subresources, GVR/GVK and cluster-scoped empty namespace, and reactor-chain behaviour for both short-circuit errors and object substitution. Assertions are limited to the generated client's own contract rather than the behaviour of the current client-go fake. DeleteCollection is asserted through the recorded DeleteCollectionActionImpl (verb, GroupVersionResource, empty namespace, and the DeleteOptions and ListOptions carried on it) instead of asserting that nothing is deleted, which would pin a testing.ObjectReaction implementation detail and break a future Kubernetes upgrade that implemented collection deletion. That limitation is recorded in a comment, and a reactor-backed test covers real deletion. RESTClient is asserted to return a typed-nil *rest.RESTClient, since that is generated code in this repository; how client-go reacts to such a client is left unasserted. The copyListMeta hook is reachable only through a filtered List, because gentype returns the tracker's list verbatim when no label selector is set, so it is exercised that way. Waits are bounded by a 2s eventTimeout through a shared receiveEvent helper and contexts derive from t.Context(). Where a behaviour belongs to client-go rather than to the generated client, only the observable contract is asserted. The watch-without-a-reactor case asserts an error and a nil watcher without matching client-go's wording, which can change on a dependency bump without the behaviour changing. Signed-off-by: Abrar Shivani <ashivani@nvidia.com>
Cover the generated NvidiaV1alpha1 typed client and both of its resource clients, NVIDIADriver and GPUCluster. setConfigDefaults is checked for GroupVersion, APIPath, a non-nil NegotiatedSerializer and UserAgent defaulting, plus the constructor error paths, NewForConfigOrDie panicking, New wiring the supplied rest.Interface, the nil-receiver branch of RESTClient, and both resource getters sharing the group REST client. Both resources are driven against a real httptest server through NewForConfig, asserting the outgoing request and decoded result for Get, List, Create, Update, UpdateStatus, Delete, DeleteCollection, Patch and Watch: paths derived from SchemeGroupVersion, ParameterCodec-encoded query params, per-PatchType Content-Type and verbatim body, the subresources variadic producing /status, and 404 mapping to IsNotFound. Every path is asserted to be cluster-scoped with no /namespaces/ segment. The verb plumbing is shared rather than written out per resource: both resources go through the same generated generic client, so a single generic resourceMatrix in helpers_test.go drives them, parameterized on resource name, selectors, object and list constructors and one function per verb, emitting subtests named <resource>/<verb> so failures identify the resource. Schema-specific assertions stay in focused per-resource tests: literal request paths, GVK, decoded spec fields, the status subresource payload and typed watch decoding. The matrix is not lifted into its own package, because api/versioned is a client-gen output tree. No assertion runs on the httptest server's goroutine: testify's require calls t.FailNow, which only the test goroutine may call, so writeJSONResponse takes no *testing.T and watch frames are marshalled ahead of time. Error paths assert typed errors rather than message text owned by client-go, the standard library or the OS. Waits are bounded by a 2s eventTimeout and contexts derive from t.Context(). Signed-off-by: Abrar Shivani <ashivani@nvidia.com>
Cover the generated fake NvidiaV1alpha1 client and the fake NVIDIADriver and GPUCluster clients using a bare testing.Fake wired to an ObjectTracker and ObjectReaction, avoiding a dependency on the top-level fake clientset. Includes the full CRUD/List/Watch/Patch surface for both resources, IsAlreadyExists and IsNotFound boundaries, label-selector filtering, the copyListMeta hook, recorded action verbs and subresources, GVR/GVK derived from SchemeGroupVersion, cluster-scoped empty namespaces, and reactor-chain short-circuiting across the typed methods. Because both resources share one testing.Fake and one reaction chain, the group-level tests also assert that actions from both interleave into a single recorder, that an NVIDIADriver and a GPUCluster with the same name are tracked independently, and that watches and reactors are scoped per GVR so driver events do not leak into a GPUCluster watch. Assertions are limited to the generated client's own contract rather than the behaviour of the current client-go fake. DeleteCollection and UpdateStatus are asserted through the recorded action (verb, GroupVersionResource, empty namespace, status subresource, and the DeleteOptions and ListOptions carried on it) instead of asserting that nothing is deleted or that the whole object is replaced, which would pin testing.ObjectReaction implementation details and break a future Kubernetes upgrade that fixed them. Those limitations are recorded in comments, and reactor-backed tests cover real collection deletion. The watch tests assert the ListOptions the generated client forwards to tracker.Watch, including the Watch field it sets itself; replay of pre-existing objects follows from that forwarding and is kept as a documented corollary. RESTClient is asserted to return a typed-nil *rest.RESTClient, which is generated code here, while how client-go reacts to such a client is left unasserted. Waits are bounded by a 2s watchTimeout and contexts derive from t.Context(). The watch tests assert only the ListOptions the generated client forwards onto the recorded action. Nothing is asserted about replayed events: whether the tracker replays already-known objects, and whether it applies the label selector when it does, belongs to client-go rather than to the generated client. Event delivery for create, update and delete is covered separately. Signed-off-by: Abrar Shivani <ashivani@nvidia.com>
Cover the generated top-level Clientset constructors. NewForConfig is tested for success, for the rest.HTTPClientFor error path, for UserAgent defaulting observed on a real request header via an httptest server, and for leaving the caller's rest.Config unmutated given the shallow-copy semantics. NewForConfigAndClient covers the burst validation error when QPS is set without a positive burst, a rate limiter being installed and shared by both group clients, a caller-supplied RateLimiter being passed through by identity, and the QPS=0 case where each group client falls back to its own default limiter. Also covers NewForConfigOrDie panicking, New wiring the supplied RESTClient into every group client and discovery, the group accessors, and Discovery returning a genuine nil for a nil receiver so a typed-nil regression would be caught. State shared with the httptest handler goroutine is mutex guarded. The recorded request paths and the observed User-Agent are written by the server's goroutine and read by the test's; the calls happen to be sequential today, so -race passes, but the accesses are unsynchronized and would become a real race if a test ever issued concurrent calls. Request contexts derive from t.Context(). Signed-off-by: Abrar Shivani <ashivani@nvidia.com>
Cover NewSimpleClientset and the generated fake clientset wiring: seeding the object tracker, CRUD round trips through both group clients, the default watch reactor delivering Added/Modified/Deleted events, action recording and ClearActions, reactor interception and fall-through, panicking on an object type absent from the fake scheme, Tracker being shared with the typed clients, Discovery returning a FakeDiscovery bound to the same Fake, IsWatchListSemanticsUnSupported, and the interface assertions from the generated var block. Also covers register.go: the package scheme recognising all nvidia kinds plus the metav1 types, AddToScheme composing into a fresh scheme, and codecs decoding both group versions. Assertions are limited to the generated client's own contract rather than the behaviour of the current client-go fake. DeleteCollection is asserted through the recorded action (verb, GroupVersionResource, empty namespace, and the DeleteOptions and ListOptions carried on it) instead of asserting that nothing is deleted, which would pin a testing.ObjectReaction implementation detail and break a future Kubernetes upgrade that implemented it. That limitation is recorded in a comment, and a reactor-backed test covers real collection deletion. The watch test asserts the ListOptions the generated client forwards to tracker.Watch, including the Watch field it sets itself; replay of pre-existing objects follows from that forwarding and is kept as a documented corollary. The unregistered-kind panic uses a private, never-registered test type rather than corev1.Pod, which would silently stop testing anything if core types were ever added to this scheme, and asserts the recovered value satisfies runtime.IsNotRegisteredError so it cannot pass for the wrong reason. Waits are bounded by a 2s eventTimeout and contexts derive from t.Context(). Where a behaviour belongs to client-go rather than to the generated client, only the observable contract is asserted. The watch test checks the ListOptions the generated client forwards onto the recorded action and asserts nothing about replayed events, since whether the tracker replays already-known objects, and whether it applies the label selector when it does, is client-go's business. The watch-error case asserts an error and a nil watcher without matching client-go's wording, which can change on a dependency bump. Signed-off-by: Abrar Shivani <ashivani@nvidia.com>
abrarshivani
force-pushed
the
unit-test-api-versioned
branch
from
July 31, 2026 04:05
c3802d0 to
abd086f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds unit test coverage for the generated
api/versionedclientset, which previously had none.Each commit covers exactly one sub-package so they can be reviewed independently.
api/versioned/schemeapi/versioned/typed/nvidia/v1api/versioned/typed/nvidia/v1/fakeapi/versioned/typed/nvidia/v1alpha1api/versioned/typed/nvidia/v1alpha1/fakeapi/versioned(clientset)api/versioned/fakeOnly
_test.gofiles are added. No generated code,go.mod,go.sumor vendored file is modified.Approach
httptestserver viaNewForConfig, asserting both the outgoing request (path, query params produced by theParameterCodec,Content-Type, body round-trip) and the decoded result across Get/List/Create/Update/UpdateStatus/Delete/DeleteCollection/Patch/Watch.k8s.io/client-go/rest/fakeis not vendored, so a real test server is used rather than re-vendoring it.testing.Fakewired to anObjectTrackerandObjectReaction, so the generated wiring is tested without depending on the top-level fake clientset, which would be an import cycle for the typed fakes./namespaces/segment, enforcing the empty-namespace argument passed by the generated constructors.-raceand with repeated-countruns.Generated-code behaviours pinned by these tests
A few behaviours are easy to assume incorrectly, so the tests assert what the code actually does rather than what it appears to do:
DeleteCollectionis a no-op on the fakes.client-go'stesting.ObjectReactionhas noDeleteCollectionActionImplcase, so the action is recorded, the reactor chain is exhausted, and the call returnsnilhaving deleted nothing. Tests relying on it for cleanup are silently not cleaning up. A reactor-backed variant is included to show the supported path.copyListMetahook only runs when a label selector is set —gentypereturns the tracker's list verbatim whenlabel == nil, so the hook is reachable only via a filteredList.RESTClient()on the fake group clients returns a typed-nil*rest.RESTClient, so a!= nilguard passes and the following call panics.UpdateStatusreplaces the whole object in the fake: the action records subresourcestatus, butObjectReactionignores the subresource, so there is no spec/status separation.Notes
api/versionedare the trailingreturn nil, errbranches inNewForConfigAndClient. All three sub-constructors fail on identical input so the first always short-circuits; reaching the others would require monkey-patching generated functions. The equivalent first branch is covered.Apply/ApplyStatusare untested because no apply configurations are generated for these groups, so those methods do not exist on these interfaces.Spec.Operator.RuntimeClassrather than the deprecatedDefaultRuntimefield, to keepstaticcheckclean.Verification
go test ./api/versioned/... -count=1 -cover -race— 7/7 passgolangci-lint run ./api/versioned/...— 0 issuesgo vet ./api/versioned/...,gofmt -l,go build ./api/...— clean