xtern commented on code in PR #5493:
URL: https://github.com/apache/ignite-3/pull/5493#discussion_r2016447334


##########
modules/table/src/integrationTest/java/org/apache/ignite/internal/table/ItKeyValueViewSimpleSchemaApiTest.java:
##########
@@ -60,28 +60,38 @@ public class ItKeyValueViewSimpleSchemaApiTest extends 
ItKeyValueViewApiBaseTest
     private static final String TABLE_NAME_NON_NULLABLE_VALUE = 
"test_non_nullable_value";
 
     @BeforeAll
-    public void createTable() {
-        createTable(TABLE_NAME_SIMPLE_TYPE, List.of(new Column("VAL", 
NativeTypes.INT64, true)));
-        createTable(TABLE_NAME_NON_NULLABLE_VALUE, List.of(new Column("VAL", 
NativeTypes.INT64, false)));
+    void createTable() {
+        List<TestTableDefinition> tables = new ArrayList<>();
+        Column[] nullableValue = {new Column("VAL", NativeTypes.INT64, true)};
+
+        tables.add(new TestTableDefinition(TABLE_NAME_SIMPLE_TYPE, 
DEFAULT_KEY, nullableValue, true));
+
+        tables.add(new TestTableDefinition(
+                TABLE_NAME_NON_NULLABLE_VALUE,
+                DEFAULT_KEY,
+                new Column[] {new Column("VAL", NativeTypes.INT64, false)},
+                true
+        ));
 
         for (NativeType type : SchemaTestUtils.ALL_TYPES) {
             String tableName = "T_" + type.spec().name();
+            Column[] values = {new Column("VAL", type, false)};
 
-            createTable(tableName, false,
-                    List.of(new Column("id", NativeTypes.INT64, false)),
-                    List.of(new Column("VAL", type, true)));
+            tables.add(new TestTableDefinition(tableName, DEFAULT_KEY, 
values));
         }
 
         // Validate all types are tested.
         Set<NativeTypeSpec> nativeTypes = EnumSet.allOf(NativeTypeSpec.class);
 
         assertEquals(nativeTypes,
                 
SchemaTestUtils.ALL_TYPES.stream().map(NativeType::spec).collect(Collectors.toSet()));
+
+        createTables(tables);
     }
 
     @ParameterizedTest
     @MethodSource("testCases")
-    public void put(TestCase<Long, Long> testCase) {
+    void put(TestCase<Long, Long> testCase) {

Review Comment:
   Thanks, reverted redundant changes, suppression added.



##########
modules/table/src/integrationTest/java/org/apache/ignite/internal/table/ItTableViewApiUnifiedBaseTest.java:
##########
@@ -82,17 +83,69 @@ protected int initialNodes() {
         return 1;
     }
 
-    void createTable(String name, List<Column> columns) {
-        createTable(name, true, DEF_PK, columns);
+    void createTables(Collection<TestTableDefinition> tables) {
+        IgniteStringBuilder buf = new IgniteStringBuilder();
+
+        for (TestTableDefinition def : tables) {
+            buf.app(generateSqlCreate(def));
+
+            if (def.clearAfterTest) {
+                tablesToClear.add(def.name);
+            }
+        }
+
+        CLUSTER.aliveNode().sql().executeScript(buf.toString());
+    }
+
+    static void assertEqualsValues(SchemaDescriptor schema, Tuple expected, 
@Nullable Tuple actual) {
+        assertNotNull(actual);
+
+        for (int i = 0; i < schema.valueColumns().size(); i++) {
+            Column col = schema.valueColumns().get(i);
+
+            String quotedName = IgniteNameUtils.quoteIfNeeded(col.name());
+
+            Object val1 = expected.value(quotedName);
+            Object val2 = actual.value(quotedName);
+
+            if (val1 instanceof byte[] && val2 instanceof byte[]) {
+                Assertions.assertArrayEquals((byte[]) val1, (byte[]) val2, 
"Equality check failed: colIdx=" + col.positionInRow());
+            } else {
+                assertEquals(val1, val2, "Equality check failed: colIdx=" + 
col.positionInRow());
+            }
+        }

Review Comment:
   Several methods are currently being used in these tests.
   ```
   assertEqualsKeys
   assertEqualsValues
   assertEqualsRows
   ```
   
   I assume you are talking about replacing `assertEqualsRows` with 
Tuple.equals(expected, actual).
   The main problem here is that Tuple.equals returns boolean - it doesn't tell 
you what exactly is wrong, where the tuples don't match.
   
   p.s. added "self-check" `assertTrue(Tuple.equals(expected, actual));` at the 
end of the `assertEqualsRows` method



-- 
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: notifications-unsubscr...@ignite.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to