-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Add unit tests for com.netflix.discovery.util.EurekaEntityFunctions #1214
Add unit tests for com.netflix.discovery.util.EurekaEntityFunctions #1214
Conversation
eureka-client/src/test/java/com/netflix/discovery/util/EurekaEntityFunctionsTests.java
Outdated
Show resolved
Hide resolved
Thanks for the PR! |
btw, if you're looking for test-related tasks, fixing the flakiness would be the top one probably. |
3923472
to
61be37e
Compare
Do you have any specific class in mind which you would like me to have a look at? :) |
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.
Most comments are around code duplication.
eureka-client/src/test/java/com/netflix/discovery/util/EurekaEntityFunctionsTests.java
Outdated
Show resolved
Hide resolved
eureka-client/src/test/java/com/netflix/discovery/util/EurekaEntityFunctionsTests.java
Outdated
Show resolved
Hide resolved
eureka-client/src/test/java/com/netflix/discovery/util/EurekaEntityFunctionsTests.java
Outdated
Show resolved
Hide resolved
eureka-client/src/test/java/com/netflix/discovery/util/EurekaEntityFunctionsTests.java
Outdated
Show resolved
Hide resolved
eureka-client/src/test/java/com/netflix/discovery/util/EurekaEntityFunctionsTests.java
Outdated
Show resolved
Hide resolved
eureka-client/src/test/java/com/netflix/discovery/util/EurekaEntityFunctionsTests.java
Outdated
Show resolved
Hide resolved
eureka-client/src/test/java/com/netflix/discovery/util/EurekaEntityFunctionsTests.java
Outdated
Show resolved
Hide resolved
eureka-client/src/test/java/com/netflix/discovery/util/EurekaEntityFunctionsTests.java
Outdated
Show resolved
Hide resolved
eureka-client/src/test/java/com/netflix/discovery/util/EurekaEntityFunctionsTests.java
Outdated
Show resolved
Hide resolved
eureka-client/src/test/java/com/netflix/discovery/util/EurekaEntityFunctionsTests.java
Outdated
Show resolved
Hide resolved
no not really, just run the tests multiple times and see if you get any intermittent failures, the ones failing would be flaky and something we'd appreciate to have fixed. |
These tests were written using Diffblue Cover.
61be37e
to
df91064
Compare
Hi @troshko111 just following up and wondering if you would like me to make any changes to the PR? |
Sorry did not realize this got updated, for the future can you please not rebase PR iterations? It makes the review very hard (every iteration has to be reviewed again as a completely new change without any intermediate diffs). |
eureka-client/src/test/java/com/netflix/discovery/util/EurekaEntityFunctionsTest.java
Outdated
Show resolved
Hide resolved
eureka-client/src/test/java/com/netflix/discovery/util/EurekaEntityFunctionsTest.java
Show resolved
Hide resolved
These tests were written using Diffblue Cover.
e9cb921
to
66a4ffd
Compare
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.
LGTM overall, a couple of nits. Thanks again!
eureka-client/src/test/java/com/netflix/discovery/util/EurekaEntityFunctionsTest.java
Show resolved
Hide resolved
eureka-client/src/test/java/com/netflix/discovery/util/EurekaEntityFunctionsTest.java
Outdated
Show resolved
Hide resolved
eureka-client/src/test/java/com/netflix/discovery/util/EurekaEntityFunctionsTest.java
Outdated
Show resolved
Hide resolved
eureka-client/src/test/java/com/netflix/discovery/util/EurekaEntityFunctionsTest.java
Show resolved
Hide resolved
These tests were written using Diffblue Cover.
I've analysed your codebase and noticed that
com.netflix.discovery.util.EurekaEntityFunctions
is not fully tested.I've written some tests for the methods in this class with the help of Diffblue Cover.
Hopefully, these tests will help you detect any regressions caused by future code changes. If you would find it useful to have additional tests written for this repository, I would be more than happy to look at other classes that you consider important.