zhangyy91 commented on code in PR #23265: URL: https://github.com/apache/flink/pull/23265#discussion_r1357750511
########## flink-runtime/src/test/java/org/apache/flink/runtime/taskmanager/TaskManagerLocationTest.java: ########## @@ -38,57 +38,61 @@ class TaskManagerLocationTest { @Test void testEqualsHashAndCompareTo() { - try { - ResourceID resourceID1 = new ResourceID("a"); - ResourceID resourceID2 = new ResourceID("b"); - ResourceID resourceID3 = new ResourceID("c"); - - // we mock the addresses to save the times of the reverse name lookups - InetAddress address1 = mock(InetAddress.class); - when(address1.getCanonicalHostName()).thenReturn("localhost"); - when(address1.getHostName()).thenReturn("localhost"); - when(address1.getHostAddress()).thenReturn("127.0.0.1"); - when(address1.getAddress()).thenReturn(new byte[] {127, 0, 0, 1}); - - InetAddress address2 = mock(InetAddress.class); - when(address2.getCanonicalHostName()).thenReturn("testhost1"); - when(address2.getHostName()).thenReturn("testhost1"); - when(address2.getHostAddress()).thenReturn("0.0.0.0"); - when(address2.getAddress()).thenReturn(new byte[] {0, 0, 0, 0}); - - InetAddress address3 = mock(InetAddress.class); - when(address3.getCanonicalHostName()).thenReturn("testhost2"); - when(address3.getHostName()).thenReturn("testhost2"); - when(address3.getHostAddress()).thenReturn("192.168.0.1"); - when(address3.getAddress()).thenReturn(new byte[] {(byte) 192, (byte) 168, 0, 1}); - - // one == four != two != three - TaskManagerLocation one = new TaskManagerLocation(resourceID1, address1, 19871); - TaskManagerLocation two = new TaskManagerLocation(resourceID2, address2, 19871); - TaskManagerLocation three = new TaskManagerLocation(resourceID3, address3, 10871); - TaskManagerLocation four = new TaskManagerLocation(resourceID1, address1, 19871); - - assertThat(one).isEqualTo(four); - assertThat(one).isNotEqualTo(two); - assertThat(one).isNotEqualTo(three); - assertThat(two).isNotEqualTo(three); - assertThat(three).isNotEqualTo(four); - - assertThat(one.compareTo(four)).isEqualTo(0); - assertThat(four.compareTo(one)).isEqualTo(0); - assertThat(one.compareTo(two)).isNotEqualTo(0); - assertThat(one.compareTo(three)).isNotEqualTo(0); - assertThat(two.compareTo(three)).isNotEqualTo(0); - assertThat(three.compareTo(four)).isNotEqualTo(0); - - { - int val = one.compareTo(two); - assertThat(two.compareTo(one)).isEqualTo(-val); - } - } catch (Exception e) { - e.printStackTrace(); - fail(e.getMessage()); - } + assertThatCode( + () -> { + ResourceID resourceID1 = new ResourceID("a"); + ResourceID resourceID2 = new ResourceID("b"); + ResourceID resourceID3 = new ResourceID("c"); + + // we mock the addresses to save the times of the reverse name lookups + InetAddress address1 = mock(InetAddress.class); + when(address1.getCanonicalHostName()).thenReturn("localhost"); + when(address1.getHostName()).thenReturn("localhost"); + when(address1.getHostAddress()).thenReturn("127.0.0.1"); + when(address1.getAddress()).thenReturn(new byte[] {127, 0, 0, 1}); + + InetAddress address2 = mock(InetAddress.class); + when(address2.getCanonicalHostName()).thenReturn("testhost1"); + when(address2.getHostName()).thenReturn("testhost1"); + when(address2.getHostAddress()).thenReturn("0.0.0.0"); + when(address2.getAddress()).thenReturn(new byte[] {0, 0, 0, 0}); + + InetAddress address3 = mock(InetAddress.class); + when(address3.getCanonicalHostName()).thenReturn("testhost2"); + when(address3.getHostName()).thenReturn("testhost2"); + when(address3.getHostAddress()).thenReturn("192.168.0.1"); + when(address3.getAddress()) + .thenReturn(new byte[] {(byte) 192, (byte) 168, 0, 1}); + + // one == four != two != three + TaskManagerLocation one = + new TaskManagerLocation(resourceID1, address1, 19871); + TaskManagerLocation two = + new TaskManagerLocation(resourceID2, address2, 19871); + TaskManagerLocation three = + new TaskManagerLocation(resourceID3, address3, 10871); + TaskManagerLocation four = + new TaskManagerLocation(resourceID1, address1, 19871); + + assertThat(one).isEqualTo(four); + assertThat(one).isNotEqualTo(two); + assertThat(one).isNotEqualTo(three); + assertThat(two).isNotEqualTo(three); + assertThat(three).isNotEqualTo(four); + + assertThat(one.compareTo(four)).isZero(); + assertThat(four.compareTo(one)).isZero(); + assertThat(one.compareTo(two)).isNotZero(); + assertThat(one.compareTo(three)).isNotZero(); + assertThat(two.compareTo(three)).isNotZero(); + assertThat(three.compareTo(four)).isNotZero(); + + { + int val = one.compareTo(two); + assertThat(two.compareTo(one)).isEqualTo(-val); + } + }) + .doesNotThrowAnyException(); Review Comment: @1996fanrui Thanks for your review. I think you are right. `e.printStackTrace()` and `fail(e.getMessage())` should be kept for their useful information. I will correct this. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org