-
Notifications
You must be signed in to change notification settings - Fork 217
Update unit tests and simplify logic for dynamic update #2237
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
Conversation
@@ -1520,6 +1590,13 @@ public void whenPodCreated_createPodWithOwnerReference() { | |||
assertThat(getCreatedPod().getMetadata().getOwnerReferences(), contains(expectedReference)); | |||
} | |||
|
|||
// TODO: add tests for dynamic updates |
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.
How hard would it be to create these tests? I'm not a fan of leaving a TODO in the code.
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.
actually the tests are there, removing TODO
There is regression in the branch, withdrawing now. |
…te the primordial domain.
I found the problem, actually it's a bug rather than a regression in mii v1 model diff code |
@jshum2479, what's the status of this change? Did you resolve the regression? |
@rjeberhard Actually, it's not a regression, I found a bug in v1 and fixed that. It should be good to go |
Russ and I worked on adding unit tests and simplify the makeright logic for dynamic update.