Adding unit tests to maestro-utils module #2081
Conversation
|
Those unit test failures are not yours - there's another PR that's going to fix those :D |
|
Hey @Matheeusb -- thanks for raising this PR! I'll take a look at this throughout the week. |
|
Thank you @axelniklasson ! |
| val ip = SocketUtils.localIp() | ||
|
|
||
| assertNotNull(ip) | ||
| assertTrue(ip.startsWith("192") || ip.startsWith("10") || ip.startsWith("172")) |
There was a problem hiding this comment.
This assertion fails for me when testing this locally as the IP resolves to 127.0.0.1 instead
There was a problem hiding this comment.
Resolved here 08c9a16
| } | ||
|
|
||
| @Test | ||
| fun `nextFreePort should throw IllegalStateException if no ports are available`() { |
There was a problem hiding this comment.
Can we rewrite this test to first use nextFreePort to find two suitable ports to use as from and to instead of hardcoding it? That way it won't clash with other processes running in the environment.
There was a problem hiding this comment.
Resolved here 08c9a16
| assertTrue(inetAddress is Inet4Address) | ||
| assertFalse(inetAddress.isLoopbackAddress) |
There was a problem hiding this comment.
This fails if the returned address is a loopback address. What is the goal of this test? SocketUtils.localIp fallbacks to a loopback address so testing localIp should return a non-loopback IPv4 address doesn't really make sense to me so would love some context here?
There was a problem hiding this comment.
You are sure @axelniklasson! I reviewed this test and saw that it does not bring any changes to test coverage. When creating this test, my objective was to validate that the connection returned was not of the loopback type, but in fact this validation makes no sense. I refactored the tests related to the localIp method, making the objectives of each test clearer and even achieving greater test coverage. You can check commit 0378f46
|
Will merge this after #2090 is merged |
axelniklasson
force-pushed
the
feature/unit_test_maestro_utils
branch
from
cb272ed to
4304529
Compare
|
Thanks for the contribution @Matheeusb 🎉 |
* test: Adding unit tests to the MaestroTimerTest class * test: Adding unit tests to the SocketUtils class * test: Adding unit tests to the Strings class * test: Adding unit tests to the Collections class * test: Adding unit tests to the DepthTracker class * test: Adding unit tests to the Insight class * test: Adding unit tests to the Errors class * test: Refactoring SocketUtils unit tests * test: Resolving commands_optional_tournee test * test: Refactoring test about localhost address * test: Refactoring the tests about localIp method * chore: Adding mockk dependency
* test: Adding unit tests to the MaestroTimerTest class * test: Adding unit tests to the SocketUtils class * test: Adding unit tests to the Strings class * test: Adding unit tests to the Collections class * test: Adding unit tests to the DepthTracker class * test: Adding unit tests to the Insight class * test: Adding unit tests to the Errors class * test: Refactoring SocketUtils unit tests * test: Resolving commands_optional_tournee test * test: Refactoring test about localhost address * test: Refactoring the tests about localIp method * chore: Adding mockk dependency
Proposed changes
copilot:summary
Testing
Issues fixed