Skip to content

Commit

Permalink
Mark more tests as flaky
Browse files Browse the repository at this point in the history
This adds the necessary annotation to repeat flaky tests if necessary
for tests described in these issues:
* JanusGraph#1457
* JanusGraph#1459
* JanusGraph#1462
* JanusGraph#1464
* JanusGraph#1465
* JanusGraph#1497
* JanusGraph#1498
* JanusGraph#2272
* JanusGraph#3142
* JanusGraph#3356
* JanusGraph#3392
* JanusGraph#3393
* JanusGraph#3651
* JanusGraph#3931
* JanusGraph#3959
* JanusGraph#3960

I went through the recent builds on `master` and checked all failed
builds. I think this should handle all flaky tests that occurred in
these builds, apart from the TinkerPop test where I just don't know how
to deal with it: JanusGraph#3841, but we can of course just address that in
another PR.
We however also sometimes get failed jobs because of some intermediate
issues where a backend cannot be properly set up or teared down for some
reason or downloading some artifact just fails. I don't know what we can
do to improve this, but I would also consider that out of scope of this
PR where I just wanted to deal with failing jobs due to specific tests
being flaky.

Hopefully, this will already lead to significantly fewer cases where we
have to manually restart failed jobs (which I had to do a lot lately for
all these Dependabot PRs).

Signed-off-by: Florian Hockmann <[email protected]>
  • Loading branch information
FlorianHockmann committed Sep 7, 2023
1 parent e2b0bed commit 80ca920
Show file tree
Hide file tree
Showing 10 changed files with 124 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2188,7 +2188,7 @@ public void testIndexReplay() throws Exception {
assertEquals(4, recoveryStats[1]); //all 4 index transaction had provoked errors in the indexing backend
}

@Test
@RepeatedIfExceptionsTest(repeats = 4, minSuccess = 2)
public void testIndexUpdatesWithoutReindex() throws InterruptedException, ExecutionException {
final Object[] settings = new Object[]{option(LOG_SEND_DELAY, MANAGEMENT_LOG), Duration.ofMillis(0),
option(KCVSLog.LOG_READ_LAG_TIME, MANAGEMENT_LOG), Duration.ofMillis(50),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.common.collect.Iterators;
import io.github.artsok.RepeatedIfExceptionsTest;
import org.apache.tinkerpop.gremlin.structure.Direction;
import org.apache.tinkerpop.gremlin.structure.Edge;
import org.apache.tinkerpop.gremlin.structure.Vertex;
Expand Down Expand Up @@ -127,7 +128,7 @@ private void verifyLockingOverwrite(long num) {
}
}

@Test
@RepeatedIfExceptionsTest(repeats = 4, minSuccess = 2)
public void testIdCounts() {
makeVertexIndexedUniqueKey("uid", Integer.class);
mgmt.setConsistency(mgmt.getGraphIndex("uid"), ConsistencyModifier.LOCK);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

package org.janusgraph.graphdb.berkeleyje;

import io.github.artsok.RepeatedIfExceptionsTest;
import org.janusgraph.BerkeleyStorageSetup;
import org.janusgraph.diskstorage.configuration.WriteConfiguration;
import org.janusgraph.graphdb.JanusGraphOperationCountingTest;
Expand All @@ -27,12 +26,6 @@ public WriteConfiguration getBaseConfiguration() {
return BerkeleyStorageSetup.getBerkeleyJEGraphConfiguration();
}

@Override
@RepeatedIfExceptionsTest(repeats = 4, minSuccess = 2)
public void testIdCounts() {
super.testIdCounts();
}

@AfterEach
public void resetCounts() {
resetMetrics(); // Metrics is a singleton, so subsequents test runs have wrong counts if we don't clean up.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public void createConfigurationShouldSupportMultiHosts() throws Exception {
}
}

@Test
@RepeatedIfExceptionsTest(repeats = 4, minSuccess = 2)
public void dropGraphShouldRemoveGraphKeyspace() throws Exception {
final MapConfiguration graphConfig = getGraphConfig();
final String graphName = graphConfig.getString(GRAPH_NAME.toStringWithoutRoot());
Expand Down Expand Up @@ -137,5 +137,11 @@ public void dropGraphShouldRemoveGraphKeyspace() throws Exception {
public void updateConfigurationShouldRemoveGraphFromCache() throws Exception {
super.updateConfigurationShouldRemoveGraphFromCache();
}

@RepeatedIfExceptionsTest(repeats = 4, minSuccess = 2)
@Override
public void dropShouldCleanUpTraversalSourceAndBindings() throws Exception {
super.dropShouldCleanUpTraversalSourceAndBindings();
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,18 @@

package org.janusgraph.graphdb.cql;

import io.github.artsok.ParameterizedRepeatedIfExceptionsTest;
import io.github.artsok.RepeatedIfExceptionsTest;
import org.janusgraph.JanusGraphCassandraContainer;
import org.janusgraph.StorageSetup;
import org.janusgraph.diskstorage.configuration.WriteConfiguration;
import org.janusgraph.graphdb.JanusGraphTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.testcontainers.junit.jupiter.Container;
import org.testcontainers.junit.jupiter.Testcontainers;

import java.util.concurrent.ExecutionException;

@Testcontainers
public class CQLGraphCacheTest extends JanusGraphTest {

Expand All @@ -31,4 +36,54 @@ public class CQLGraphCacheTest extends JanusGraphTest {
public WriteConfiguration getConfiguration() {
return StorageSetup.addPermanentCache(cqlContainer.getConfiguration(getClass().getSimpleName()));
}

@RepeatedIfExceptionsTest
@Override
public void testIndexUpdatesWithReindexAndRemove() throws InterruptedException, ExecutionException {
super.testIndexUpdatesWithReindexAndRemove();
}

@ParameterizedRepeatedIfExceptionsTest(repeats = 4, minSuccess = 2)
@ValueSource(booleans = {true, false})
@Override
public void simpleLogTest(boolean useStringId) throws InterruptedException {
super.simpleLogTest(useStringId);
}

@ParameterizedRepeatedIfExceptionsTest(repeats = 4, minSuccess = 2)
@ValueSource(booleans = {true, false})
@Override
public void simpleLogTestWithFailure(boolean useStringId) throws InterruptedException {
super.simpleLogTestWithFailure(useStringId);
}

@RepeatedIfExceptionsTest(repeats = 4, minSuccess = 2)
@Override
public void testEdgeTTLTiming() throws Exception {
super.testEdgeTTLTiming();
}

@RepeatedIfExceptionsTest(repeats = 4, minSuccess = 2)
@Override
public void testEdgeTTLWithTransactions() throws Exception {
super.testEdgeTTLWithTransactions();
}

@RepeatedIfExceptionsTest(repeats = 4, minSuccess = 2)
@Override
public void testVertexTTLWithCompositeIndex() throws Exception {
super.testVertexTTLWithCompositeIndex();
}

@RepeatedIfExceptionsTest(repeats = 4, minSuccess = 2)
@Override
public void testVertexTTLImplicitKey() throws Exception {
super.testVertexTTLImplicitKey();
}

@RepeatedIfExceptionsTest(repeats = 3)
@Override
public void testReindexingForEdgeIndex() throws ExecutionException, InterruptedException {
super.testReindexingForEdgeIndex();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

package org.janusgraph.graphdb.cql;

import io.github.artsok.ParameterizedRepeatedIfExceptionsTest;
import io.github.artsok.RepeatedIfExceptionsTest;
import org.apache.commons.lang3.RandomStringUtils;
import org.apache.commons.lang3.exception.ExceptionUtils;
Expand Down Expand Up @@ -227,18 +228,36 @@ public void testHasTTL() {
assertTrue(features.hasCellTTL());
}

@ParameterizedTest
@ParameterizedRepeatedIfExceptionsTest(repeats = 4, minSuccess = 2)
@ValueSource(booleans = {true, false})
public void simpleLogTest(boolean useStringId) {
for (int i = 0; i < 3; i++) {
try {
super.simpleLogTest(useStringId);
// If the test passes, break out of the loop.
break;
} catch (Exception ex) {
log.info("Attempt #{} fails", i, ex);
}
}
@Override
public void simpleLogTest(boolean useStringId) throws InterruptedException {
super.simpleLogTest(useStringId);
}

@ParameterizedRepeatedIfExceptionsTest(repeats = 4, minSuccess = 2)
@ValueSource(booleans = {true, false})
@Override
public void simpleLogTestWithFailure(boolean useStringId) throws InterruptedException {
super.simpleLogTestWithFailure(useStringId);
}

@RepeatedIfExceptionsTest(repeats = 4, minSuccess = 2)
@Override
public void testEdgeTTLTiming() throws Exception {
super.testEdgeTTLTiming();
}

@RepeatedIfExceptionsTest(repeats = 4, minSuccess = 2)
@Override
public void testVertexTTLWithCompositeIndex() throws Exception {
super.testVertexTTLWithCompositeIndex();
}

@RepeatedIfExceptionsTest(repeats = 4, minSuccess = 2)
@Override
public void testVertexTTLImplicitKey() throws Exception {
super.testVertexTTLImplicitKey();
}

@RepeatedIfExceptionsTest(repeats = 3)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

package org.janusgraph.graphdb.cql;

import io.github.artsok.RepeatedIfExceptionsTest;
import org.janusgraph.JanusGraphCassandraContainer;
import org.janusgraph.diskstorage.configuration.WriteConfiguration;
import org.janusgraph.olap.OLAPTest;
Expand All @@ -29,4 +30,10 @@ public class CQLOLAPTest extends OLAPTest {
public WriteConfiguration getConfiguration() {
return cqlContainer.getConfiguration(getClass().getSimpleName()).getConfiguration();
}

@RepeatedIfExceptionsTest(repeats = 4, minSuccess = 2)
@Override
public void testShortestDistance() throws Exception {
super.testShortestDistance();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ public ModifiableConfiguration getStorageConfiguration() {
return getBerkeleyJEConfiguration();
}

@RepeatedIfExceptionsTest(repeats = 4, minSuccess = 2)
@Override
public void indexShouldNotExistAfterDeletion() throws Exception {
super.indexShouldNotExistAfterDeletion();
}

/**
* Test {@link org.janusgraph.example.GraphOfTheGodsFactory#create(String)}.
*/
Expand All @@ -55,9 +61,15 @@ public void testGraphOfTheGodsFactoryCreate() {
gotg.close();
}

@RepeatedIfExceptionsTest(repeats = 3)
@RepeatedIfExceptionsTest(repeats = 4, minSuccess = 2)
@Override
public void testDisableAndDiscardManuallyAndDropEnabledIndex() throws Exception {
super.testDisableAndDiscardManuallyAndDropEnabledIndex();
}

@RepeatedIfExceptionsTest(repeats = 4, minSuccess = 2)
@Override
public void testIndexUpdatesWithoutReindex() throws InterruptedException, ExecutionException {
super.testIndexUpdatesWithoutReindex();
public void testDiscardAndDropRegisteredIndex() throws ExecutionException, InterruptedException {
super.testDiscardAndDropRegisteredIndex();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,12 @@

package org.janusgraph.diskstorage.es;

import io.github.artsok.RepeatedIfExceptionsTest;
import org.janusgraph.JanusGraphCassandraContainer;
import org.janusgraph.diskstorage.configuration.ModifiableConfiguration;
import org.junit.jupiter.api.Disabled;
import org.testcontainers.junit.jupiter.Container;
import org.testcontainers.junit.jupiter.Testcontainers;

import java.util.concurrent.ExecutionException;

@Testcontainers
public class CQLElasticsearchTest extends ElasticsearchJanusGraphIndexTest {

Expand All @@ -37,10 +34,4 @@ public ModifiableConfiguration getStorageConfiguration() {
@Override
@Disabled("CQL seems to not clear storage correctly")
public void testClearStorage() {}

@RepeatedIfExceptionsTest(repeats = 3)
@Override
public void testIndexUpdatesWithoutReindex() throws InterruptedException, ExecutionException {
super.testIndexUpdatesWithoutReindex();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

package org.janusgraph.diskstorage.solr;

import io.github.artsok.RepeatedIfExceptionsTest;
import org.janusgraph.JanusGraphCassandraContainer;
import org.janusgraph.diskstorage.configuration.ModifiableConfiguration;
import org.junit.jupiter.api.Test;
Expand All @@ -38,6 +39,12 @@ public boolean supportsWildcardQuery() {
return false;
}

@RepeatedIfExceptionsTest(repeats = 4, minSuccess = 2)
@Override
public void testSetIndexing() {
super.testSetIndexing();
}

@Test
@Override
public void testDiscardAndDropRegisteredIndex() {
Expand Down

1 comment on commit 80ca920

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Benchmark

Benchmark suite Current: 80ca920 Previous: c7f1414 Ratio
org.janusgraph.JanusGraphSpeedBenchmark.basicAddAndDelete 18045.527510436037 ms/op 22832.725066731808 ms/op 0.79
org.janusgraph.GraphCentricQueryBenchmark.getVertices 1666.9211424671946 ms/op 1846.1651353102989 ms/op 0.90
org.janusgraph.MgmtOlapJobBenchmark.runClearIndex 223.1174173652174 ms/op 225.06416455256917 ms/op 0.99
org.janusgraph.MgmtOlapJobBenchmark.runReindex 511.6983136470707 ms/op 577.6853888111111 ms/op 0.89
org.janusgraph.JanusGraphSpeedBenchmark.basicCount 557.7640916903018 ms/op 404.26724358193667 ms/op 1.38
org.janusgraph.CQLMultiQueryMultiSlicesBenchmark.getValuesAllPropertiesWithAllMultiQuerySlicesUnderMaxRequestsPerConnection 12110.929831873293 ms/op 14049.432726261206 ms/op 0.86
org.janusgraph.CQLMultiQueryBenchmark.getElementsWithUsingEmitRepeatSteps 41255.28314532 ms/op 48032.01077955 ms/op 0.86
org.janusgraph.CQLMultiQueryMultiSlicesBenchmark.getValuesMultiplePropertiesWithSmallBatch 42373.17268303333 ms/op 48915.53240540001 ms/op 0.87
org.janusgraph.CQLMultiQueryMultiSlicesBenchmark.vertexCentricPropertiesFetching 86376.7400133 ms/op 105756.8187758 ms/op 0.82
org.janusgraph.CQLMultiQueryBenchmark.getAllElementsTraversedFromOuterVertex 20586.68721763 ms/op 22789.450908244446 ms/op 0.90
org.janusgraph.CQLMultiQueryBenchmark.getVerticesWithDoubleUnion 788.610438992032 ms/op 834.129436581297 ms/op 0.95
org.janusgraph.CQLMultiQueryMultiSlicesBenchmark.getValuesAllPropertiesWithUnlimitedBatch 11110.931732854242 ms/op 12360.737748881762 ms/op 0.90
org.janusgraph.CQLMultiQueryBenchmark.getNames 20156.523978964047 ms/op 22053.325934522858 ms/op 0.91
org.janusgraph.CQLMultiQueryMultiSlicesBenchmark.getValuesThreePropertiesWithAllMultiQuerySlicesUnderMaxRequestsPerConnection 14670.859546341919 ms/op 17268.2833808608 ms/op 0.85
org.janusgraph.CQLMultiQueryBenchmark.getLabels 17495.367401562085 ms/op 19012.308155116116 ms/op 0.92
org.janusgraph.CQLMultiQueryBenchmark.getVerticesFilteredByAndStep 864.4783371097303 ms/op 877.034097341064 ms/op 0.99
org.janusgraph.CQLMultiQueryBenchmark.getVerticesFromMultiNestedRepeatStepStartingFromSingleVertex 28086.259274625005 ms/op 29542.13639576095 ms/op 0.95
org.janusgraph.CQLMultiQueryBenchmark.getVerticesWithCoalesceUsage 719.3315745395477 ms/op 801.440679281071 ms/op 0.90
org.janusgraph.CQLMultiQueryMultiSlicesBenchmark.getValuesMultiplePropertiesWithAllMultiQuerySlicesUnderMaxRequestsPerConnection 38023.237671565716 ms/op 44586.59237236953 ms/op 0.85
org.janusgraph.CQLMultiQueryBenchmark.getIdToOutVerticesProjection 523.3455840596073 ms/op 539.5587642404585 ms/op 0.97
org.janusgraph.CQLMultiQueryMultiSlicesBenchmark.getValuesMultiplePropertiesWithUnlimitedBatch 39738.48283129015 ms/op 48771.697695169096 ms/op 0.81
org.janusgraph.CQLMultiQueryBenchmark.getNeighborNames 20073.46397466461 ms/op 22258.082075511105 ms/op 0.90
org.janusgraph.CQLMultiQueryBenchmark.getElementsWithUsingRepeatUntilSteps 21931.891939964848 ms/op 24524.48026238206 ms/op 0.89
org.janusgraph.CQLMultiQueryBenchmark.getAdjacentVerticesLocalCounts 20781.877719266842 ms/op 23609.766987193336 ms/op 0.88

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.