Migrate tests from Java Driver to Testkit (#982) (#985)

New features:
- `Temporary:DriverMaxTxRetryTime` - configures maximum transaction retry time.

Replaced tests:
- shouldCloseChannelWhenResetFails -> ChannelErrorHandler.channelInactive (existing unit test)

Migrated tests:
- shouldRetryReadTransactionUntilFailure -> test_should_fail_when_reading_from_unexpectedly_interrupting_readers_using_tx_function
- shouldRetryWriteTransactionUntilFailure -> test_should_fail_when_writing_to_unexpectedly_interrupting_writers_using_tx_function
- useSessionAfterDriverIsClosed -> test_should_fail_when_driver_closed_using_session_run

Author: injectives

driver/src/main/java/org/neo4j/driver/internal/async/pool/ConnectionPoolImpl.java

@@ -52,6 +52,8 @@
 
 public class ConnectionPoolImpl implements ConnectionPool
 {
+    public static final String CONNECTION_POOL_CLOSED_ERROR_MESSAGE = "Pool closed";
+
     private final ChannelConnector connector;
     private final Bootstrap bootstrap;
     private final NettyChannelTracker nettyChannelTracker;
@@ -227,7 +229,7 @@ private void assertNotClosed()
     {
         if ( closed.get() )
         {
-            throw new IllegalStateException( "Pool closed" );
+            throw new IllegalStateException( CONNECTION_POOL_CLOSED_ERROR_MESSAGE );
         }
     }
 

driver/src/test/java/org/neo4j/driver/integration/RoutingDriverBoltKitIT.java

@@ -27,27 +27,14 @@
 
 import java.io.IOException;
 import java.net.URI;
-import java.util.Comparator;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicInteger;
 
-import org.neo4j.driver.AccessMode;
-import org.neo4j.driver.AuthToken;
-import org.neo4j.driver.AuthTokens;
 import org.neo4j.driver.Config;
 import org.neo4j.driver.Driver;
 import org.neo4j.driver.GraphDatabase;
 import org.neo4j.driver.Record;
-import org.neo4j.driver.Session;
-import org.neo4j.driver.TransactionWork;
 import org.neo4j.driver.async.AsyncSession;
-import org.neo4j.driver.exceptions.SessionExpiredException;
-import org.neo4j.driver.internal.DriverFactory;
-import org.neo4j.driver.internal.cluster.RoutingSettings;
-import org.neo4j.driver.internal.retry.RetrySettings;
-import org.neo4j.driver.internal.security.SecurityPlanImpl;
-import org.neo4j.driver.internal.util.DriverFactoryWithFixedRetryLogic;
 import org.neo4j.driver.internal.util.Futures;
 import org.neo4j.driver.net.ServerAddress;
 import org.neo4j.driver.net.ServerAddressResolver;
@@ -66,7 +53,6 @@
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 import static org.neo4j.driver.SessionConfig.builder;
-import static org.neo4j.driver.util.StubServer.INSECURE_CONFIG;
 import static org.neo4j.driver.util.StubServer.insecureBuilder;
 
 /**
@@ -166,50 +152,6 @@ void shouldHandleLeaderSwitchAndRetryWhenWritingInTxFunctionRX() throws IOExcept
         assertThat( writeServer.exitStatus(), equalTo( 0 ) );
     }
 
-    // fixed retries are not currently supported in testkit
-    @Test
-    void shouldRetryReadTransactionUntilFailure() throws Exception
-    {
-        StubServer router = stubController.startStub( "acquire_endpoints_v3.script", 9001 );
-        StubServer brokenReader1 = stubController.startStub( "dead_read_server_tx.script", 9005 );
-        StubServer brokenReader2 = stubController.startStub( "dead_read_server_tx.script", 9006 );
-
-        try ( Driver driver = newDriverWithFixedRetries( "neo4j://127.0.0.1:9001", 1 ); Session session = driver.session() )
-        {
-            AtomicInteger invocations = new AtomicInteger();
-            assertThrows( SessionExpiredException.class, () -> session.readTransaction( queryWork( "MATCH (n) RETURN n.name", invocations ) ) );
-            assertEquals( 2, invocations.get() );
-        }
-        finally
-        {
-            assertEquals( 0, router.exitStatus() );
-            assertEquals( 0, brokenReader1.exitStatus() );
-            assertEquals( 0, brokenReader2.exitStatus() );
-        }
-    }
-
-    // fixed retries are not currently supported in testkit
-    @Test
-    void shouldRetryWriteTransactionUntilFailure() throws Exception
-    {
-        StubServer router = stubController.startStub( "acquire_endpoints_v3.script", 9001 );
-        StubServer brokenWriter1 = stubController.startStub( "dead_write_server.script", 9007 );
-        StubServer brokenWriter2 = stubController.startStub( "dead_write_server.script", 9008 );
-
-        try ( Driver driver = newDriverWithFixedRetries( "neo4j://127.0.0.1:9001", 1 ); Session session = driver.session() )
-        {
-            AtomicInteger invocations = new AtomicInteger();
-            assertThrows( SessionExpiredException.class, () -> session.writeTransaction( queryWork( "CREATE (n {name:'Bob'})", invocations ) ) );
-            assertEquals( 2, invocations.get() );
-        }
-        finally
-        {
-            assertEquals( 0, router.exitStatus() );
-            assertEquals( 0, brokenWriter1.exitStatus() );
-            assertEquals( 0, brokenWriter2.exitStatus() );
-        }
-    }
-
     @Test
     void shouldFailInitialDiscoveryWhenConfiguredResolverThrows()
     {
@@ -223,65 +165,4 @@ void shouldFailInitialDiscoveryWhenConfiguredResolverThrows()
         assertEquals( "Resolution failure!", error.getMessage() );
         verify( resolver ).resolve( ServerAddress.of( "my.server.com", 9001 ) );
     }
-
-    // general error reporting and handling should be improved before this can be moved to testkit
-    // also, backend closes socket on general errors and it negatively impacts testkit's teardown process
-    @Test
-    void useSessionAfterDriverIsClosed() throws Exception
-    {
-        StubServer router = stubController.startStub( "acquire_endpoints_v3.script", 9001 );
-        StubServer readServer = stubController.startStub( "read_server_v3_read.script", 9005 );
-
-        try ( Driver driver = GraphDatabase.driver( "neo4j://127.0.0.1:9001", INSECURE_CONFIG ) )
-        {
-            try ( Session session = driver.session( builder().withDefaultAccessMode( AccessMode.READ ).build() ) )
-            {
-                List<Record> records = session.run( "MATCH (n) RETURN n.name" ).list();
-                assertEquals( 3, records.size() );
-            }
-
-            Session session = driver.session( builder().withDefaultAccessMode( AccessMode.READ ).build() );
-
-            driver.close();
-
-            assertThrows( IllegalStateException.class, () -> session.run( "MATCH (n) RETURN n.name" ) );
-        }
-        finally
-        {
-            assertEquals( 0, readServer.exitStatus() );
-            assertEquals( 0, router.exitStatus() );
-        }
-    }
-
-    private static Driver newDriverWithFixedRetries( String uriString, int retries )
-    {
-        DriverFactory driverFactory = new DriverFactoryWithFixedRetryLogic( retries );
-        return newDriver( uriString, driverFactory, INSECURE_CONFIG );
-    }
-
-    private static Driver newDriver( String uriString, DriverFactory driverFactory, Config config )
-    {
-        URI uri = URI.create( uriString );
-        RoutingSettings routingConf = new RoutingSettings( 1, 1, 0, null );
-        AuthToken auth = AuthTokens.none();
-        return driverFactory.newInstance( uri, auth, routingConf, RetrySettings.DEFAULT, config, SecurityPlanImpl.insecure() );
-    }
-
-    private static TransactionWork<List<Record>> queryWork( final String query, final AtomicInteger invocations )
-    {
-        return tx ->
-        {
-            invocations.incrementAndGet();
-            return tx.run( query ).list();
-        };
-    }
-
-    static class PortBasedServerAddressComparator implements Comparator<ServerAddress>
-    {
-        @Override
-        public int compare( ServerAddress a1, ServerAddress a2 )
-        {
-            return Integer.compare( a1.port(), a2.port() );
-        }
-    }
 }

driver/src/test/java/org/neo4j/driver/internal/DirectDriverBoltKitIT.java

@@ -18,42 +18,29 @@
  */
 package org.neo4j.driver.internal;
 
-import io.netty.channel.Channel;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
 import reactor.test.StepVerifier;
 
-import java.net.URI;
 import java.util.ArrayList;
 import java.util.List;
 
-import org.neo4j.driver.AuthTokens;
-import org.neo4j.driver.Config;
 import org.neo4j.driver.Driver;
 import org.neo4j.driver.GraphDatabase;
-import org.neo4j.driver.Session;
 import org.neo4j.driver.async.AsyncSession;
 import org.neo4j.driver.async.ResultCursor;
-import org.neo4j.driver.internal.cluster.RoutingSettings;
-import org.neo4j.driver.internal.retry.RetrySettings;
-import org.neo4j.driver.internal.security.SecurityPlanImpl;
-import org.neo4j.driver.internal.util.Clock;
-import org.neo4j.driver.internal.util.io.ChannelTrackingDriverFactory;
 import org.neo4j.driver.reactive.RxResult;
 import org.neo4j.driver.reactive.RxSession;
 import org.neo4j.driver.util.StubServer;
 import org.neo4j.driver.util.StubServerController;
 
 import static java.util.Arrays.asList;
 import static java.util.Collections.singletonList;
-import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.neo4j.driver.SessionConfig.builder;
-import static org.neo4j.driver.internal.logging.DevNullLogging.DEV_NULL_LOGGING;
 import static org.neo4j.driver.util.StubServer.INSECURE_CONFIG;
 import static org.neo4j.driver.util.TestUtil.await;
 
@@ -73,37 +60,6 @@ public void killServers()
         stubController.reset();
     }
 
-    @Test
-    void shouldCloseChannelWhenResetFails() throws Exception
-    {
-        StubServer server = stubController.startStub( "reset_error.script", 9001 );
-        try
-        {
-            URI uri = URI.create( "bolt://localhost:9001" );
-            Config config = Config.builder().withLogging( DEV_NULL_LOGGING ).withoutEncryption().build();
-            ChannelTrackingDriverFactory driverFactory = new ChannelTrackingDriverFactory( 1, Clock.SYSTEM );
-
-            try ( Driver driver = driverFactory.newInstance( uri, AuthTokens.none(), RoutingSettings.DEFAULT, RetrySettings.DEFAULT, config,
-                                                             SecurityPlanImpl.insecure() ) )
-            {
-                try ( Session session = driver.session() )
-                {
-                    assertEquals( 42, session.run( "RETURN 42 AS answer" ).single().get( 0 ).asInt() );
-                }
-
-                List<Channel> channels = driverFactory.pollChannels();
-                // there should be a single channel
-                assertEquals( 1, channels.size() );
-                // and it should be closed because it failed to RESET
-                assertNull( channels.get( 0 ).closeFuture().get( 30, SECONDS ) );
-            }
-        }
-        finally
-        {
-            assertEquals( 0, server.exitStatus() );
-        }
-    }
-
     @Test
     void shouldStreamingRecordsInBatchesRx() throws Exception
     {