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:
* #1457
* #1459
* #1462
* #1464
* #1465
* #1497
* #1498
* #2272
* #3142
* #3356
* #3392
* #3393
* #3651
* #3931
* #3959
* #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: #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).

I also added a link to the relevant issue to all flaky tests.

Signed-off-by: Florian Hockmann <[email protected]>
  • Loading branch information
FlorianHockmann committed Sep 14, 2023
1 parent 5480d13 commit 2ba933c
Show file tree
Hide file tree
Showing 17 changed files with 173 additions and 45 deletions.
20 changes: 12 additions & 8 deletions TESTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,14 @@ All of JanusGraph's tests are written for JUnit. JanusGraph's JUnit tests are a

### Marking tests as flaky

If a test should be marked as flaky add following annotation to the test and open an issue.
If a test should be marked as flaky, then first [open an issue](https://github.com/JanusGraph/janusgraph/issues/new?assignees=&labels=testing%2Fflaky&projects=&template=flaky-test.md)
where you can add information about the flaky test that could be helpful later to others to understand why the test is
marked as flaky and hopefully for fixing it.
Afterwards, add the annotation to the test and link to the issue you just created:

```java
@RepeatedIfExceptionsTest(repeats = 4, minSuccess = 2)
// flaky test: https://github.com/JanusGraph/janusgraph/issues/[ISSUE_NUMBER]
@RepeatedIfExceptionsTest(repeats = 3)
public void testFlakyFailsSometimes(){}
```

Expand Down Expand Up @@ -127,7 +131,7 @@ mvn clean install -pl janusgraph-es -Delasticsearch.docker.version=6.0.0 -Delast

**Note** Running CQL tests require Docker.

CQL tests are executed using [testcontainers-java](https://www.testcontainers.org/).
CQL tests are executed using [testcontainers-java](https://www.testcontainers.org/).
CQL tests can be executed against a Cassandra 3 using the profile `cassandra3`, or a Scylla 3 using the profile `scylladb`.

```bash
Expand Down Expand Up @@ -157,10 +161,10 @@ mvn clean install -pl janusgraph-cql -Dcassandra.docker.image=cassandra -Dcassan

### TinkerPop tests

The CQL backend is tested with TinkerPop tests using following command.
The CQL backend is tested with TinkerPop tests using following command.

**Note: Profiles are not supported during running TinkerPop tests.
If you do not want to use the default config, you can set `cassandra.docker.image`,
**Note: Profiles are not supported during running TinkerPop tests.
If you do not want to use the default config, you can set `cassandra.docker.image`,
`cassandra.docker.version`, or `cassandra.docker.partitioner`.**

```bash
Expand All @@ -170,8 +174,8 @@ mvn clean install -Dtest.skip.tp=false -DskipTests=true -pl janusgraph-cql \

### Create new configuration files for new Versions of Cassandra

The file `janusgraph-cql/src/test/resources/docker/docker-compose.yml` can be used to generate new configuration files.
Therefore, you have to start a Cassandra instance using `docker-compose up`.
The file `janusgraph-cql/src/test/resources/docker/docker-compose.yml` can be used to generate new configuration files.
Therefore, you have to start a Cassandra instance using `docker-compose up`.
Afterward, you can extract the configuration which is located in the following file `/etc/cassandra/cassandra.yaml`.

## Running hbase tests
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -845,7 +845,8 @@ public void testRestore() throws Exception {
assertTrue(results.contains("restore-doc1"));
}

@RepeatedIfExceptionsTest(repeats = 4, minSuccess = 2)
// flaky test: https://github.com/JanusGraph/janusgraph/issues/1091
@RepeatedIfExceptionsTest(repeats = 3)
public void testTTL() throws Exception {
if (!index.getFeatures().supportsDocumentTTL())
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ public void mediumSendReceiveSerial() throws Exception {
simpleSendReceive(2000,1);
}

@RepeatedIfExceptionsTest(repeats = 4, minSuccess = 2)
// flaky test: https://github.com/JanusGraph/janusgraph/issues/1445
@RepeatedIfExceptionsTest(repeats = 3)
@Tag(LogTest.requiresOrderPreserving)
public void testMultipleReadersOnSingleLogSerial() throws Exception {
sendReceive(4, 2000, 5, true, TIMEOUT_MS);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2188,7 +2188,8 @@ public void testIndexReplay() throws Exception {
assertEquals(4, recoveryStats[1]); //all 4 index transaction had provoked errors in the indexing backend
}

@Test
// flaky test: https://github.com/JanusGraph/janusgraph/issues/2272
@RepeatedIfExceptionsTest(repeats = 3)
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 Expand Up @@ -3197,7 +3198,8 @@ public void testOrderByWithRange() {

}

@RepeatedIfExceptionsTest(repeats = 4, minSuccess = 2)
// flaky test: https://github.com/JanusGraph/janusgraph/issues/3976
@RepeatedIfExceptionsTest(repeats = 3)
public void shouldUpdateIndexFieldsAfterIndexModification() throws InterruptedException, ExecutionException {
clopen(option(FORCE_INDEX_USAGE), true, option(LOG_READ_INTERVAL, MANAGEMENT_LOG), Duration.ofMillis(5000));
String key1 = "testKey1";
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,8 @@ private void verifyLockingOverwrite(long num) {
}
}

@Test
// flaky test: https://github.com/JanusGraph/janusgraph/issues/1459
@RepeatedIfExceptionsTest(repeats = 3)
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,8 @@ public void createConfigurationShouldSupportMultiHosts() throws Exception {
}
}

@Test
// flaky test: https://github.com/JanusGraph/janusgraph/issues/3393
@RepeatedIfExceptionsTest(repeats = 3)
public void dropGraphShouldRemoveGraphKeyspace() throws Exception {
final MapConfiguration graphConfig = getGraphConfig();
final String graphName = graphConfig.getString(GRAPH_NAME.toStringWithoutRoot());
Expand All @@ -132,10 +133,18 @@ public void dropGraphShouldRemoveGraphKeyspace() throws Exception {
}
}

// flaky test: https://github.com/JanusGraph/janusgraph/issues/3096
@RepeatedIfExceptionsTest(repeats = 3)
@Override
public void updateConfigurationShouldRemoveGraphFromCache() throws Exception {
super.updateConfigurationShouldRemoveGraphFromCache();
}

// flaky test: https://github.com/JanusGraph/janusgraph/issues/3959
@RepeatedIfExceptionsTest(repeats = 3)
@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,62 @@ public class CQLGraphCacheTest extends JanusGraphTest {
public WriteConfiguration getConfiguration() {
return StorageSetup.addPermanentCache(cqlContainer.getConfiguration(getClass().getSimpleName()));
}

// flaky test: https://github.com/JanusGraph/janusgraph/issues/1498
@RepeatedIfExceptionsTest(repeats = 3)
@Override
public void testIndexUpdatesWithReindexAndRemove() throws InterruptedException, ExecutionException {
super.testIndexUpdatesWithReindexAndRemove();
}

// flaky test: https://github.com/JanusGraph/janusgraph/issues/1457
@ParameterizedRepeatedIfExceptionsTest(repeats = 4, minSuccess = 2)
@ValueSource(booleans = {true, false})
@Override
public void simpleLogTest(boolean useStringId) throws InterruptedException {
super.simpleLogTest(useStringId);
}

// flaky test: https://github.com/JanusGraph/janusgraph/issues/1457
@ParameterizedRepeatedIfExceptionsTest(repeats = 4, minSuccess = 2)
@ValueSource(booleans = {true, false})
@Override
public void simpleLogTestWithFailure(boolean useStringId) throws InterruptedException {
super.simpleLogTestWithFailure(useStringId);
}

// flaky test: https://github.com/JanusGraph/janusgraph/issues/1497
@RepeatedIfExceptionsTest(repeats = 3)
@Override
public void testEdgeTTLTiming() throws Exception {
super.testEdgeTTLTiming();
}

// flaky test: https://github.com/JanusGraph/janusgraph/issues/1462
@RepeatedIfExceptionsTest(repeats = 3)
@Override
public void testEdgeTTLWithTransactions() throws Exception {
super.testEdgeTTLWithTransactions();
}

// flaky test: https://github.com/JanusGraph/janusgraph/issues/1464
@RepeatedIfExceptionsTest(repeats = 3)
@Override
public void testVertexTTLWithCompositeIndex() throws Exception {
super.testVertexTTLWithCompositeIndex();
}

// flaky test: https://github.com/JanusGraph/janusgraph/issues/1465
@RepeatedIfExceptionsTest(repeats = 3)
@Override
public void testVertexTTLImplicitKey() throws Exception {
super.testVertexTTLImplicitKey();
}

// flaky test: https://github.com/JanusGraph/janusgraph/issues/3142
@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,20 +228,44 @@ public void testHasTTL() {
assertTrue(features.hasCellTTL());
}

@ParameterizedTest
// flaky test: https://github.com/JanusGraph/janusgraph/issues/1457
@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);
}

// flaky test: https://github.com/JanusGraph/janusgraph/issues/1457
@ParameterizedRepeatedIfExceptionsTest(repeats = 4, minSuccess = 2)
@ValueSource(booleans = {true, false})
@Override
public void simpleLogTestWithFailure(boolean useStringId) throws InterruptedException {
super.simpleLogTestWithFailure(useStringId);
}

// flaky test: https://github.com/JanusGraph/janusgraph/issues/1497
@RepeatedIfExceptionsTest(repeats = 3)
@Override
public void testEdgeTTLTiming() throws Exception {
super.testEdgeTTLTiming();
}

// flaky test: https://github.com/JanusGraph/janusgraph/issues/1464
@RepeatedIfExceptionsTest(repeats = 3)
@Override
public void testVertexTTLWithCompositeIndex() throws Exception {
super.testVertexTTLWithCompositeIndex();
}

// flaky test: https://github.com/JanusGraph/janusgraph/issues/1465
@RepeatedIfExceptionsTest(repeats = 3)
@Override
public void testVertexTTLImplicitKey() throws Exception {
super.testVertexTTLImplicitKey();
}

// flaky test: https://github.com/JanusGraph/janusgraph/issues/3142
@RepeatedIfExceptionsTest(repeats = 3)
@Override
public void testReindexingForEdgeIndex() throws ExecutionException, InterruptedException {
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,11 @@ public class CQLOLAPTest extends OLAPTest {
public WriteConfiguration getConfiguration() {
return cqlContainer.getConfiguration(getClass().getSimpleName()).getConfiguration();
}

// flaky test: https://github.com/JanusGraph/janusgraph/issues/3392
@RepeatedIfExceptionsTest(repeats = 3)
@Override
public void testShortestDistance() throws Exception {
super.testShortestDistance();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ public WriteConfiguration getConfiguration() {
return cql.getConfiguration(getClass().getSimpleName().toLowerCase()).getConfiguration();
}

// flaky test: https://github.com/JanusGraph/janusgraph/issues/3132
@RepeatedIfExceptionsTest(repeats = 3)
@Override
public void testRepairRelationIndex() throws ExecutionException, InterruptedException, BackendException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,13 @@ public ModifiableConfiguration getStorageConfiguration() {
return getBerkeleyJEConfiguration();
}

// flaky test: https://github.com/JanusGraph/janusgraph/issues/3960
@RepeatedIfExceptionsTest(repeats = 3)
@Override
public void indexShouldNotExistAfterDeletion() throws Exception {
super.indexShouldNotExistAfterDeletion();
}

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

// flaky test: https://github.com/JanusGraph/janusgraph/issues/3651
@RepeatedIfExceptionsTest(repeats = 3)
@Override
public void testDisableAndDiscardManuallyAndDropEnabledIndex() throws Exception {
super.testDisableAndDiscardManuallyAndDropEnabledIndex();
}

// flaky test: https://github.com/JanusGraph/janusgraph/issues/3931
@RepeatedIfExceptionsTest(repeats = 3)
@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();
}
}
Loading

1 comment on commit 2ba933c

@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: 2ba933c Previous: 6205752 Ratio
org.janusgraph.JanusGraphSpeedBenchmark.basicAddAndDelete 17790.028860433857 ms/op 22579.688640648168 ms/op 0.79
org.janusgraph.GraphCentricQueryBenchmark.getVertices 1573.3348664649886 ms/op 1693.3271383287579 ms/op 0.93
org.janusgraph.MgmtOlapJobBenchmark.runClearIndex 223.07867799130435 ms/op 222.8017045478261 ms/op 1.00
org.janusgraph.MgmtOlapJobBenchmark.runReindex 516.2405165678788 ms/op 565.8578123657144 ms/op 0.91
org.janusgraph.JanusGraphSpeedBenchmark.basicCount 462.25062694375237 ms/op 458.33261599744327 ms/op 1.01
org.janusgraph.CQLMultiQueryMultiSlicesBenchmark.getValuesAllPropertiesWithAllMultiQuerySlicesUnderMaxRequestsPerConnection 12021.021447749816 ms/op 13027.779634965924 ms/op 0.92
org.janusgraph.CQLMultiQueryBenchmark.getElementsWithUsingEmitRepeatSteps 40034.42651710477 ms/op 42947.178598 ms/op 0.93
org.janusgraph.CQLMultiQueryMultiSlicesBenchmark.getValuesMultiplePropertiesWithSmallBatch 42200.704798399995 ms/op 47549.632632216664 ms/op 0.89
org.janusgraph.CQLMultiQueryMultiSlicesBenchmark.vertexCentricPropertiesFetching 80824.7785585 ms/op 99834.0656897 ms/op 0.81
org.janusgraph.CQLMultiQueryBenchmark.getAllElementsTraversedFromOuterVertex 19983.408471161616 ms/op 21717.084192162452 ms/op 0.92
org.janusgraph.CQLMultiQueryBenchmark.getVerticesWithDoubleUnion 721.1253109541932 ms/op 763.9341528866134 ms/op 0.94
org.janusgraph.CQLMultiQueryMultiSlicesBenchmark.getValuesAllPropertiesWithUnlimitedBatch 10536.424702940381 ms/op 12156.922564930786 ms/op 0.87
org.janusgraph.CQLMultiQueryBenchmark.getNames 19550.846777023096 ms/op 21623.489783162986 ms/op 0.90
org.janusgraph.CQLMultiQueryMultiSlicesBenchmark.getValuesThreePropertiesWithAllMultiQuerySlicesUnderMaxRequestsPerConnection 14481.04516395956 ms/op 16175.502236128377 ms/op 0.90
org.janusgraph.CQLMultiQueryBenchmark.getLabels 17014.382313124403 ms/op
org.janusgraph.CQLMultiQueryBenchmark.getVerticesFilteredByAndStep 851.7295468137982 ms/op 828.1404002420064 ms/op 1.03
org.janusgraph.CQLMultiQueryBenchmark.getVerticesFromMultiNestedRepeatStepStartingFromSingleVertex 26824.522596788887 ms/op 28624.24182517619 ms/op 0.94
org.janusgraph.CQLMultiQueryBenchmark.getVerticesWithCoalesceUsage 744.8942274406727 ms/op 770.5690844916234 ms/op 0.97
org.janusgraph.CQLMultiQueryMultiSlicesBenchmark.getValuesMultiplePropertiesWithAllMultiQuerySlicesUnderMaxRequestsPerConnection 35939.989489688094 ms/op 42753.61769863095 ms/op 0.84
org.janusgraph.CQLMultiQueryBenchmark.getIdToOutVerticesProjection 512.1523021435565 ms/op 518.0698862502832 ms/op 0.99
org.janusgraph.CQLMultiQueryMultiSlicesBenchmark.getValuesMultiplePropertiesWithUnlimitedBatch 39350.08721186206 ms/op 45390.244630558336 ms/op 0.87
org.janusgraph.CQLMultiQueryBenchmark.getNeighborNames 19836.121105306152 ms/op 21751.645129234243 ms/op 0.91
org.janusgraph.CQLMultiQueryBenchmark.getElementsWithUsingRepeatUntilSteps 21223.90408379369 ms/op 22817.45437812944 ms/op 0.93
org.janusgraph.CQLMultiQueryBenchmark.getAdjacentVerticesLocalCounts 20301.19161131714 ms/op 21574.947662841663 ms/op 0.94

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

Please sign in to comment.