Unflake by only asserting on termination, not on explicitness

neo4j · Mar 24, 2017 · 48e7007 · 48e7007
1 parent 990158c
commit 48e7007
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 16 deletions.
diff --git a/.../java/org/neo4j/server/security/enterprise/auth/BuiltInProceduresInteractionTestBase.java b/.../java/org/neo4j/server/security/enterprise/auth/BuiltInProceduresInteractionTestBase.java
@@ -360,7 +360,7 @@ public void queryWaitingForLocksShouldBeKilledBeforeLocksAreReleased() throws Th
                 "RETURN 1",
                 itr -> assertThat( itr.hasNext(), equalTo( true ) ) );
 
-        tx2.closeAndAssertQueryKilled();
+        tx2.closeAndAssertSomeTermination();
 
         // allow query1 to exit procedure and finish
         ClassWithProcedures.doubleLatch.finish();
@@ -406,7 +406,7 @@ private void executeTwoQueriesAndKillTheFirst( S executor1, S executor2, S kille
         );
 
         latch.finishAndWaitForAllToFinish();
-        tx1.closeAndAssertTransactionTermination();
+        tx1.closeAndAssertExplicitTermination();
         tx2.closeAndAssertSuccess();
 
         assertEmpty( adminSubject,
@@ -517,7 +517,10 @@ public void shouldTerminateQueriesEvenIfUsingPeriodicCommit() throws Throwable
                 latch.finishAndWaitForAllToFinish();
 
                 // Then
-                write.closeAndAssertTransactionTermination();
+                // We cannot assert on explicit termination here, because if the termination is detected when trying
+                // to lock we will only get the general TransactionTerminatedException
+                // (see {@link LockClientStateHolder}).
+                write.closeAndAssertSomeTermination();
 
                 // stop server after assertion to avoid other kind of failures due to races (e.g., already closed
                 // lock clients )
@@ -560,8 +563,8 @@ public void shouldKillMultipleUserQueries() throws Throwable
         );
 
         latch.finishAndWaitForAllToFinish();
-        read1.closeAndAssertTransactionTermination();
+        read1.closeAndAssertExplicitTermination();
-        read2.closeAndAssertTransactionTermination();
+        read2.closeAndAssertExplicitTermination();
         read3.closeAndAssertSuccess();
         write.closeAndAssertSuccess();
 
@@ -845,7 +848,7 @@ public void shouldTerminateTransactionForUser() throws Throwable
 
         latch.finishAndWaitForAllToFinish();
 
-        write.closeAndAssertTransactionTermination();
+        write.closeAndAssertExplicitTermination();
 
         assertEmpty( adminSubject, "MATCH (n:Test) RETURN n.name AS name" );
     }
@@ -870,7 +873,7 @@ public void shouldTerminateOnlyGivenUsersTransaction() throws Throwable
 
         latch.finishAndWaitForAllToFinish();
 
-        schema.closeAndAssertTransactionTermination();
+        schema.closeAndAssertExplicitTermination();
         write.closeAndAssertSuccess();
 
         assertSuccess( adminSubject, "MATCH (n:Test) RETURN n.name AS name",
@@ -896,8 +899,8 @@ public void shouldTerminateAllTransactionsForGivenUser() throws Throwable
 
         latch.finishAndWaitForAllToFinish();
 
-        schema1.closeAndAssertTransactionTermination();
+        schema1.closeAndAssertExplicitTermination();
-        schema2.closeAndAssertTransactionTermination();
+        schema2.closeAndAssertExplicitTermination();
 
         assertEmpty( adminSubject, "MATCH (n:Test) RETURN n.name AS name" );
     }
@@ -937,7 +940,7 @@ private void shouldTerminateSelfTransactionsExceptTerminationTransaction( S subj
 
         latch.finishAndWaitForAllToFinish();
 
-        create.closeAndAssertTransactionTermination();
+        create.closeAndAssertExplicitTermination();
 
         assertEmpty( adminSubject, "MATCH (n:Test) RETURN n.name AS name" );
     }

diff --git a/...src/test/java/org/neo4j/server/security/enterprise/auth/ProcedureInteractionTestBase.java b/...src/test/java/org/neo4j/server/security/enterprise/auth/ProcedureInteractionTestBase.java
@@ -477,7 +477,7 @@ void shouldTerminateTransactionsForUser( S subject, String procedure ) throws Th
 
         latch.finishAndWaitForAllToFinish();
 
-        userThread.closeAndAssertTransactionTermination();
+        userThread.closeAndAssertExplicitTermination();
 
         assertEmpty( adminSubject, "MATCH (n:Test) RETURN n.name AS name" );
     }

diff --git a/...security/src/test/java/org/neo4j/server/security/enterprise/auth/ThreadedTransaction.java b/...security/src/test/java/org/neo4j/server/security/enterprise/auth/ThreadedTransaction.java
@@ -24,6 +24,7 @@
 import java.util.concurrent.Future;
 
 import org.neo4j.graphdb.Result;
+import org.neo4j.graphdb.TransactionTerminatedException;
 import org.neo4j.helpers.Exceptions;
 import org.neo4j.kernel.api.KernelTransaction;
 import org.neo4j.kernel.impl.coreapi.InternalTransaction;
@@ -34,6 +35,7 @@
 import static junit.framework.TestCase.fail;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.instanceOf;
 
 class ThreadedTransaction<S>
 {
@@ -136,25 +138,27 @@ void closeAndAssertSuccess() throws Throwable
         }
     }
 
-    void closeAndAssertTransactionTermination() throws Throwable
+    void closeAndAssertExplicitTermination() throws Throwable
     {
         Throwable exceptionInOtherThread = join();
         if ( exceptionInOtherThread == null )
         {
-            fail( "Expected BridgeTransactionTerminatedException in ThreadedCreate, but no exception was raised" );
+            fail( "Expected explicit TransactionTerminatedException in the threaded transaction, " +
+                    "but no exception was raised" );
         }
         assertThat( Exceptions.stringify( exceptionInOtherThread ),
                 exceptionInOtherThread.getMessage(), containsString( "Explicitly terminated by the user.") );
     }
 
-    void closeAndAssertQueryKilled() throws Throwable
+    void closeAndAssertSomeTermination() throws Throwable
     {
         Throwable exceptionInOtherThread = join();
         if ( exceptionInOtherThread == null )
         {
-            fail( "Expected a exception from the query having been killed, but no exception was raised" );
+            fail( "Expected a TransactionTerminatedException in the threaded transaction, but no exception was raised" );
         }
-        assertThat( exceptionInOtherThread.getMessage(), containsString( "The transaction has been terminated.") );
+        assertThat( Exceptions.stringify( exceptionInOtherThread ),
+                exceptionInOtherThread, instanceOf( TransactionTerminatedException.class ) );
     }
 
     private Throwable join() throws ExecutionException, InterruptedException