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