New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow test cases to interact with client builder #328
Conversation
Codecov ReportBase: 60.51% // Head: 60.36% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #328 +/- ##
==========================================
- Coverage 60.51% 60.36% -0.15%
==========================================
Files 17 17
Lines 2193 2210 +17
==========================================
+ Hits 1327 1334 +7
- Misses 789 798 +9
- Partials 77 78 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
obj := objs[i].DeepCopyObject().(client.Object) | ||
// default to a non-zero creation timestamp | ||
if obj.GetCreationTimestamp().Time.IsZero() { | ||
obj.SetCreationTimestamp(metav1.NewTime(time.UnixMilli(1000))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[thought]: We default a resource's version to 999
. In my understanding, that's an intentionally odd value so that it stands out. I am wondering if defaulting the creation timestamp to time.UnixMilli(999)
would serve a similar purpose. It would also feel consistent; once you see 999
it's like "Bart reconciler-runtime was here".
reconciler-runtime/testing/subreconciler.go
Line 211 in d526d74
resource.SetResourceVersion("999") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code isn't new, just moved. While I agree with the spirit, I'd rather not risk a subtile regression
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Totally makes sense to me!
Controller runtime now supports list queries using field indexes with the fake client. If a fake client makes a request using a field index that is not registered, it will return an error. The fake client was previously created with a scheme and an initial set of objects. It now can be created with a func that can interact with the client builder which is where a field index can be registered. The `WithClientBuilder` field is optional on each test case struct and ExpectConfig, allowing the fake client to be further configured. Signed-off-by: Scott Andrews <andrewssc@vmware.com>
cdfdec3
to
50c3b86
Compare
Controller runtime now supports list queries using field indexes with the fake client. If a fake client makes a request using a field index that is not registered, it will return an error.
The fake client was previously created with a scheme and an initial set of objects. It now can be created with a func that can interact with the client builder which is where a field index can be registered. The
WithClientBuilder
field is optional on each test case struct and ExpectConfig, allowing the fake client to be further configured.Signed-off-by: Scott Andrews andrewssc@vmware.com