Add unit test for pkg/apiserver/registry #4304
Conversation
Codecov Report
@@ Coverage Diff @@
## main #4304 +/- ##
==========================================
- Coverage 65.49% 57.98% -7.52%
==========================================
Files 364 388 +24
Lines 52925 55187 +2262
==========================================
- Hits 34665 31998 -2667
- Misses 15704 20764 +5060
+ Partials 2556 2425 -131
|
tnqn
force-pushed
the
unit-test-for-registry
branch
2 times, most recently
from
f112fea
to
b58938f
Compare
| watcher, err := r.Watch(context.TODO(), &internalversion.ListOptions{LabelSelector: tt.labelSelector}) | ||
| assert.NoError(t, err) | ||
| defer watcher.Stop() | ||
| for _, expectedObj := range tt.expectedEvents { |
There was a problem hiding this comment.
for the "label selector selecting nothing" test case, I don't think we are actually testing anything? at least it doesn't seem that we are checking that no events are received on the channel.
There was a problem hiding this comment.
Thanks for catching it. I have added a check for unexpected events.
| name: "one object", | ||
| object: stats, | ||
| expectedTable: &metav1.Table{ | ||
| ColumnDefinitions: []metav1.TableColumnDefinition{ |
There was a problem hiding this comment.
is it useful to redefine ColumnDefinitions here, or could we just use a shared global variable with the ConvertToTable function?
There was a problem hiding this comment.
extracted a variable as you suggested
tnqn
force-pushed
the
unit-test-for-registry
branch
2 times, most recently
from
d68b723
to
3e6d26b
Compare
tnqn
force-pushed
the
unit-test-for-registry
branch
from
3e6d26b
to
800cc2b
Compare
Signed-off-by: Quan Tian <qtian@vmware.com>
tnqn
force-pushed
the
unit-test-for-registry
branch
from
800cc2b
to
2313a71
Compare
| case <-time.NewTimer(time.Second).C: | ||
| t.Errorf("Failed to get expected object %v from watcher in time", expectedObj) |
There was a problem hiding this comment.
I think the timer won't be garbage collected until it fires, but that's probably not an issue for unit tests...
There was a problem hiding this comment.
You are right. Since this is not the first place not stopping the timer explicitly in unit tests, I will leave it as is for now.
|
/skip-all |
Signed-off-by: Quan Tian <qtian@vmware.com>
Signed-off-by: Quan Tian <qtian@vmware.com>
Signed-off-by: Quan Tian qtian@vmware.com
For #4142