You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2020/11/03 13:49:42 UTC
[GitHub] [ignite] alamar commented on a change in pull request #8367: Add travis check that tests are not in suites
alamar commented on a change in pull request #8367:
URL: https://github.com/apache/ignite/pull/8367#discussion_r516483599
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/cache/distributed/GridCacheMultithreadedFailoverAbstractTest.java
##########
@@ -69,6 +70,7 @@
/**
* Base test for all multithreaded cache scenarios w/ and w/o failover.
*/
+@Ignore("FAILED! Abstract")
Review comment:
Does it have any running subclasses? If so, it does not need to be included in suites itself. If it doesn't, let's make it not abstrace?
##########
File path: modules/core/src/test/java/org/apache/ignite/loadtests/communication/GridIoManagerBenchmark0.java
##########
@@ -44,13 +44,15 @@
import org.apache.ignite.spi.communication.tcp.TcpCommunicationSpi;
import org.apache.ignite.testframework.GridTestUtils;
import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Ignore;
import org.junit.Test;
import static org.apache.ignite.internal.managers.communication.GridIoPolicy.PUBLIC_POOL;
/**
*
*/
+@Ignore("Benchmark")
Review comment:
please file an additional ticket to deal with all benchmark/load tests, link to it here.
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/cache/distributed/IgniteBinaryMetadataUpdateNodeRestartTest.java
##########
@@ -47,6 +48,7 @@
/**
*
*/
+@Ignore("IGNITE-4768, IGNITE-9214")
Review comment:
Here and elsewhere: please choose a single most relevant ticket and put the complete JIRA URL in @Ignore message. TC/MTCGA wll recognize and link it.
##########
File path: modules/core/src/test/java/org/apache/ignite/testsuites/CoreZookeeperDiscoverySpiTestSuite3.java
##########
@@ -0,0 +1,51 @@
+/*
+ * 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.testsuites;
+
+import org.apache.ignite.internal.processors.cache.datastructures.IgniteClientDataStructuresTest;
+import org.apache.ignite.internal.processors.cache.datastructures.partitioned.GridCachePartitionedNodeRestartTxSelfTest;
+import org.apache.ignite.internal.processors.cache.datastructures.partitioned.GridCachePartitionedSequenceApiSelfTest;
+import org.apache.ignite.internal.processors.cache.datastructures.replicated.GridCacheReplicatedSequenceApiSelfTest;
+import org.apache.ignite.internal.processors.cache.distributed.replicated.GridCacheReplicatedNodeRestartSelfTest;
+import org.apache.ignite.internal.processors.cache.multijvm.GridCacheAtomicMultiJvmFullApiSelfTest;
+import org.apache.ignite.internal.processors.cache.multijvm.GridCachePartitionedMultiJvmFullApiSelfTest;
+import org.apache.ignite.internal.processors.cache.query.continuous.CacheContinuousQueryLongP2PTest;
+import org.apache.ignite.internal.processors.cache.query.continuous.CacheContinuousQueryOperationP2PTest;
+import org.apache.ignite.internal.processors.continuous.GridEventConsumeSelfTest;
+import org.apache.ignite.p2p.GridP2PContinuousDeploymentSelfTest;
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+/**
+ */
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+ GridCacheReplicatedNodeRestartSelfTest.class,
+ GridEventConsumeSelfTest.class,
+ GridCachePartitionedNodeRestartTxSelfTest.class,
+ IgniteClientDataStructuresTest.class,
+ GridCacheReplicatedSequenceApiSelfTest.class,
+ GridCachePartitionedSequenceApiSelfTest.class,
+ GridCacheAtomicMultiJvmFullApiSelfTest.class,
+ GridCachePartitionedMultiJvmFullApiSelfTest.class,
+ GridP2PContinuousDeploymentSelfTest.class,
+ CacheContinuousQueryOperationP2PTest.class,
+ CacheContinuousQueryLongP2PTest.class,
+})
+public class CoreZookeeperDiscoverySpiTestSuite3 {
Review comment:
Why do you call all suites Core*? We do not have such conventions, please rename them like the existing ones. Then we can update TC. Did you try to run them on TC, by the way?
##########
File path: modules/core/src/test/java/org/apache/ignite/testsuites/CoreIgniteSpringTestSuite.java
##########
@@ -0,0 +1,35 @@
+/*
+ * 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.testsuites;
+
+import org.apache.ignite.internal.processors.cache.distributed.dht.GridCacheDhtMultiBackupTest;
+import org.apache.ignite.startup.cmdline.GridCommandLineLoaderTest;
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+/**
+ * Spring tests.
+ */
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+ GridCommandLineLoaderTest.class,
+
+ GridCacheDhtMultiBackupTest.class
+})
+public class CoreIgniteSpringTestSuite {
Review comment:
Can we perhaps get rid of these tiny suites, move these tests to already existing suites?
##########
File path: modules/indexing/src/test/java/org/apache/ignite/testsuites/IgniteCacheQuerySelfTestSuite3.java
##########
@@ -48,34 +26,7 @@
*/
@RunWith(Suite.class)
@Suite.SuiteClasses({
- CacheContinuousQueryBufferLimitTest.class,
-
- GridCacheContinuousQueryNodesFilteringTest.class,
- GridCacheContinuousQueryPartitionTxOneNodeTest.class,
- CacheContinuousWithTransformerReplicatedSelfTest.class,
- CacheContinuousWithTransformerClientSelfTest.class,
- CacheContinuousQueryExecuteInPrimaryTest.class,
- IgniteCacheContinuousQueryNoUnsubscribeTest.class,
-
- ClientReconnectContinuousQueryTest.class,
- IgniteCacheContinuousQueryReconnectTest.class,
- IgniteCacheContinuousQueryClientTxReconnectTest.class,
- IgniteCacheContinuousQueryClientReconnectTest.class,
-
- GridCacheContinuousQueryAtomicSelfTest.class,
- GridCacheContinuousQueryAtomicNearEnabledSelfTest.class,
- GridCacheContinuousQueryPartitionedSelfTest.class,
- GridCacheContinuousQueryReplicatedSelfTest.class,
-
- CacheContinuousQueryFactoryAsyncFilterRandomOperationTest.class,
- CacheContinuousBatchForceServerModeAckTest.class,
- ContinuousQueryReassignmentTest.class,
- CacheContinuousQueryConcurrentPartitionUpdateTest.class,
-
- CacheContinuousQueryCounterPartitionedAtomicTest.class,
- CacheContinuousQueryCounterPartitionedTxTest.class,
- CacheContinuousQueryCounterReplicatedAtomicTest.class,
- CacheContinuousQueryCounterReplicatedTxTest.class
+ CoreIgniteCacheQuerySelfTestSuite3.class
Review comment:
Why? What's the point?
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/db/wal/WalCompactionAfterRestartTest.java
##########
@@ -40,11 +40,13 @@
import org.apache.ignite.lang.IgniteBiTuple;
import org.apache.ignite.testframework.GridTestUtils;
import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Ignore;
import org.junit.Test;
import static org.apache.ignite.events.EventType.EVT_WAL_SEGMENT_COMPACTED;
/** */
+@Ignore("FAILED!")
Review comment:
Please file a separate ticket to deal with all failed tests, link it here
##########
File path: modules/tools/src/main/java/org/apache/ignite/tools/surefire/testsuites/CheckAllTestsInSuites.java
##########
@@ -0,0 +1,112 @@
+/*
+ * 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.tools.surefire.testsuites;
+
+import java.lang.reflect.Modifier;
+import java.util.HashSet;
+import java.util.Set;
+import org.junit.Ignore;
+import org.junit.Test;
+import org.junit.runner.Description;
+import org.junit.runner.Request;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Suite;
+
+/** Checks that all test classes are part of any suite. */
+public class CheckAllTestsInSuites {
+ /** List of test classes. Input of the test. */
+ static Iterable<Class<?>> testClasses;
+
+ /** */
+ @Test
+ public void check() {
+ Set<Class<?>> suitedTestClasses = new HashSet<>();
+ Set<Class<?>> allTestClasses = new HashSet<>();
+ Set<Class<?>> suites = new HashSet<>();
+
+ // Workaround to handle cases when a class has descenders and it's OK to skip the base class.
+ // Also it works for DynamicSuite that can use a base class to create new test classes with reflection.
+ Set<Class<?>> superClasses = new HashSet<>();
+
+ for (Class<?> clazz: testClasses) {
Review comment:
white space is needed before :
##########
File path: modules/tools/src/main/java/org/apache/ignite/tools/surefire/testsuites/CheckAllTestsInSuites.java
##########
@@ -0,0 +1,112 @@
+/*
+ * 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.tools.surefire.testsuites;
+
+import java.lang.reflect.Modifier;
+import java.util.HashSet;
+import java.util.Set;
+import org.junit.Ignore;
+import org.junit.Test;
+import org.junit.runner.Description;
+import org.junit.runner.Request;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Suite;
+
+/** Checks that all test classes are part of any suite. */
+public class CheckAllTestsInSuites {
+ /** List of test classes. Input of the test. */
+ static Iterable<Class<?>> testClasses;
+
+ /** */
+ @Test
+ public void check() {
+ Set<Class<?>> suitedTestClasses = new HashSet<>();
+ Set<Class<?>> allTestClasses = new HashSet<>();
+ Set<Class<?>> suites = new HashSet<>();
+
+ // Workaround to handle cases when a class has descenders and it's OK to skip the base class.
+ // Also it works for DynamicSuite that can use a base class to create new test classes with reflection.
+ Set<Class<?>> superClasses = new HashSet<>();
+
+ for (Class<?> clazz: testClasses) {
+ if (Modifier.isAbstract(clazz.getModifiers()))
+ continue;
+
+ if (clazz.getAnnotation(Ignore.class) != null)
+ continue;
+
+ Description desc = Request.aClass(clazz).getRunner().getDescription();
+
+ if (isTestClass(desc)) {
+ allTestClasses.add(clazz);
+ superClasses.add(clazz.getSuperclass());
+ }
+ else
+ processSuite(desc, suitedTestClasses, suites, superClasses);
+ }
+
+ allTestClasses.removeAll(suitedTestClasses);
+ allTestClasses.removeAll(superClasses);
+
+ if (!allTestClasses.isEmpty()) {
+ StringBuilder builder = new StringBuilder("All test classes must be include in any test suite.")
+ .append("\nList of non-suited classes (")
+ .append(allTestClasses.size())
+ .append(" items):\n");
+
+ for (Class<?> c: allTestClasses)
+ builder.append("\t").append(c.getName()).append("\n");
+
+ throw new AssertionError(builder.toString());
+ }
+ }
+
+ /**
+ * Recursively hadnle suites - mark all test classes as suited.
+ */
+ private void processSuite(Description suite, Set<Class<?>> suitedClasses,
+ Set<Class<?>> suites, Set<Class<?>> superClasses) {
+ suites.add(suite.getTestClass());
+
+ for (Description desc: suite.getChildren()) {
+ if (!isTestClass(desc))
+ processSuite(desc, suitedClasses, suites, superClasses);
+ else {
+ suitedClasses.add(desc.getTestClass());
+ superClasses.add(desc.getTestClass().getSuperclass());
+ }
+ }
+ }
+
+ /**
+ * Check whether class is a test class or a suite.
+ *
+ * Suite classes are marked with RunWith annotation and value of it is a descender of Suite.class.
+ * For scala tests suite must be inherited from {@link org.scalatest.Suites} class.
+ * Exclusion of the rule is Parameterized.class, so classes are marked with it are test classes.
+ */
+ private boolean isTestClass(Description desc) {
+ RunWith runWith = desc.getAnnotation(RunWith.class);
+
+ return runWith == null
+ || runWith.value().equals(Parameterized.class)
Review comment:
Please use just 4 whitespaces as indentation level, as per coding guidelines.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org