ibessonov commented on code in PR #1833:
URL: https://github.com/apache/ignite-3/pull/1833#discussion_r1145730152
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/matchers/CompletableFutureMatcher.java:
##########
@@ -53,15 +53,18 @@
* Class of throwable that should be the cause of fail if the future
should fail. If {@code null}, the future should be completed
* successfully.
*/
- private final Class<? extends Throwable> causeOfFail;
+ private final @Nullable Class<? extends Throwable> causeOfFail;
+
+ /** Fragment that must be a substring of a error message (if {@code null},
message won't be checked). */
Review Comment:
```suggestion
/** Fragment that must be a substring of an error message (if {@code
null}, message won't be checked). */
```
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/matchers/CompletableFutureMatcher.java:
##########
@@ -70,22 +73,23 @@ private CompletableFutureMatcher(Matcher<T> matcher) {
* @param matcher Matcher to forward the result of the completable future.
* @param timeout Timeout.
* @param timeoutTimeUnit {@link TimeUnit} for timeout.
- * @param causeOfFail If {@code null}, the future should be completed
successfully, otherwise it specifies the class of cause
- * throwable.
+ * @param causeOfFail If {@code null}, the future should be completed
successfully, otherwise it specifies class of cause throwable.
+ * @param errorMessageFragment Fragment that must be a substring of a
error message (if {@code null}, message won't be checked).
Review Comment:
```suggestion
* @param errorMessageFragment Fragment that must be a substring of an
error message (if {@code null}, message won't be checked).
```
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/matchers/CompletableFutureMatcher.java:
##########
@@ -70,22 +73,23 @@ private CompletableFutureMatcher(Matcher<T> matcher) {
* @param matcher Matcher to forward the result of the completable future.
* @param timeout Timeout.
* @param timeoutTimeUnit {@link TimeUnit} for timeout.
- * @param causeOfFail If {@code null}, the future should be completed
successfully, otherwise it specifies the class of cause
- * throwable.
+ * @param causeOfFail If {@code null}, the future should be completed
successfully, otherwise it specifies class of cause throwable.
Review Comment:
Why did you remove "the"?
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/matchers/CompletableFutureMatcher.java:
##########
@@ -53,15 +53,18 @@
* Class of throwable that should be the cause of fail if the future
should fail. If {@code null}, the future should be completed
* successfully.
*/
- private final Class<? extends Throwable> causeOfFail;
+ private final @Nullable Class<? extends Throwable> causeOfFail;
+
+ /** Fragment that must be a substring of a error message (if {@code null},
message won't be checked). */
+ private final @Nullable String errorMessageFragment;
Review Comment:
Since you added a new parameter, please add it into `describeMismatchSafely`
or other appropriate place
##########
modules/index/src/test/java/org/apache/ignite/internal/index/IndexManagerTest.java:
##########
@@ -92,100 +64,57 @@
*/
@ExtendWith(ConfigurationExtension.class)
public class IndexManagerTest {
- /** Configuration registry with one table for each test. */
- private static ConfigurationRegistry confRegistry;
-
- /** Tables configuration. */
- private static TablesConfiguration tablesConfig;
-
- /** Index manager. */
- private static IndexManager indexManager;
-
- /** Per test unique index name. */
- private static AtomicInteger index = new AtomicInteger();
-
- /**
- * Common components initialization.
- */
- @BeforeAll
- public static void setUp() throws Exception {
- confRegistry = new ConfigurationRegistry(
- List.of(TablesConfiguration.KEY),
- Set.of(IndexValidatorImpl.INSTANCE,
TableValidatorImpl.INSTANCE),
- new TestConfigurationStorage(DISTRIBUTED),
- List.of(ExtendedTableConfigurationSchema.class),
- List.of(
- HashIndexConfigurationSchema.class,
- SortedIndexConfigurationSchema.class,
-
- UnknownDataStorageConfigurationSchema.class,
- ConstantValueDefaultConfigurationSchema.class,
- FunctionCallDefaultConfigurationSchema.class,
- NullValueDefaultConfigurationSchema.class
- )
- );
+ @InjectConfiguration(
+ "mock.tables.tName {"
+ + "columns.c1 {type.type: STRING}, "
+ + "columns.c2 {type.type: STRING}, "
+ + "primaryKey {columns: [c1], colocationColumns: [c1]}"
+ + "}"
+ )
+ private TablesConfiguration tablesConfig;
- confRegistry.start();
+ private IndexManager indexManager;
- tablesConfig = confRegistry.getConfiguration(TablesConfiguration.KEY);
-
- TableManager tableManagerMock = Mockito.mock(TableManager.class);
- Map<UUID, TableImpl> tables = Mockito.mock(Map.class);
+ @BeforeEach
+ public void setUp() {
+ TableManager tableManagerMock = mock(TableManager.class);
when(tableManagerMock.tableAsync(anyLong(),
any(UUID.class))).thenAnswer(inv -> {
- InternalTable tbl = Mockito.mock(InternalTable.class);
+ InternalTable tbl = mock(InternalTable.class);
+
Mockito.doReturn(inv.getArgument(1)).when(tbl).tableId();
- return CompletableFuture.completedFuture(
- new TableImpl(tbl, new HeapLockManager(), () ->
CompletableFuture.completedFuture(List.of())));
+
+ return completedFuture(new TableImpl(tbl, new HeapLockManager(),
() -> completedFuture(List.of())));
});
SchemaManager schManager = mock(SchemaManager.class);
- when(schManager.schemaRegistry(anyLong(),
any())).thenReturn(CompletableFuture.completedFuture(null));
+
+ when(schManager.schemaRegistry(anyLong(),
any())).thenReturn(completedFuture(null));
indexManager = new IndexManager(tablesConfig, schManager,
tableManagerMock);
indexManager.start();
Review Comment:
Shouldn't you stop the manager in "tearDown"?
##########
modules/schema/src/test/java/org/apache/ignite/internal/schema/configuration/index/IndexValidatorImplTest.java:
##########
@@ -0,0 +1,141 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.schema.configuration.index;
+
+import static
org.apache.ignite.internal.configuration.validation.TestValidationUtil.validate;
+import static
org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.UUID;
+import java.util.function.Consumer;
+import org.apache.ignite.configuration.NamedListView;
+import org.apache.ignite.configuration.validation.ValidationContext;
+import
org.apache.ignite.internal.configuration.testframework.ConfigurationExtension;
+import
org.apache.ignite.internal.configuration.testframework.InjectConfiguration;
+import
org.apache.ignite.internal.schema.configuration.ExtendedTableConfiguration;
+import
org.apache.ignite.internal.schema.configuration.ExtendedTableConfigurationSchema;
+import org.apache.ignite.internal.schema.configuration.TablesConfiguration;
+import org.jetbrains.annotations.Nullable;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+/**
+ * For {@link IndexValidatorImpl} testing.
+ */
+@ExtendWith(ConfigurationExtension.class)
+public class IndexValidatorImplTest {
+ @InjectConfiguration(
+ value = "mock.tables.fooTable {columns.column0 {type.type:
STRING}}",
+ internalExtensions = ExtendedTableConfigurationSchema.class
Review Comment:
Is this necessary? I believe that all extensions are loaded implicitly
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/matchers/CompletableFutureMatcher.java:
##########
@@ -70,22 +73,23 @@ private CompletableFutureMatcher(Matcher<T> matcher) {
* @param matcher Matcher to forward the result of the completable future.
* @param timeout Timeout.
* @param timeoutTimeUnit {@link TimeUnit} for timeout.
- * @param causeOfFail If {@code null}, the future should be completed
successfully, otherwise it specifies the class of cause
- * throwable.
+ * @param causeOfFail If {@code null}, the future should be completed
successfully, otherwise it specifies class of cause throwable.
+ * @param errorMessageFragment Fragment that must be a substring of a
error message (if {@code null}, message won't be checked).
Review Comment:
You get the point
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
