Skip to content

Commit 31e9046

Browse files
lutovichlutovich
committed
Always close driver instance in tests
Driver now holds Netty related resources which include channels and event loop threads. This makes it more important to close driver instances in tests. This commit makes sure all existing unit & integration tests close drivers.
1 parent d27f4f3 commit 31e9046

File tree

6 files changed

+199
-162
lines changed

6 files changed

+199
-162
lines changed

‎driver/src/test/java/org/neo4j/driver/internal/RoutingDriverTest.java‎

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
*/
1919
package org.neo4j.driver.internal;
2020

21+
import org.junit.After;
2122
import org.junit.Rule;
2223
import org.junit.Test;
2324
import org.junit.rules.ExpectedException;
@@ -77,6 +78,17 @@ public class RoutingDriverTest
7778
private static final String GET_SERVERS = "CALL dbms.cluster.routing.getServers";
7879
private final FakeClock clock = new FakeClock();
7980

81+
private Driver driver;
82+
83+
@After
84+
public void tearDown()
85+
{
86+
if ( driver != null )
87+
{
88+
driver.close();
89+
}
90+
}
91+
8092
@Test
8193
public void shouldDiscoveryOnInitialization()
8294
{
@@ -88,7 +100,7 @@ public void shouldDiscoveryOnInitialization()
88100
serverInfo( "WRITE", "localhost:3333" ) );
89101

90102
// When
91-
driverWithPool( pool );
103+
driver = driverWithPool( pool );
92104

93105
// Then
94106
verify( pool ).acquire( SEED );
@@ -98,7 +110,7 @@ public void shouldDiscoveryOnInitialization()
98110
public void shouldRediscoveryIfNoWritersProvided()
99111
{
100112
// Given
101-
Driver driver = driverWithPool( pool(
113+
driver = driverWithPool( pool(
102114
withServers( 10, serverInfo( "ROUTE", "localhost:1111" ),
103115
serverInfo( "WRITE" ),
104116
serverInfo( "READ", "localhost:5555" ) ),
@@ -117,7 +129,7 @@ public void shouldRediscoveryIfNoWritersProvided()
117129
public void shouldNotRediscoveryOnSessionAcquisitionIfNotNecessary()
118130
{
119131
// Given
120-
Driver driver = driverWithPool( pool(
132+
driver = driverWithPool( pool(
121133
withServers( 10, serverInfo( "ROUTE", "localhost:1111", "localhost:1112", "localhost:1113" ),
122134
serverInfo( "READ", "localhost:2222" ),
123135
serverInfo( "WRITE", "localhost:3333" ) ),
@@ -144,7 +156,7 @@ public void shouldFailIfNoRouting()
144156
// When
145157
try
146158
{
147-
driverWithPool( pool );
159+
driver = driverWithPool( pool );
148160
}
149161
// Then
150162
catch ( ServiceUnavailableException e )
@@ -167,7 +179,7 @@ public void shouldFailIfNoRoutersProvided()
167179
// When
168180
try
169181
{
170-
driverWithPool( pool );
182+
driver = driverWithPool( pool );
171183
}
172184
// Then
173185
catch ( ProtocolException e )
@@ -189,7 +201,7 @@ public void shouldFailIfNoReaderProvided()
189201
// When
190202
try
191203
{
192-
driverWithPool( pool );
204+
driver = driverWithPool( pool );
193205
}
194206
// Then
195207
catch ( ProtocolException e )
@@ -210,7 +222,7 @@ public void shouldForgetServersOnRediscovery()
210222
serverInfo( "READ", "localhost:2222" ),
211223
serverInfo( "WRITE", "localhost:3333" ) ) );
212224

213-
Driver driver = driverWithPool( pool );
225+
driver = driverWithPool( pool );
214226

215227
// When
216228
NetworkSessionWithAddress write1 = (NetworkSessionWithAddress) driver.session( AccessMode.WRITE );
@@ -225,7 +237,7 @@ public void shouldForgetServersOnRediscovery()
225237
public void shouldRediscoverOnTimeout()
226238
{
227239
// Given
228-
Driver driver = driverWithPool( pool(
240+
driver = driverWithPool( pool(
229241
withServers( 10, serverInfo( "ROUTE", "localhost:1111", "localhost:1112", "localhost:1113" ),
230242
serverInfo( "READ", "localhost:2222" ),
231243
serverInfo( "WRITE", "localhost:3333" ) ),
@@ -248,7 +260,7 @@ public void shouldRediscoverOnTimeout()
248260
public void shouldNotRediscoverWhenNoTimeout()
249261
{
250262
// Given
251-
Driver driver = driverWithPool( pool(
263+
driver = driverWithPool( pool(
252264
withServers( 10, serverInfo( "ROUTE", "localhost:1111", "localhost:1112", "localhost:1113" ),
253265
serverInfo( "READ", "localhost:2222" ),
254266
serverInfo( "WRITE", "localhost:3333" ) ),
@@ -270,7 +282,7 @@ public void shouldNotRediscoverWhenNoTimeout()
270282
public void shouldRoundRobinAmongReadServers()
271283
{
272284
// Given
273-
Driver driver = driverWithServers( 60,
285+
driver = driverWithServers( 60,
274286
serverInfo( "ROUTE", "localhost:1111", "localhost:1112" ),
275287
serverInfo( "READ", "localhost:2222", "localhost:2223", "localhost:2224" ),
276288
serverInfo( "WRITE", "localhost:3333" ) );
@@ -296,7 +308,7 @@ public void shouldRoundRobinAmongReadServers()
296308
public void shouldRoundRobinAmongWriteServers()
297309
{
298310
// Given
299-
Driver driver = driverWithServers( 60, serverInfo( "ROUTE", "localhost:1111", "localhost:1112" ),
311+
driver = driverWithServers( 60, serverInfo( "ROUTE", "localhost:1111", "localhost:1112" ),
300312
serverInfo( "READ", "localhost:3333" ),
301313
serverInfo( "WRITE", "localhost:2222", "localhost:2223", "localhost:2224" ) );
302314

@@ -329,7 +341,7 @@ public void testTrustOnFirstUseNotCompatibleWithRoutingDriver()
329341
try
330342
{
331343
// When
332-
GraphDatabase.driver( "bolt+routing://127.0.0.1:7687", tofuConfig );
344+
driver = GraphDatabase.driver( "bolt+routing://127.0.0.1:7687", tofuConfig );
333345
fail();
334346
}
335347
catch ( IllegalArgumentException e )

‎driver/src/test/java/org/neo4j/driver/v1/integration/SessionIT.java‎

Lines changed: 30 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
package org.neo4j.driver.v1.integration;
2020

2121
import org.hamcrest.MatcherAssert;
22+
import org.junit.After;
2223
import org.junit.Rule;
2324
import org.junit.Test;
2425
import org.junit.rules.ExpectedException;
@@ -96,36 +97,43 @@ public class SessionIT
9697
@Rule
9798
public ExpectedException exception = ExpectedException.none();
9899

100+
private Driver driver;
101+
102+
@After
103+
public void tearDown()
104+
{
105+
if ( driver != null )
106+
{
107+
driver.close();
108+
}
109+
}
110+
99111
@Test
100112
public void shouldKnowSessionIsClosed() throws Throwable
101113
{
102114
// Given
103-
try ( Driver driver = newDriver() )
104-
{
105-
Session session = driver.session();
115+
driver = newDriver();
116+
Session session = driver.session();
106117

107-
// When
108-
session.close();
118+
// When
119+
session.close();
109120

110-
// Then
111-
assertFalse( session.isOpen() );
112-
}
121+
// Then
122+
assertFalse( session.isOpen() );
113123
}
114124

115125
@Test
116126
public void shouldHandleNullConfig() throws Throwable
117127
{
118128
// Given
119-
try ( Driver driver = GraphDatabase.driver( neo4j.uri(), neo4j.authToken(), null ) )
120-
{
121-
Session session = driver.session();
129+
driver = GraphDatabase.driver( neo4j.uri(), neo4j.authToken(), null );
130+
Session session = driver.session();
122131

123-
// When
124-
session.close();
132+
// When
133+
session.close();
125134

126-
// Then
127-
assertFalse( session.isOpen() );
128-
}
135+
// Then
136+
assertFalse( session.isOpen() );
129137
}
130138

131139
@SuppressWarnings( "ConstantConditions" )
@@ -138,15 +146,15 @@ public void shouldHandleNullAuthToken() throws Throwable
138146

139147
// null auth token should be interpreted as AuthTokens.none() and fail driver creation
140148
// because server expects basic auth
141-
GraphDatabase.driver( neo4j.uri(), token );
149+
driver = GraphDatabase.driver( neo4j.uri(), token );
142150
}
143151

144152
@Test
145153
public void shouldKillLongRunningStatement() throws Throwable
146154
{
147155
neo4j.ensureProcedures( "longRunningStatement.jar" );
148156
// Given
149-
Driver driver = newDriver();
157+
driver = newDriver();
150158

151159
int executionTimeout = 10; // 10s
152160
final int killTimeout = 1; // 1s
@@ -184,7 +192,7 @@ public void shouldKillLongStreamingResult() throws Throwable
184192
{
185193
neo4j.ensureProcedures( "longRunningStatement.jar" );
186194
// Given
187-
Driver driver = newDriver();
195+
driver = newDriver();
188196

189197
int executionTimeout = 10; // 10s
190198
final int killTimeout = 1; // 1s
@@ -226,7 +234,7 @@ public void shouldNotAllowBeginTxIfResetFailureIsNotConsumed() throws Throwable
226234
{
227235
// Given
228236
neo4j.ensureProcedures( "longRunningStatement.jar" );
229-
Driver driver = newDriver();
237+
driver = newDriver();
230238

231239
try ( Session session = driver.session() )
232240
{
@@ -253,7 +261,7 @@ public void shouldThrowExceptionOnCloseIfResetFailureIsNotConsumed() throws Thro
253261
{
254262
// Given
255263
neo4j.ensureProcedures( "longRunningStatement.jar" );
256-
Driver driver = newDriver();
264+
driver = newDriver();
257265

258266
Session session = driver.session();
259267
session.run( "CALL test.driver.longRunningStatement({seconds})",
@@ -275,7 +283,7 @@ public void shouldBeAbleToBeginTxAfterResetFailureIsConsumed() throws Throwable
275283
{
276284
// Given
277285
neo4j.ensureProcedures( "longRunningStatement.jar" );
278-
Driver driver = newDriver();
286+
driver = newDriver();
279287

280288
try ( Session session = driver.session() )
281289
{

‎driver/src/test/java/org/neo4j/driver/v1/tck/DriverSecurityComplianceSteps.java‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ public void aRunningDatabase() throws Throwable
7272
@When( "I connect via a TLS-enabled transport for the first time for the given hostname and port$" )
7373
public void firstUseConnect() throws Throwable
7474
{
75+
closeExistingDriver();
7576
knownHostsFile = tempFile( "known_hosts", ".tmp" );
7677
driver = GraphDatabase.driver(
7778
Neo4jRunner.DEFAULT_URI,
@@ -104,6 +105,7 @@ public void aRunningNeoJDatabaseThatIHaveConnectedTo() throws Throwable
104105
@When( "^I connect via a TLS-enabled transport again$" )
105106
public void iConnectViaATlsEnabledTransportAgain() throws Throwable
106107
{
108+
closeExistingDriver();
107109
try
108110
{
109111
driver = GraphDatabase.driver(
@@ -242,6 +244,7 @@ public void aRunningNeo4jDatabaseUsingACertificateSignedByTheSameTrustedCertific
242244
@When( "^I connect via a TLS-enabled transport$" )
243245
public void iConnectViaATlsEnabledTransport()
244246
{
247+
closeExistingDriver();
245248
try
246249
{
247250
// give root certificate to driver
@@ -285,12 +288,7 @@ public void iShouldGetAHelpfulErrorExplainingThatCertificatedNotSigned() throws
285288
@After( "@tls" )
286289
public void clearAfterEachScenario() throws Throwable
287290
{
288-
if ( driver != null )
289-
{
290-
driver.close();
291-
}
292-
293-
driver = null;
291+
closeExistingDriver();
294292
knownHostsFile = null;
295293
exception = null;
296294

@@ -307,6 +305,15 @@ public void resetDbWithDefaultSettings() throws Throwable
307305
neo4j.restartDb();
308306
}
309307

308+
private void closeExistingDriver()
309+
{
310+
if ( driver != null )
311+
{
312+
driver.close();
313+
driver = null;
314+
}
315+
}
316+
310317
private File tempFile( String prefix, String suffix ) throws Throwable
311318
{
312319
File file = createTempFile( prefix, suffix );

‎driver/src/test/java/org/neo4j/driver/v1/util/Neo4jRunner.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,7 @@ public void run()
293293
try
294294
{
295295
debug("Starting shutdown hook");
296+
driver.close();
296297
stopNeo4j();
297298
debug("Finished shutdown hook");
298299
}

‎driver/src/test/java/org/neo4j/driver/v1/util/cc/Cluster.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,10 @@ private static Driver createDriver( Set<ClusterMember> members, String password
301301
{
302302
return driver;
303303
}
304+
else
305+
{
306+
driver.close();
307+
}
304308
}
305309
catch ( Exception e )
306310
{

0 commit comments

Comments
 (0)