bereng commented on a change in pull request #1221:
URL: https://github.com/apache/cassandra/pull/1221#discussion_r728648758



##########
File path: test/unit/org/apache/cassandra/schema/SchemaKeyspaceTest.java
##########
@@ -183,8 +269,32 @@ public void testSchemaNoColumn()
                                     KeyspaceParams.simple(1),
                                     SchemaLoader.standardCFMD(testKS, 
testTable));
         // Delete all colmns in the schema
-        String query = String.format("DELETE FROM %s.%s WHERE keyspace_name=? 
and table_name=?", SchemaConstants.SCHEMA_KEYSPACE_NAME, 
SchemaKeyspace.COLUMNS);
+        String query = String.format("DELETE FROM %s.%s WHERE keyspace_name=? 
and table_name=?", SchemaConstants.SCHEMA_KEYSPACE_NAME, 
SystemKeyspaceConstants.COLUMNS);
         executeOnceInternal(query, testKS, testTable);
         SchemaKeyspace.fetchNonSystemKeyspaces();
     }
+
+    private void testMethodSynchronicity(Runnable method) throws Exception
+    {
+        ExecutorService pool = Executors.newFixedThreadPool(2);
+        CountDownLatch pendingStarts = new CountDownLatch(2);
+
+        threadBlock = new CountDownLatch(2);
+        pool.execute(() -> {
+            pendingStarts.countDown();
+            method.run();
+        });
+        pool.execute(() -> {
+            pendingStarts.countDown();
+            method.run();
+        });
+
+        // We have started 2 threads. 1 thread should be held by the latch 
threadBlock, the other should be held as
+        // proof of thread safety.
+        pendingStarts.await(10, TimeUnit.SECONDS);
+        Util.spinAssertEquals(1L, () -> threadBlock.getCount(), 10);
+
+        // Do not execute the tasks. Just kill it.
+        pool.shutdownNow();
+    }

Review comment:
       Mmmm I still disagree sorry, it is not checking _only_ the syncs:
   - Remove the syncs
   - The `assertFalse` on the `blockedThreadFinished` correctly fails 
indicating there was no timeout, meaning the sync was not effective. That's a 
good thing to check imo. This is what you checked iiuc.
   - Next remove that `assertFalse` and run the test again. It fails next line 
bc it found mutations on the `sandbox` ks. It is not checking for `>0` but for 
mutations against the `sandbox` ks. Not any mutation but only on that specific 
ks. That is what you want to check, you don't want to see partially applied 
things.
   
   Does this change your pov? So why have a fuzz test when we have a fully 
deterministic one ready? Sorry not trying to be difficult but I don't want to 
drop a deterministic test if A. I am being an idiot and missing my test is 
broken but I am blind and can't see it B. My test is really not broken if I 
manage to explain how it works.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to