subject:"\[GitHub\] \[flink\] reswqa commented on a diff in pull request #22445\: \[FLINK\-31876\]\[QS\] Migrate flink\-queryable\-state\-client tests to JUnit5"

[GitHub] [flink] reswqa commented on a diff in pull request #22445: [FLINK-31876][QS] Migrate flink-queryable-state-client tests to JUnit5

2023-04-25 Thread via GitHub



reswqa commented on code in PR #22445:
URL: https://github.com/apache/flink/pull/22445#discussion_r1177297848


##
flink-queryable-state/flink-queryable-state-client-java/src/test/java/org/apache/flink/queryablestate/client/state/ImmutableListStateTest.java:
##
@@ -56,26 +57,23 @@ public void setUp() throws Exception {
 listState = ImmutableListState.createState(listStateDesc, serInit);
 }
 
-@Test(expected = UnsupportedOperationException.class)
-public void testUpdate() throws Exception {
+@Test
+void testUpdate() throws Exception {
 List list = getStateContents();
-assertEquals(1L, list.size());
-
-long element = list.get(0);
-assertEquals(42L, element);
-
-listState.add(54L);
+assertThat(list).hasSize(1);
+assertThat(list).containsExactly(42L);
+assertThatThrownBy(() -> listState.add(54L))
+.isInstanceOf(UnsupportedOperationException.class);
 }
 
-@Test(expected = UnsupportedOperationException.class)
-public void testClear() throws Exception {
+@Test
+void testClear() throws Exception {
 List list = getStateContents();
-assertEquals(1L, list.size());
-
-long element = list.get(0);
-assertEquals(42L, element);
+assertThat(list).hasSize(1);
 
-listState.clear();
+assertThat(list).containsExactly(42L);

Review Comment:
   assertThat(list).containsExactly(42L);



##
flink-queryable-state/flink-queryable-state-client-java/src/test/java/org/apache/flink/queryablestate/client/state/ImmutableMapStateTest.java:
##
@@ -63,130 +62,147 @@ public void setUp() throws Exception {
 mapState = ImmutableMapState.createState(mapStateDesc, initSer);
 }
 
-@Test(expected = UnsupportedOperationException.class)
-public void testPut() throws Exception {
-assertTrue(mapState.contains(1L));
+@Test
+void testPut() throws Exception {
+assertThat(mapState.contains(1L)).isTrue();
 long value = mapState.get(1L);
-assertEquals(5L, value);
+assertThat(value).isEqualTo(5L);
 
-assertTrue(mapState.contains(2L));
+assertThat(mapState.contains(2L)).isTrue();
 value = mapState.get(2L);
-assertEquals(5L, value);
-
-mapState.put(2L, 54L);
+assertThat(value).isEqualTo(5L);
+assertThatThrownBy(() -> mapState.put(2L, 54L))
+.isInstanceOf(UnsupportedOperationException.class);
 }
 
-@Test(expected = UnsupportedOperationException.class)
-public void testPutAll() throws Exception {
-assertTrue(mapState.contains(1L));
+@Test
+void testPutAll() throws Exception {
+assertThat(mapState.contains(1L)).isTrue();
 long value = mapState.get(1L);
-assertEquals(5L, value);
+assertThat(value).isEqualTo(5L);
 
-assertTrue(mapState.contains(2L));
+assertThat(mapState.contains(2L)).isTrue();
 value = mapState.get(2L);
-assertEquals(5L, value);
+assertThat(value).isEqualTo(5L);
 
 Map nMap = new HashMap<>();
 nMap.put(1L, 7L);
 nMap.put(2L, 7L);
-
-mapState.putAll(nMap);
+assertThatThrownBy(() -> mapState.putAll(nMap))
+.isInstanceOf(UnsupportedOperationException.class);
 }
 
-@Test(expected = UnsupportedOperationException.class)
-public void testUpdate() throws Exception {
-assertTrue(mapState.contains(1L));
+@Test
+void testUpdate() throws Exception {
+assertThat(mapState.contains(1L)).isTrue();
 long value = mapState.get(1L);
-assertEquals(5L, value);
+assertThat(value).isEqualTo(5L);
 
-assertTrue(mapState.contains(2L));
+assertThat(mapState.contains(2L)).isTrue();
 value = mapState.get(2L);
-assertEquals(5L, value);
-
-mapState.put(2L, 54L);
+assertThat(value).isEqualTo(5L);
+assertThatThrownBy(() -> mapState.put(2L, 54L))
+.isInstanceOf(UnsupportedOperationException.class);
 }
 
-@Test(expected = UnsupportedOperationException.class)
-public void testIterator() throws Exception {
-assertTrue(mapState.contains(1L));
+@Test
+void testIterator() throws Exception {
+assertThat(mapState.contains(1L)).isTrue();
 long value = mapState.get(1L);
-assertEquals(5L, value);
+assertThat(value).isEqualTo(5L);
 
-assertTrue(mapState.contains(2L));
+assertThat(mapState.contains(2L)).isTrue();
 value = mapState.get(2L);
-assertEquals(5L, value);
-
-Iterator> iterator = mapState.iterator();
-while (iterator.hasNext()) {
-iterator.remove();
-}
+assertThat(value).isEqualTo(5L);
+assertThatThrownBy(
+() -> {
+Iterator> iterator = 
mapState.iterator();
+

[GitHub] [flink] reswqa commented on a diff in pull request #22445: [FLINK-31876][QS] Migrate flink-queryable-state-client tests to JUnit5

2023-04-25 Thread via GitHub



reswqa commented on code in PR #22445:
URL: https://github.com/apache/flink/pull/22445#discussion_r1176281072


##
flink-queryable-state/flink-queryable-state-client-java/src/test/java/org/apache/flink/queryablestate/client/state/ImmutableAggregatingStateTest.java:
##
@@ -24,23 +24,24 @@
 import org.apache.flink.api.common.state.AggregatingStateDescriptor;
 import org.apache.flink.core.memory.DataOutputViewStreamWrapper;
 
-import org.junit.Before;
-import org.junit.Test;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
 
 import java.io.ByteArrayOutputStream;
 
-import static org.junit.Assert.assertEquals;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.junit.jupiter.api.Assertions.assertEquals;

Review Comment:
   Why is it still there, we shouldn't use any non-assertJ assertions anymore.



##
flink-queryable-state/flink-queryable-state-client-java/src/test/java/org/apache/flink/queryablestate/client/state/ImmutableMapStateTest.java:
##
@@ -24,28 +24,29 @@
 import org.apache.flink.api.common.typeinfo.BasicTypeInfo;
 import 
org.apache.flink.queryablestate.client.state.serialization.KvStateSerializer;
 
-import org.junit.Before;
-import org.junit.Test;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
 
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;

Review Comment:
   These should all be removed.



##
flink-queryable-state/flink-queryable-state-client-java/src/test/java/org/apache/flink/queryablestate/client/state/ImmutableListStateTest.java:
##
@@ -56,26 +58,23 @@ public void setUp() throws Exception {
 listState = ImmutableListState.createState(listStateDesc, serInit);
 }
 
-@Test(expected = UnsupportedOperationException.class)
-public void testUpdate() throws Exception {
+@Test
+void testUpdate() throws Exception {
 List list = getStateContents();
-assertEquals(1L, list.size());
-
-long element = list.get(0);
-assertEquals(42L, element);
-
-listState.add(54L);
+assertThat(list).hasSize(1);
+assertThat(list).contains(42L, Index.atIndex(0));

Review Comment:
   ```suggestion
   assertThat(list).containsExactly(42L);
   ```



-- 
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

[GitHub] [flink] reswqa commented on a diff in pull request #22445: [FLINK-31876][QS] Migrate flink-queryable-state-client tests to JUnit5

2023-04-24 Thread via GitHub



reswqa commented on code in PR #22445:
URL: https://github.com/apache/flink/pull/22445#discussion_r1176019508


##
flink-queryable-state/flink-queryable-state-client-java/src/test/java/org/apache/flink/queryablestate/client/state/ImmutableAggregatingStateTest.java:
##
@@ -53,20 +54,28 @@ public void setUp() throws Exception {
 aggrState = ImmutableAggregatingState.createState(aggrStateDesc, 
out.toByteArray());
 }
 
-@Test(expected = UnsupportedOperationException.class)
-public void testUpdate() throws Exception {
-String value = aggrState.get();
-assertEquals("42", value);
+@Test
+void testUpdate() {
+assertThatThrownBy(
+() -> {
+String value = aggrState.get();
+assertEquals("42", value);

Review Comment:
   All assertions should be migrated to AssertJ.



##
flink-queryable-state/flink-queryable-state-client-java/src/test/java/org/apache/flink/queryablestate/client/state/ImmutableAggregatingStateTest.java:
##
@@ -53,20 +54,28 @@ public void setUp() throws Exception {
 aggrState = ImmutableAggregatingState.createState(aggrStateDesc, 
out.toByteArray());
 }
 
-@Test(expected = UnsupportedOperationException.class)
-public void testUpdate() throws Exception {
-String value = aggrState.get();
-assertEquals("42", value);
+@Test
+void testUpdate() {
+assertThatThrownBy(
+() -> {
+String value = aggrState.get();
+assertEquals("42", value);
 
-aggrState.add(54L);
+aggrState.add(54L);

Review Comment:
   We should make the code block of `assertThatThrownBy` as small as possible, 
ideally it only contains codes that will throw errors.



-- 
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

[GitHub] [flink] reswqa commented on a diff in pull request #22445: [FLINK-31876][QS] Migrate flink-queryable-state-client tests to JUnit5

[GitHub] [flink] reswqa commented on a diff in pull request #22445: [FLINK-31876][QS] Migrate flink-queryable-state-client tests to JUnit5

[GitHub] [flink] reswqa commented on a diff in pull request #22445: [FLINK-31876][QS] Migrate flink-queryable-state-client tests to JUnit5

3 matches

Site Navigation

Mail list logo

Footer information