From 612c238510c0277e282f47864b5f46ecca2f37f1 Mon Sep 17 00:00:00 2001 From: Dmitriy Tverdiakov <11927660+injectives@users.noreply.github.com> Date: Fri, 13 Aug 2021 13:08:51 +0100 Subject: [PATCH] Migrate tests from Java Driver to Testkit (#981) Replaced tests: - boltSchemeShouldInstantiateDirectDriver -> shouldCreateAppropriateDriverType (DriverFactoryTest unit test) - boltPlusDiscoverySchemeShouldInstantiateClusterDriver -> shouldCreateAppropriateDriverType (DriverFactoryTest unit test) - shouldLogWhenUnableToCreateRoutingDriver -> shouldLogWhenUnableToCreateRoutingDriver (converted to unit test) Migrated tests: - shouldLogWhenUnableToCreateRoutingDriver -> test_should_fail_discovery_when_router_fails_with_procedure_not_found_code (covers the actual logic when error occurs, the logging verification has been converted to unit test) - shouldOnlyPullRecordsWhenNeededSimpleSession -> test_should_accept_custom_fetch_size_using_session_configuration (existing test) --- .../java/org/neo4j/driver/GraphDatabase.java | 18 +++- .../org/neo4j/driver/GraphDatabaseTest.java | 90 ++++++++----------- .../driver/internal/DriverFactoryTest.java | 24 +++++ .../discover_not_supported_9001.script | 10 --- .../discover_not_supported_9002.script | 10 --- .../test/resources/discover_servers.script | 10 --- .../test/resources/dummy_connection.script | 4 - 7 files changed, 73 insertions(+), 93 deletions(-) delete mode 100644 driver/src/test/resources/discover_not_supported_9001.script delete mode 100644 driver/src/test/resources/discover_not_supported_9002.script delete mode 100644 driver/src/test/resources/discover_servers.script delete mode 100644 driver/src/test/resources/dummy_connection.script diff --git a/driver/src/main/java/org/neo4j/driver/GraphDatabase.java b/driver/src/main/java/org/neo4j/driver/GraphDatabase.java index 830b8d9473..e5739d8be2 100644 --- a/driver/src/main/java/org/neo4j/driver/GraphDatabase.java +++ b/driver/src/main/java/org/neo4j/driver/GraphDatabase.java @@ -124,19 +124,24 @@ public static Driver driver( String uri, AuthToken authToken, Config config ) /** * Return a driver for a Neo4j instance with custom configuration. * - * @param uri the URL to a Neo4j instance + * @param uri the URL to a Neo4j instance * @param authToken authentication to use, see {@link AuthTokens} - * @param config user defined configuration + * @param config user defined configuration * @return a new driver to the database instance specified by the URL */ public static Driver driver( URI uri, AuthToken authToken, Config config ) + { + return driver( uri, authToken, config, new DriverFactory() ); + } + + static Driver driver( URI uri, AuthToken authToken, Config config, DriverFactory driverFactory ) { config = getOrDefault( config ); RoutingSettings routingSettings = config.routingSettings(); RetrySettings retrySettings = config.retrySettings(); SecuritySettings securitySettings = config.securitySettings(); SecurityPlan securityPlan = securitySettings.createSecurityPlan( uri.getScheme() ); - return new DriverFactory().newInstance( uri, authToken, routingSettings, retrySettings, config, securityPlan ); + return driverFactory.newInstance( uri, authToken, routingSettings, retrySettings, config, securityPlan ); } /** @@ -151,13 +156,18 @@ public static Driver driver( URI uri, AuthToken authToken, Config config ) * @return a new driver instance */ public static Driver routingDriver( Iterable routingUris, AuthToken authToken, Config config ) + { + return routingDriver( routingUris, authToken, config, new DriverFactory() ); + } + + static Driver routingDriver( Iterable routingUris, AuthToken authToken, Config config, DriverFactory driverFactory ) { assertRoutingUris( routingUris ); Logger log = createLogger( config ); for ( URI uri : routingUris ) { - final Driver driver = driver( uri, authToken, config ); + final Driver driver = driver( uri, authToken, config, driverFactory ); try { driver.verifyConnectivity(); diff --git a/driver/src/test/java/org/neo4j/driver/GraphDatabaseTest.java b/driver/src/test/java/org/neo4j/driver/GraphDatabaseTest.java index df4be61c9d..7cc9bee955 100644 --- a/driver/src/test/java/org/neo4j/driver/GraphDatabaseTest.java +++ b/driver/src/test/java/org/neo4j/driver/GraphDatabaseTest.java @@ -18,6 +18,7 @@ */ package org.neo4j.driver; +import io.netty.util.concurrent.EventExecutorGroup; import org.junit.jupiter.api.Test; import java.io.IOException; @@ -26,68 +27,34 @@ import java.util.List; import org.neo4j.driver.exceptions.ServiceUnavailableException; -import org.neo4j.driver.util.StubServer; +import org.neo4j.driver.internal.BoltServerAddress; +import org.neo4j.driver.internal.DriverFactory; +import org.neo4j.driver.internal.InternalDriver; +import org.neo4j.driver.internal.cluster.RoutingSettings; +import org.neo4j.driver.internal.metrics.MetricsProvider; +import org.neo4j.driver.internal.retry.RetryLogic; +import org.neo4j.driver.internal.security.SecurityPlan; +import org.neo4j.driver.internal.spi.ConnectionPool; import org.neo4j.driver.util.TestUtil; import static java.util.Arrays.asList; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.core.IsEqual.equalTo; import static org.hamcrest.junit.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.neo4j.driver.internal.logging.DevNullLogging.DEV_NULL_LOGGING; -import static org.neo4j.driver.internal.util.Matchers.clusterDriver; -import static org.neo4j.driver.internal.util.Matchers.directDriver; import static org.neo4j.driver.util.StubServer.INSECURE_CONFIG; class GraphDatabaseTest { - @Test - void boltSchemeShouldInstantiateDirectDriver() throws Exception - { - // Given - StubServer server = StubServer.start( "dummy_connection.script", 9001 ); - URI uri = URI.create( "bolt://localhost:9001" ); - - // When - Driver driver = GraphDatabase.driver( uri, INSECURE_CONFIG ); - driver.verifyConnectivity(); - - // Then - assertThat( driver, is( directDriver() ) ); - - // Finally - driver.close(); - assertThat( server.exitStatus(), equalTo( 0 ) ); - } - - @Test - void boltPlusDiscoverySchemeShouldInstantiateClusterDriver() throws Exception - { - // Given - StubServer server = StubServer.start( "discover_servers.script", 9001 ); - URI uri = URI.create( "neo4j://127.0.0.1:9001" ); - - // When - Driver driver = GraphDatabase.driver( uri, INSECURE_CONFIG ); - driver.verifyConnectivity(); - - // Then - assertThat( driver, is( clusterDriver() ) ); - - // Finally - driver.close(); - assertThat( server.exitStatus(), equalTo( 0 ) ); - } - @Test void throwsWhenBoltSchemeUsedWithRoutingParams() { @@ -95,34 +62,29 @@ void throwsWhenBoltSchemeUsedWithRoutingParams() } @Test - void shouldLogWhenUnableToCreateRoutingDriver() throws Exception + void shouldLogWhenUnableToCreateRoutingDriver() { - StubServer server1 = StubServer.start( "discover_not_supported_9001.script", 9001 ); - StubServer server2 = StubServer.start( "discover_not_supported_9002.script", 9002 ); - Logging logging = mock( Logging.class ); Logger logger = mock( Logger.class ); when( logging.getLog( anyString() ) ).thenReturn( logger ); - + InternalDriver driver = mock( InternalDriver.class ); + doThrow( ServiceUnavailableException.class ).when( driver ).verifyConnectivity(); + DriverFactory driverFactory = new MockSupplyingDriverFactory( driver ); Config config = Config.builder() - .withoutEncryption() - .withLogging( logging ) - .build(); + .withLogging( logging ) + .build(); List routingUris = asList( URI.create( "neo4j://localhost:9001" ), URI.create( "neo4j://localhost:9002" ) ); - assertThrows( ServiceUnavailableException.class, () -> GraphDatabase.routingDriver( routingUris, AuthTokens.none(), config ) ); + assertThrows( ServiceUnavailableException.class, () -> GraphDatabase.routingDriver( routingUris, AuthTokens.none(), config, driverFactory ) ); verify( logger ).warn( eq( "Unable to create routing driver for URI: neo4j://localhost:9001" ), any( Throwable.class ) ); verify( logger ).warn( eq( "Unable to create routing driver for URI: neo4j://localhost:9002" ), any( Throwable.class ) ); - - assertEquals( 0, server1.exitStatus() ); - assertEquals( 0, server2.exitStatus() ); } @Test @@ -215,4 +177,22 @@ private static Config createConfig( boolean encrypted, int timeoutMillis ) return configBuilder.build(); } + + private static class MockSupplyingDriverFactory extends DriverFactory + { + private final InternalDriver driver; + + private MockSupplyingDriverFactory( InternalDriver driver ) + { + this.driver = driver; + } + + @Override + protected InternalDriver createRoutingDriver( SecurityPlan securityPlan, BoltServerAddress address, ConnectionPool connectionPool, + EventExecutorGroup eventExecutorGroup, RoutingSettings routingSettings, RetryLogic retryLogic, + MetricsProvider metricsProvider, Config config ) + { + return driver; + } + } } diff --git a/driver/src/test/java/org/neo4j/driver/internal/DriverFactoryTest.java b/driver/src/test/java/org/neo4j/driver/internal/DriverFactoryTest.java index 3e186ba7fa..8ce2979f7c 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/DriverFactoryTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/DriverFactoryTest.java @@ -57,6 +57,7 @@ import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.fail; import static org.mockito.Mockito.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -66,6 +67,8 @@ import static org.neo4j.driver.internal.metrics.MetricsProvider.METRICS_DISABLED_PROVIDER; import static org.neo4j.driver.internal.util.Futures.completedWithNull; import static org.neo4j.driver.internal.util.Futures.failedFuture; +import static org.neo4j.driver.internal.util.Matchers.clusterDriver; +import static org.neo4j.driver.internal.util.Matchers.directDriver; class DriverFactoryTest { @@ -168,6 +171,27 @@ void shouldCreateDriverMetricsIfMonitoringEnabled() assertThat( provider instanceof InternalMetricsProvider, is( true ) ); } + @ParameterizedTest + @MethodSource( "testUris" ) + void shouldCreateAppropriateDriverType( String uri ) + { + DriverFactory driverFactory = new DriverFactory(); + Driver driver = createDriver( uri, driverFactory ); + + if ( uri.startsWith( "bolt://" ) ) + { + assertThat( driver, is( directDriver() ) ); + } + else if ( uri.startsWith( "neo4j://" ) ) + { + assertThat( driver, is( clusterDriver() ) ); + } + else + { + fail( "Unexpected scheme provided in argument" ); + } + } + private Driver createDriver( String uri, DriverFactory driverFactory ) { return createDriver( uri, driverFactory, defaultConfig() ); diff --git a/driver/src/test/resources/discover_not_supported_9001.script b/driver/src/test/resources/discover_not_supported_9001.script deleted file mode 100644 index 4152127939..0000000000 --- a/driver/src/test/resources/discover_not_supported_9001.script +++ /dev/null @@ -1,10 +0,0 @@ -!: BOLT 3 -!: AUTO RESET -!: AUTO HELLO -!: AUTO GOODBYE - -C: RUN "CALL dbms.cluster.routing.getRoutingTable($context)" {"context": { "address": "localhost:9001" }} {} -C: PULL_ALL -S: FAILURE {"code": "Neo.ClientError.Procedure.ProcedureNotFound", "message": "blabla"} -S: IGNORED -S: diff --git a/driver/src/test/resources/discover_not_supported_9002.script b/driver/src/test/resources/discover_not_supported_9002.script deleted file mode 100644 index d6227b1501..0000000000 --- a/driver/src/test/resources/discover_not_supported_9002.script +++ /dev/null @@ -1,10 +0,0 @@ -!: BOLT 3 -!: AUTO RESET -!: AUTO HELLO -!: AUTO GOODBYE - -C: RUN "CALL dbms.cluster.routing.getRoutingTable($context)" {"context": { "address": "localhost:9002" }} {} -C: PULL_ALL -S: FAILURE {"code": "Neo.ClientError.Procedure.ProcedureNotFound", "message": "blabla"} -S: IGNORED -S: diff --git a/driver/src/test/resources/discover_servers.script b/driver/src/test/resources/discover_servers.script deleted file mode 100644 index 77b607db56..0000000000 --- a/driver/src/test/resources/discover_servers.script +++ /dev/null @@ -1,10 +0,0 @@ -!: BOLT 3 -!: AUTO RESET -!: AUTO HELLO -!: AUTO GOODBYE - -C: RUN "CALL dbms.cluster.routing.getRoutingTable($context)" {"context": {"address": "127.0.0.1:9001"}} {} - PULL_ALL -S: SUCCESS {"fields": ["ttl", "servers"]} - RECORD [9223372036854775807, [{"addresses": ["127.0.0.1:9001"],"role": "WRITE"}, {"addresses": ["127.0.0.1:9002","127.0.0.1:9003","127.0.0.1:9004"], "role": "READ"},{"addresses": ["127.0.0.1:9001","127.0.0.1:9002","127.0.0.1:9003"], "role": "ROUTE"}]] - SUCCESS {} diff --git a/driver/src/test/resources/dummy_connection.script b/driver/src/test/resources/dummy_connection.script deleted file mode 100644 index 0dbb929280..0000000000 --- a/driver/src/test/resources/dummy_connection.script +++ /dev/null @@ -1,4 +0,0 @@ -!: BOLT 3 -!: AUTO RESET -!: AUTO HELLO -!: AUTO GOODBYE