[GitHub] [cassandra] dcapwell commented on a change in pull request #824: CASSANDRA-16143: Streaming fails when s SSTable writer finish() exceeds internode_tcp_user_timeout

[GitBox <gi...@apache.org>]

dcapwell commented on a change in pull request #824:
URL: https://github.com/apache/cassandra/pull/824#discussion_r535524693



##########
File path: test/distributed/org/apache/cassandra/distributed/test/NodeToolTest.java
##########
@@ -67,9 +72,37 @@ public void testCaptureConsoleOutput() throws Throwable
     public void testNodetoolSystemExit()
     {
         // Verify currently calls System.exit, this test uses that knowlege to test System.exit behavior in jvm-dtest
-        CLUSTER.get(1).nodetoolResult("verify", "--check-tokens")
-               .asserts()
-               .failure()
-               .stdoutContains("Token verification requires --extended-verify");
+        NODE.nodetoolResult("verify", "--check-tokens")

Review comment:
       for jvm-dtest I would not agree with this.  `NodeToolResult` is more fleshed out than tool runner offers (as it is deeply integrated with jvm-dtest), and the added ToolRunner wrapper adds more complexity without offering simplification to test authors.
   
   I feel that ToolRunner should be used when calling `main` or calling a CLI on disk, but not when using jvm-dtest nodetool.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org