You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by GitBox <gi...@apache.org> on 2020/10/26 11:05:37 UTC

[GitHub] [cassandra] bereng opened a new pull request #788: CASSANDRA-16227 Add netstats and tablestats unit tests

bereng opened a new pull request #788:
URL: https://github.com/apache/cassandra/pull/788


   


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


[GitHub] [cassandra] adelapena commented on a change in pull request #788: CASSANDRA-16227 Add netstats and tablestats unit tests

Posted by GitBox <gi...@apache.org>.
adelapena commented on a change in pull request #788:
URL: https://github.com/apache/cassandra/pull/788#discussion_r514178743



##########
File path: test/unit/org/apache/cassandra/tools/NodetoolNetStatsTest.java
##########
@@ -0,0 +1,170 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.tools;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.PrintStream;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+import java.util.List;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.apache.cassandra.OrderedJUnit4ClassRunner;
+import org.apache.cassandra.cql3.CQLTester;
+import org.apache.cassandra.locator.InetAddressAndPort;
+import org.apache.cassandra.net.Message;
+import org.apache.cassandra.net.MessagingService;
+import org.apache.cassandra.net.NoPayload;
+import org.apache.cassandra.schema.TableId;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.streaming.SessionInfo;
+import org.apache.cassandra.streaming.StreamSession.State;
+import org.apache.cassandra.streaming.StreamSummary;
+import org.apache.cassandra.tools.ToolRunner.ToolResult;
+import org.apache.cassandra.tools.nodetool.NetStats;
+import org.apache.cassandra.utils.FBUtilities;
+import org.assertj.core.api.Assertions;
+import org.hamcrest.CoreMatchers;
+
+import static org.apache.cassandra.net.Verb.ECHO_REQ;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+@RunWith(OrderedJUnit4ClassRunner.class)
+public class NodetoolNetStatsTest extends CQLTester
+{
+    private static NodeProbe probe;
+
+    @BeforeClass
+    public static void setup() throws Exception
+    {
+        StorageService.instance.initServer();
+        startJMXServer();
+        probe = new NodeProbe(jmxHost, jmxPort);
+    }
+
+    @AfterClass
+    public static void teardown() throws IOException
+    {
+        probe.close();
+    }
+
+    @Test
+    public void testMaybeChangeDocs()
+    {
+        // If you added, modified options or help, please update docs if necessary
+        ToolResult tool = ToolRunner.invokeNodetool("help", "netstats");
+        String help =   "NAME\n" + 
+                        "        nodetool netstats - Print network information on provided host\n" + 
+                        "        (connecting node by default)\n" + 
+                        "\n" + 
+                        "SYNOPSIS\n" + 
+                        "        nodetool [(-h <host> | --host <host>)] [(-p <port> | --port <port>)]\n" + 
+                        "                [(-pp | --print-port)] [(-pw <password> | --password <password>)]\n" + 
+                        "                [(-pwf <passwordFilePath> | --password-file <passwordFilePath>)]\n" + 
+                        "                [(-u <username> | --username <username>)] netstats\n" + 
+                        "                [(-H | --human-readable)]\n" + 
+                        "\n" + 
+                        "OPTIONS\n" + 
+                        "        -h <host>, --host <host>\n" + 
+                        "            Node hostname or ip address\n" + 
+                        "\n" + 
+                        "        -H, --human-readable\n" + 
+                        "            Display bytes in human readable form, i.e. KiB, MiB, GiB, TiB\n" + 
+                        "\n" + 
+                        "        -p <port>, --port <port>\n" + 
+                        "            Remote jmx agent port number\n" + 
+                        "\n" + 
+                        "        -pp, --print-port\n" + 
+                        "            Operate in 4.0 mode with hosts disambiguated by port number\n" + 
+                        "\n" + 
+                        "        -pw <password>, --password <password>\n" + 
+                        "            Remote jmx agent password\n" + 
+                        "\n" + 
+                        "        -pwf <passwordFilePath>, --password-file <passwordFilePath>\n" + 
+                        "            Path to the JMX password file\n" + 
+                        "\n" + 
+                        "        -u <username>, --username <username>\n" + 
+                        "            Remote jmx agent username\n" + 
+                        "\n" + 
+                        "\n";
+        Assertions.assertThat(tool.getStdout()).isEqualTo(help);
+    }
+
+    @Test
+    public void testNetStats()
+    {
+        Message<NoPayload> echoMessageOut = Message.out(ECHO_REQ, NoPayload.noPayload);
+        MessagingService.instance().send(echoMessageOut, FBUtilities.getBroadcastAddressAndPort());
+        
+        ToolResult tool = ToolRunner.invokeNodetool("netstats");
+        assertThat(tool.getStdout(), CoreMatchers.containsString("Gossip messages                 n/a         0              2         0"));
+        assertTrue(tool.getCleanedStderr().isEmpty());
+        assertEquals(0, tool.getExitCode());
+    }
+
+    @Test
+    public void testHumanReadable() throws IOException
+    {
+        List<StreamSummary> streamSummaries = Arrays.asList(new StreamSummary(TableId.generate(), 1, 1024));
+        SessionInfo info = new SessionInfo(InetAddressAndPort.getLocalHost(),
+                                           1,
+                                           InetAddressAndPort.getLocalHost(),
+                                           streamSummaries,
+                                           streamSummaries,
+                                           State.COMPLETE);
+
+        try(ByteArrayOutputStream baos = new ByteArrayOutputStream(); PrintStream out = new PrintStream(baos);)

Review comment:
       ```suggestion
           try (ByteArrayOutputStream baos = new ByteArrayOutputStream(); PrintStream out = new PrintStream(baos))
   ```




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


[GitHub] [cassandra] bereng closed pull request #788: CASSANDRA-16227 Add netstats and tablestats unit tests

Posted by GitBox <gi...@apache.org>.
bereng closed pull request #788:
URL: https://github.com/apache/cassandra/pull/788


   


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


[GitHub] [cassandra] bereng commented on pull request #788: CASSANDRA-16227 Add netstats and tablestats unit tests

Posted by GitBox <gi...@apache.org>.
bereng commented on pull request #788:
URL: https://github.com/apache/cassandra/pull/788#issuecomment-718703452


   CI [j11](https://app.circleci.com/pipelines/github/bereng/cassandra/164/workflows/d973e46b-4cde-410e-9f14-ba47de292297)
   CI [j8](https://app.circleci.com/pipelines/github/bereng/cassandra/164/workflows/d6ce22f1-2d05-4cbb-9f6b-cb56cc0f4ab5)


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


[GitHub] [cassandra] adelapena commented on a change in pull request #788: CASSANDRA-16227 Add netstats and tablestats unit tests

Posted by GitBox <gi...@apache.org>.
adelapena commented on a change in pull request #788:
URL: https://github.com/apache/cassandra/pull/788#discussion_r514232034



##########
File path: test/unit/org/apache/cassandra/tools/nodetool/stats/NodetoolTableStatsTest.java
##########
@@ -0,0 +1,242 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.tools.nodetool.stats;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.commons.lang3.StringUtils;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.apache.cassandra.OrderedJUnit4ClassRunner;
+import org.apache.cassandra.cql3.CQLTester;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.tools.NodeProbe;
+import org.apache.cassandra.tools.ToolRunner;
+import org.apache.cassandra.tools.ToolRunner.ToolResult;
+import org.assertj.core.api.Assertions;
+import org.hamcrest.CoreMatchers;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+@RunWith(OrderedJUnit4ClassRunner.class)
+public class NodetoolTableStatsTest extends CQLTester
+{
+    private static NodeProbe probe;
+
+    @BeforeClass
+    public static void setup() throws Exception
+    {
+        StorageService.instance.initServer();
+        startJMXServer();
+        probe = new NodeProbe(jmxHost, jmxPort);
+    }
+
+    @AfterClass
+    public static void teardown() throws IOException
+    {
+        probe.close();
+    }
+
+    @Test
+    public void testMaybeChangeDocs()
+    {
+        // If you added, modified options or help, please update docs if necessary
+        ToolResult tool = ToolRunner.invokeNodetool("help", "tablestats");
+        String help =   "NAME\n" +
+                        "        nodetool tablestats - Print statistics on tables\n" + 
+                        "\n" + 
+                        "SYNOPSIS\n" + 
+                        "        nodetool [(-h <host> | --host <host>)] [(-p <port> | --port <port>)]\n" + 
+                        "                [(-pp | --print-port)] [(-pw <password> | --password <password>)]\n" + 
+                        "                [(-pwf <passwordFilePath> | --password-file <passwordFilePath>)]\n" + 
+                        "                [(-u <username> | --username <username>)] tablestats\n" + 
+                        "                [(-F <format> | --format <format>)] [(-H | --human-readable)] [-i]\n" + 
+                        "                [(-s <sort_key> | --sort <sort_key>)] [(-t <top> | --top <top>)] [--]\n" + 
+                        "                [<keyspace.table>...]\n" + 
+                        "\n" + 
+                        "OPTIONS\n" + 
+                        "        -F <format>, --format <format>\n" + 
+                        "            Output format (json, yaml)\n" + 
+                        "\n" + 
+                        "        -h <host>, --host <host>\n" + 
+                        "            Node hostname or ip address\n" + 
+                        "\n" + 
+                        "        -H, --human-readable\n" + 
+                        "            Display bytes in human readable form, i.e. KiB, MiB, GiB, TiB\n" + 
+                        "\n" + 
+                        "        -i\n" + 
+                        "            Ignore the list of tables and display the remaining tables\n" + 
+                        "\n" + 
+                        "        -p <port>, --port <port>\n" + 
+                        "            Remote jmx agent port number\n" + 
+                        "\n" + 
+                        "        -pp, --print-port\n" + 
+                        "            Operate in 4.0 mode with hosts disambiguated by port number\n" + 
+                        "\n" + 
+                        "        -pw <password>, --password <password>\n" + 
+                        "            Remote jmx agent password\n" + 
+                        "\n" + 
+                        "        -pwf <passwordFilePath>, --password-file <passwordFilePath>\n" + 
+                        "            Path to the JMX password file\n" + 
+                        "\n" + 
+                        "        -s <sort_key>, --sort <sort_key>\n" + 
+                        "            Sort tables by specified sort key\n" + 
+                        "            (average_live_cells_per_slice_last_five_minutes,\n" + 
+                        "            average_tombstones_per_slice_last_five_minutes,\n" + 
+                        "            bloom_filter_false_positives, bloom_filter_false_ratio,\n" + 
+                        "            bloom_filter_off_heap_memory_used, bloom_filter_space_used,\n" + 
+                        "            compacted_partition_maximum_bytes, compacted_partition_mean_bytes,\n" + 
+                        "            compacted_partition_minimum_bytes,\n" + 
+                        "            compression_metadata_off_heap_memory_used, dropped_mutations,\n" + 
+                        "            full_name, index_summary_off_heap_memory_used, local_read_count,\n" + 
+                        "            local_read_latency_ms, local_write_latency_ms,\n" + 
+                        "            maximum_live_cells_per_slice_last_five_minutes,\n" + 
+                        "            maximum_tombstones_per_slice_last_five_minutes, memtable_cell_count,\n" + 
+                        "            memtable_data_size, memtable_off_heap_memory_used,\n" + 
+                        "            memtable_switch_count, number_of_partitions_estimate,\n" + 
+                        "            off_heap_memory_used_total, pending_flushes, percent_repaired,\n" + 
+                        "            read_latency, reads, space_used_by_snapshots_total, space_used_live,\n" + 
+                        "            space_used_total, sstable_compression_ratio, sstable_count,\n" + 
+                        "            table_name, write_latency, writes)\n" + 
+                        "\n" + 
+                        "        -t <top>, --top <top>\n" + 
+                        "            Show only the top K tables for the sort key (specify the number K of\n" + 
+                        "            tables to be shown\n" + 
+                        "\n" + 
+                        "        -u <username>, --username <username>\n" + 
+                        "            Remote jmx agent username\n" + 
+                        "\n" + 
+                        "        --\n" + 
+                        "            This option can be used to separate command-line options from the\n" + 
+                        "            list of argument, (useful when arguments might be mistaken for\n" + 
+                        "            command-line options\n" + 
+                        "\n" + 
+                        "        [<keyspace.table>...]\n" + 
+                        "            List of tables (or keyspace) names\n" + 
+                        "\n" + 
+                        "\n";
+        Assertions.assertThat(tool.getStdout()).isEqualTo(help);
+    }
+
+    @Test
+    public void testTableStats()
+    {
+        ToolResult tool = ToolRunner.invokeNodetool("tablestats");
+
+        assertThat(tool.getStdout(), CoreMatchers.containsString("Keyspace : system_schema"));
+        assertTrue(StringUtils.countMatches(tool.getStdout(), "Table:") > 1);
+        assertTrue(tool.getCleanedStderr().isEmpty());
+        assertEquals(0, tool.getExitCode());

Review comment:
       ```suggestion
           tool.assertOnCleanExit();
   ```

##########
File path: test/unit/org/apache/cassandra/tools/nodetool/stats/NodetoolTableStatsTest.java
##########
@@ -0,0 +1,242 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.tools.nodetool.stats;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.commons.lang3.StringUtils;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.apache.cassandra.OrderedJUnit4ClassRunner;
+import org.apache.cassandra.cql3.CQLTester;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.tools.NodeProbe;
+import org.apache.cassandra.tools.ToolRunner;
+import org.apache.cassandra.tools.ToolRunner.ToolResult;
+import org.assertj.core.api.Assertions;
+import org.hamcrest.CoreMatchers;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+@RunWith(OrderedJUnit4ClassRunner.class)
+public class NodetoolTableStatsTest extends CQLTester
+{
+    private static NodeProbe probe;
+
+    @BeforeClass
+    public static void setup() throws Exception
+    {
+        StorageService.instance.initServer();
+        startJMXServer();
+        probe = new NodeProbe(jmxHost, jmxPort);
+    }
+
+    @AfterClass
+    public static void teardown() throws IOException
+    {
+        probe.close();
+    }
+
+    @Test
+    public void testMaybeChangeDocs()
+    {
+        // If you added, modified options or help, please update docs if necessary
+        ToolResult tool = ToolRunner.invokeNodetool("help", "tablestats");
+        String help =   "NAME\n" +
+                        "        nodetool tablestats - Print statistics on tables\n" + 
+                        "\n" + 
+                        "SYNOPSIS\n" + 
+                        "        nodetool [(-h <host> | --host <host>)] [(-p <port> | --port <port>)]\n" + 
+                        "                [(-pp | --print-port)] [(-pw <password> | --password <password>)]\n" + 
+                        "                [(-pwf <passwordFilePath> | --password-file <passwordFilePath>)]\n" + 
+                        "                [(-u <username> | --username <username>)] tablestats\n" + 
+                        "                [(-F <format> | --format <format>)] [(-H | --human-readable)] [-i]\n" + 
+                        "                [(-s <sort_key> | --sort <sort_key>)] [(-t <top> | --top <top>)] [--]\n" + 
+                        "                [<keyspace.table>...]\n" + 
+                        "\n" + 
+                        "OPTIONS\n" + 
+                        "        -F <format>, --format <format>\n" + 
+                        "            Output format (json, yaml)\n" + 
+                        "\n" + 
+                        "        -h <host>, --host <host>\n" + 
+                        "            Node hostname or ip address\n" + 
+                        "\n" + 
+                        "        -H, --human-readable\n" + 
+                        "            Display bytes in human readable form, i.e. KiB, MiB, GiB, TiB\n" + 
+                        "\n" + 
+                        "        -i\n" + 
+                        "            Ignore the list of tables and display the remaining tables\n" + 
+                        "\n" + 
+                        "        -p <port>, --port <port>\n" + 
+                        "            Remote jmx agent port number\n" + 
+                        "\n" + 
+                        "        -pp, --print-port\n" + 
+                        "            Operate in 4.0 mode with hosts disambiguated by port number\n" + 
+                        "\n" + 
+                        "        -pw <password>, --password <password>\n" + 
+                        "            Remote jmx agent password\n" + 
+                        "\n" + 
+                        "        -pwf <passwordFilePath>, --password-file <passwordFilePath>\n" + 
+                        "            Path to the JMX password file\n" + 
+                        "\n" + 
+                        "        -s <sort_key>, --sort <sort_key>\n" + 
+                        "            Sort tables by specified sort key\n" + 
+                        "            (average_live_cells_per_slice_last_five_minutes,\n" + 
+                        "            average_tombstones_per_slice_last_five_minutes,\n" + 
+                        "            bloom_filter_false_positives, bloom_filter_false_ratio,\n" + 
+                        "            bloom_filter_off_heap_memory_used, bloom_filter_space_used,\n" + 
+                        "            compacted_partition_maximum_bytes, compacted_partition_mean_bytes,\n" + 
+                        "            compacted_partition_minimum_bytes,\n" + 
+                        "            compression_metadata_off_heap_memory_used, dropped_mutations,\n" + 
+                        "            full_name, index_summary_off_heap_memory_used, local_read_count,\n" + 
+                        "            local_read_latency_ms, local_write_latency_ms,\n" + 
+                        "            maximum_live_cells_per_slice_last_five_minutes,\n" + 
+                        "            maximum_tombstones_per_slice_last_five_minutes, memtable_cell_count,\n" + 
+                        "            memtable_data_size, memtable_off_heap_memory_used,\n" + 
+                        "            memtable_switch_count, number_of_partitions_estimate,\n" + 
+                        "            off_heap_memory_used_total, pending_flushes, percent_repaired,\n" + 
+                        "            read_latency, reads, space_used_by_snapshots_total, space_used_live,\n" + 
+                        "            space_used_total, sstable_compression_ratio, sstable_count,\n" + 
+                        "            table_name, write_latency, writes)\n" + 
+                        "\n" + 
+                        "        -t <top>, --top <top>\n" + 
+                        "            Show only the top K tables for the sort key (specify the number K of\n" + 
+                        "            tables to be shown\n" + 
+                        "\n" + 
+                        "        -u <username>, --username <username>\n" + 
+                        "            Remote jmx agent username\n" + 
+                        "\n" + 
+                        "        --\n" + 
+                        "            This option can be used to separate command-line options from the\n" + 
+                        "            list of argument, (useful when arguments might be mistaken for\n" + 
+                        "            command-line options\n" + 
+                        "\n" + 
+                        "        [<keyspace.table>...]\n" + 
+                        "            List of tables (or keyspace) names\n" + 
+                        "\n" + 
+                        "\n";
+        Assertions.assertThat(tool.getStdout()).isEqualTo(help);
+    }
+
+    @Test
+    public void testTableStats()
+    {
+        ToolResult tool = ToolRunner.invokeNodetool("tablestats");
+
+        assertThat(tool.getStdout(), CoreMatchers.containsString("Keyspace : system_schema"));
+        assertTrue(StringUtils.countMatches(tool.getStdout(), "Table:") > 1);
+        assertTrue(tool.getCleanedStderr().isEmpty());
+        assertEquals(0, tool.getExitCode());
+
+        tool = ToolRunner.invokeNodetool("tablestats", "system_distributed");
+        assertThat(tool.getStdout(), CoreMatchers.containsString("Keyspace : system_distributed"));
+        assertThat(tool.getStdout(), CoreMatchers.not(CoreMatchers.containsString("Keyspace : system_schema")));
+        assertTrue(StringUtils.countMatches(tool.getStdout(), "Table:") > 1);
+        assertTrue(tool.getCleanedStderr().isEmpty());
+        assertEquals(0, tool.getExitCode());

Review comment:
       ```suggestion
           tool.assertOnCleanExit();
   ```




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


[GitHub] [cassandra] bereng commented on pull request #788: CASSANDRA-16227 Add netstats and tablestats unit tests

Posted by GitBox <gi...@apache.org>.
bereng commented on pull request #788:
URL: https://github.com/apache/cassandra/pull/788#issuecomment-718505768


   I have gone through unused imports in `test/unit/org/apache/cassandra/tools/nodetool/stats` if that is what you were referring to. I also tweaked generics usage but [this](https://github.com/bereng/cassandra/blob/CASSANDRA-16227/test/unit/org/apache/cassandra/tools/nodetool/stats/StatsTableComparatorTest.java#L53) guy is giving me a warning despite both the List and the  Comparator are both typed :shrug:. I must be missing sthg...


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


[GitHub] [cassandra] adelapena commented on a change in pull request #788: CASSANDRA-16227 Add netstats and tablestats unit tests

Posted by GitBox <gi...@apache.org>.
adelapena commented on a change in pull request #788:
URL: https://github.com/apache/cassandra/pull/788#discussion_r514230222



##########
File path: test/unit/org/apache/cassandra/tools/NodetoolNetStatsTest.java
##########
@@ -0,0 +1,171 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.tools;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.PrintStream;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.apache.cassandra.OrderedJUnit4ClassRunner;
+import org.apache.cassandra.cql3.CQLTester;
+import org.apache.cassandra.locator.InetAddressAndPort;
+import org.apache.cassandra.net.Message;
+import org.apache.cassandra.net.MessagingService;
+import org.apache.cassandra.net.NoPayload;
+import org.apache.cassandra.schema.TableId;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.streaming.SessionInfo;
+import org.apache.cassandra.streaming.StreamSession.State;
+import org.apache.cassandra.streaming.StreamSummary;
+import org.apache.cassandra.tools.ToolRunner.ToolResult;
+import org.apache.cassandra.tools.nodetool.NetStats;
+import org.apache.cassandra.utils.FBUtilities;
+import org.assertj.core.api.Assertions;
+import org.hamcrest.CoreMatchers;
+
+import static org.apache.cassandra.net.Verb.ECHO_REQ;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+@RunWith(OrderedJUnit4ClassRunner.class)
+public class NodetoolNetStatsTest extends CQLTester
+{
+    private static NodeProbe probe;
+
+    @BeforeClass
+    public static void setup() throws Exception
+    {
+        StorageService.instance.initServer();
+        startJMXServer();
+        probe = new NodeProbe(jmxHost, jmxPort);
+    }
+
+    @AfterClass
+    public static void teardown() throws IOException
+    {
+        probe.close();
+    }
+
+    @Test
+    public void testMaybeChangeDocs()
+    {
+        // If you added, modified options or help, please update docs if necessary
+        ToolResult tool = ToolRunner.invokeNodetool("help", "netstats");
+        String help =   "NAME\n" + 
+                        "        nodetool netstats - Print network information on provided host\n" + 
+                        "        (connecting node by default)\n" + 
+                        "\n" + 
+                        "SYNOPSIS\n" + 
+                        "        nodetool [(-h <host> | --host <host>)] [(-p <port> | --port <port>)]\n" + 
+                        "                [(-pp | --print-port)] [(-pw <password> | --password <password>)]\n" + 
+                        "                [(-pwf <passwordFilePath> | --password-file <passwordFilePath>)]\n" + 
+                        "                [(-u <username> | --username <username>)] netstats\n" + 
+                        "                [(-H | --human-readable)]\n" + 
+                        "\n" + 
+                        "OPTIONS\n" + 
+                        "        -h <host>, --host <host>\n" + 
+                        "            Node hostname or ip address\n" + 
+                        "\n" + 
+                        "        -H, --human-readable\n" + 
+                        "            Display bytes in human readable form, i.e. KiB, MiB, GiB, TiB\n" + 
+                        "\n" + 
+                        "        -p <port>, --port <port>\n" + 
+                        "            Remote jmx agent port number\n" + 
+                        "\n" + 
+                        "        -pp, --print-port\n" + 
+                        "            Operate in 4.0 mode with hosts disambiguated by port number\n" + 
+                        "\n" + 
+                        "        -pw <password>, --password <password>\n" + 
+                        "            Remote jmx agent password\n" + 
+                        "\n" + 
+                        "        -pwf <passwordFilePath>, --password-file <passwordFilePath>\n" + 
+                        "            Path to the JMX password file\n" + 
+                        "\n" + 
+                        "        -u <username>, --username <username>\n" + 
+                        "            Remote jmx agent username\n" + 
+                        "\n" + 
+                        "\n";
+        Assertions.assertThat(tool.getStdout()).isEqualTo(help);
+    }
+
+    @Test
+    public void testNetStats()
+    {
+        Message<NoPayload> echoMessageOut = Message.out(ECHO_REQ, NoPayload.noPayload);
+        MessagingService.instance().send(echoMessageOut, FBUtilities.getBroadcastAddressAndPort());
+        
+        ToolResult tool = ToolRunner.invokeNodetool("netstats");
+        assertThat(tool.getStdout(), CoreMatchers.containsString("Gossip messages                 n/a         0              2         0"));
+        assertTrue(tool.getCleanedStderr().isEmpty());
+        assertEquals(0, tool.getExitCode());

Review comment:
       ```suggestion
           tool.assertOnCleanExit();
   ```




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


[GitHub] [cassandra] adelapena commented on a change in pull request #788: CASSANDRA-16227 Add netstats and tablestats unit tests

Posted by GitBox <gi...@apache.org>.
adelapena commented on a change in pull request #788:
URL: https://github.com/apache/cassandra/pull/788#discussion_r514230611



##########
File path: test/unit/org/apache/cassandra/tools/nodetool/stats/NodetoolTableStatsTest.java
##########
@@ -0,0 +1,242 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.tools.nodetool.stats;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;

Review comment:
       Nit: unused import




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


[GitHub] [cassandra] adelapena commented on a change in pull request #788: CASSANDRA-16227 Add netstats and tablestats unit tests

Posted by GitBox <gi...@apache.org>.
adelapena commented on a change in pull request #788:
URL: https://github.com/apache/cassandra/pull/788#discussion_r514173964



##########
File path: test/unit/org/apache/cassandra/tools/nodetool/stats/NodetoolTableStatsTest.java
##########
@@ -0,0 +1,237 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.tools.nodetool.stats;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.commons.lang3.StringUtils;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.apache.cassandra.OrderedJUnit4ClassRunner;
+import org.apache.cassandra.cql3.CQLTester;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.tools.NodeProbe;
+import org.apache.cassandra.tools.ToolRunner;
+import org.apache.cassandra.tools.ToolRunner.ToolResult;
+import org.assertj.core.api.Assertions;
+import org.hamcrest.CoreMatchers;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+@RunWith(OrderedJUnit4ClassRunner.class)
+public class NodetoolTableStatsTest extends CQLTester
+{
+    private static NodeProbe probe;
+
+    @BeforeClass
+    public static void setup() throws Exception
+    {
+        StorageService.instance.initServer();
+        startJMXServer();
+        probe = new NodeProbe(jmxHost, jmxPort);
+    }
+
+    @AfterClass
+    public static void teardown() throws IOException
+    {
+        probe.close();
+    }
+
+    @Test
+    public void testMaybeChangeDocs()
+    {
+        // If you added, modified options or help, please update docs if necessary
+        ToolResult tool = ToolRunner.invokeNodetool("help", "tablestats");
+        String help =   "NAME\n" +
+                        "        nodetool tablestats - Print statistics on tables\n" + 
+                        "\n" + 
+                        "SYNOPSIS\n" + 
+                        "        nodetool [(-h <host> | --host <host>)] [(-p <port> | --port <port>)]\n" + 
+                        "                [(-pp | --print-port)] [(-pw <password> | --password <password>)]\n" + 
+                        "                [(-pwf <passwordFilePath> | --password-file <passwordFilePath>)]\n" + 
+                        "                [(-u <username> | --username <username>)] tablestats\n" + 
+                        "                [(-F <format> | --format <format>)] [(-H | --human-readable)] [-i]\n" + 
+                        "                [(-s <sort_key> | --sort <sort_key>)] [(-t <top> | --top <top>)] [--]\n" + 
+                        "                [<keyspace.table>...]\n" + 
+                        "\n" + 
+                        "OPTIONS\n" + 
+                        "        -F <format>, --format <format>\n" + 
+                        "            Output format (json, yaml)\n" + 
+                        "\n" + 
+                        "        -h <host>, --host <host>\n" + 
+                        "            Node hostname or ip address\n" + 
+                        "\n" + 
+                        "        -H, --human-readable\n" + 
+                        "            Display bytes in human readable form, i.e. KiB, MiB, GiB, TiB\n" + 
+                        "\n" + 
+                        "        -i\n" + 
+                        "            Ignore the list of tables and display the remaining tables\n" + 
+                        "\n" + 
+                        "        -p <port>, --port <port>\n" + 
+                        "            Remote jmx agent port number\n" + 
+                        "\n" + 
+                        "        -pp, --print-port\n" + 
+                        "            Operate in 4.0 mode with hosts disambiguated by port number\n" + 
+                        "\n" + 
+                        "        -pw <password>, --password <password>\n" + 
+                        "            Remote jmx agent password\n" + 
+                        "\n" + 
+                        "        -pwf <passwordFilePath>, --password-file <passwordFilePath>\n" + 
+                        "            Path to the JMX password file\n" + 
+                        "\n" + 
+                        "        -s <sort_key>, --sort <sort_key>\n" + 
+                        "            Sort tables by specified sort key\n" + 
+                        "            (average_live_cells_per_slice_last_five_minutes,\n" + 
+                        "            average_tombstones_per_slice_last_five_minutes,\n" + 
+                        "            bloom_filter_false_positives, bloom_filter_false_ratio,\n" + 
+                        "            bloom_filter_off_heap_memory_used, bloom_filter_space_used,\n" + 
+                        "            compacted_partition_maximum_bytes, compacted_partition_mean_bytes,\n" + 
+                        "            compacted_partition_minimum_bytes,\n" + 
+                        "            compression_metadata_off_heap_memory_used, dropped_mutations,\n" + 
+                        "            full_name, index_summary_off_heap_memory_used, local_read_count,\n" + 
+                        "            local_read_latency_ms, local_write_latency_ms,\n" + 
+                        "            maximum_live_cells_per_slice_last_five_minutes,\n" + 
+                        "            maximum_tombstones_per_slice_last_five_minutes, memtable_cell_count,\n" + 
+                        "            memtable_data_size, memtable_off_heap_memory_used,\n" + 
+                        "            memtable_switch_count, number_of_partitions_estimate,\n" + 
+                        "            off_heap_memory_used_total, pending_flushes, percent_repaired,\n" + 
+                        "            read_latency, reads, space_used_by_snapshots_total, space_used_live,\n" + 
+                        "            space_used_total, sstable_compression_ratio, sstable_count,\n" + 
+                        "            table_name, write_latency, writes)\n" + 
+                        "\n" + 
+                        "        -t <top>, --top <top>\n" + 
+                        "            Show only the top K tables for the sort key (specify the number K of\n" + 
+                        "            tables to be shown\n" + 
+                        "\n" + 
+                        "        -u <username>, --username <username>\n" + 
+                        "            Remote jmx agent username\n" + 
+                        "\n" + 
+                        "        --\n" + 
+                        "            This option can be used to separate command-line options from the\n" + 
+                        "            list of argument, (useful when arguments might be mistaken for\n" + 
+                        "            command-line options\n" + 
+                        "\n" + 
+                        "        [<keyspace.table>...]\n" + 
+                        "            List of tables (or keyspace) names\n" + 
+                        "\n" + 
+                        "\n";
+        Assertions.assertThat(tool.getStdout()).isEqualTo(help);
+    }
+
+    @Test
+    public void testTableStats()
+    {
+        ToolResult tool = ToolRunner.invokeNodetool("tablestats");
+
+        assertThat(tool.getStdout(), CoreMatchers.containsStringIgnoringCase("Keyspace : system_schema"));
+        assertTrue(StringUtils.countMatches(tool.getStdout(), "Table:") > 1);
+        assertTrue(tool.getCleanedStderr().isEmpty());
+        assertEquals(0, tool.getExitCode());
+
+        tool = ToolRunner.invokeNodetool("tablestats", "system_distributed");
+        assertThat(tool.getStdout(), CoreMatchers.containsStringIgnoringCase("Keyspace : system_distributed"));
+        assertThat(tool.getStdout(), CoreMatchers.not(CoreMatchers.containsStringIgnoringCase("Keyspace : system_schema")));
+        assertTrue(StringUtils.countMatches(tool.getStdout(), "Table:") > 1);
+        assertTrue(tool.getCleanedStderr().isEmpty());
+        assertEquals(0, tool.getExitCode());
+    }
+
+    @Test
+    public void testTableIgnoreArg()
+    {
+        ToolResult tool = ToolRunner.invokeNodetool("tablestats", "-i", "system_schema.aggregates");
+
+        assertThat(tool.getStdout(), CoreMatchers.containsStringIgnoringCase("Keyspace : system_schema"));
+        assertThat(tool.getStdout(), CoreMatchers.not(CoreMatchers.containsStringIgnoringCase("Table: system_schema.aggregates")));
+        assertTrue(StringUtils.countMatches(tool.getStdout(), "Table:") > 1);
+        assertTrue(tool.getCleanedStderr().isEmpty());
+        assertEquals(0, tool.getExitCode());
+    }
+
+    @Test
+    public void testHumanReadableArg()
+    {
+        Arrays.asList("-H", "--human-readable").forEach(arg -> {
+            ToolResult tool = ToolRunner.invokeNodetool("tablestats", arg);
+            assertThat("Arg: [" + arg + "]", tool.getStdout(), CoreMatchers.containsStringIgnoringCase(" KiB"));
+            assertTrue("Arg: [" + arg + "]", tool.getCleanedStderr().isEmpty());
+            assertEquals("Arg: [" + arg + "]", 0, tool.getExitCode());

Review comment:
       Ok, I didn't understand that this is only used when there are multiple ways to write the option, it looks good to me as it is. By the way, what I was thinking for the messages was something like:
   ```java
   Arrays.asList("-H", "--human-readable").forEach(option -> {
       ToolResult tool = ToolRunner.invokeNodetool("tablestats", option);
       assertThat(String.format("Unmatched pattern in stdout for option [%s], found: %s", option, tool.getStdout()),
                  tool.getStdout(), CoreMatchers.containsString(" KiB"));
       assertTrue(String.format("Expected empty stderr for option [%s] but found: %s", option, tool.getCleanedStderr()),
                  tool.getCleanedStderr().isEmpty());
       assertEquals(String.format("Expected exit code 0 for option [%s] but found: %s", option, tool.getExitCode()),
                    0, tool.getExitCode());
   });
   ```




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


[GitHub] [cassandra] adelapena commented on a change in pull request #788: CASSANDRA-16227 Add netstats and tablestats unit tests

Posted by GitBox <gi...@apache.org>.
adelapena commented on a change in pull request #788:
URL: https://github.com/apache/cassandra/pull/788#discussion_r513426212



##########
File path: test/unit/org/apache/cassandra/tools/NodetoolNetStatsTest.java
##########
@@ -0,0 +1,116 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.tools;
+
+import java.io.IOException;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.apache.cassandra.OrderedJUnit4ClassRunner;
+import org.apache.cassandra.cql3.CQLTester;
+import org.apache.cassandra.net.Message;
+import org.apache.cassandra.net.MessagingService;
+import org.apache.cassandra.net.NoPayload;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.tools.ToolRunner.ToolResult;
+import org.apache.cassandra.utils.FBUtilities;
+import org.assertj.core.api.Assertions;
+import org.hamcrest.CoreMatchers;
+
+import static org.apache.cassandra.net.Verb.ECHO_REQ;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+@RunWith(OrderedJUnit4ClassRunner.class)
+public class NodetoolNetStatsTest extends CQLTester

Review comment:
       We could add a test for the `-H, --human-readable` option, like the one in `NodetoolTableStatsTest#testHumanReadableArg`.

##########
File path: test/unit/org/apache/cassandra/tools/nodetool/stats/NodetoolTableStatsTest.java
##########
@@ -0,0 +1,237 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.tools.nodetool.stats;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.commons.lang3.StringUtils;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.apache.cassandra.OrderedJUnit4ClassRunner;
+import org.apache.cassandra.cql3.CQLTester;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.tools.NodeProbe;
+import org.apache.cassandra.tools.ToolRunner;
+import org.apache.cassandra.tools.ToolRunner.ToolResult;
+import org.assertj.core.api.Assertions;
+import org.hamcrest.CoreMatchers;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+@RunWith(OrderedJUnit4ClassRunner.class)
+public class NodetoolTableStatsTest extends CQLTester
+{
+    private static NodeProbe probe;
+
+    @BeforeClass
+    public static void setup() throws Exception
+    {
+        StorageService.instance.initServer();
+        startJMXServer();
+        probe = new NodeProbe(jmxHost, jmxPort);
+    }
+
+    @AfterClass
+    public static void teardown() throws IOException
+    {
+        probe.close();
+    }
+
+    @Test
+    public void testMaybeChangeDocs()
+    {
+        // If you added, modified options or help, please update docs if necessary
+        ToolResult tool = ToolRunner.invokeNodetool("help", "tablestats");
+        String help =   "NAME\n" +
+                        "        nodetool tablestats - Print statistics on tables\n" + 
+                        "\n" + 
+                        "SYNOPSIS\n" + 
+                        "        nodetool [(-h <host> | --host <host>)] [(-p <port> | --port <port>)]\n" + 
+                        "                [(-pp | --print-port)] [(-pw <password> | --password <password>)]\n" + 
+                        "                [(-pwf <passwordFilePath> | --password-file <passwordFilePath>)]\n" + 
+                        "                [(-u <username> | --username <username>)] tablestats\n" + 
+                        "                [(-F <format> | --format <format>)] [(-H | --human-readable)] [-i]\n" + 
+                        "                [(-s <sort_key> | --sort <sort_key>)] [(-t <top> | --top <top>)] [--]\n" + 
+                        "                [<keyspace.table>...]\n" + 
+                        "\n" + 
+                        "OPTIONS\n" + 
+                        "        -F <format>, --format <format>\n" + 
+                        "            Output format (json, yaml)\n" + 
+                        "\n" + 
+                        "        -h <host>, --host <host>\n" + 
+                        "            Node hostname or ip address\n" + 
+                        "\n" + 
+                        "        -H, --human-readable\n" + 
+                        "            Display bytes in human readable form, i.e. KiB, MiB, GiB, TiB\n" + 
+                        "\n" + 
+                        "        -i\n" + 
+                        "            Ignore the list of tables and display the remaining tables\n" + 
+                        "\n" + 
+                        "        -p <port>, --port <port>\n" + 
+                        "            Remote jmx agent port number\n" + 
+                        "\n" + 
+                        "        -pp, --print-port\n" + 
+                        "            Operate in 4.0 mode with hosts disambiguated by port number\n" + 
+                        "\n" + 
+                        "        -pw <password>, --password <password>\n" + 
+                        "            Remote jmx agent password\n" + 
+                        "\n" + 
+                        "        -pwf <passwordFilePath>, --password-file <passwordFilePath>\n" + 
+                        "            Path to the JMX password file\n" + 
+                        "\n" + 
+                        "        -s <sort_key>, --sort <sort_key>\n" + 
+                        "            Sort tables by specified sort key\n" + 
+                        "            (average_live_cells_per_slice_last_five_minutes,\n" + 
+                        "            average_tombstones_per_slice_last_five_minutes,\n" + 
+                        "            bloom_filter_false_positives, bloom_filter_false_ratio,\n" + 
+                        "            bloom_filter_off_heap_memory_used, bloom_filter_space_used,\n" + 
+                        "            compacted_partition_maximum_bytes, compacted_partition_mean_bytes,\n" + 
+                        "            compacted_partition_minimum_bytes,\n" + 
+                        "            compression_metadata_off_heap_memory_used, dropped_mutations,\n" + 
+                        "            full_name, index_summary_off_heap_memory_used, local_read_count,\n" + 
+                        "            local_read_latency_ms, local_write_latency_ms,\n" + 
+                        "            maximum_live_cells_per_slice_last_five_minutes,\n" + 
+                        "            maximum_tombstones_per_slice_last_five_minutes, memtable_cell_count,\n" + 
+                        "            memtable_data_size, memtable_off_heap_memory_used,\n" + 
+                        "            memtable_switch_count, number_of_partitions_estimate,\n" + 
+                        "            off_heap_memory_used_total, pending_flushes, percent_repaired,\n" + 
+                        "            read_latency, reads, space_used_by_snapshots_total, space_used_live,\n" + 
+                        "            space_used_total, sstable_compression_ratio, sstable_count,\n" + 
+                        "            table_name, write_latency, writes)\n" + 
+                        "\n" + 
+                        "        -t <top>, --top <top>\n" + 
+                        "            Show only the top K tables for the sort key (specify the number K of\n" + 
+                        "            tables to be shown\n" + 
+                        "\n" + 
+                        "        -u <username>, --username <username>\n" + 
+                        "            Remote jmx agent username\n" + 
+                        "\n" + 
+                        "        --\n" + 
+                        "            This option can be used to separate command-line options from the\n" + 
+                        "            list of argument, (useful when arguments might be mistaken for\n" + 
+                        "            command-line options\n" + 
+                        "\n" + 
+                        "        [<keyspace.table>...]\n" + 
+                        "            List of tables (or keyspace) names\n" + 
+                        "\n" + 
+                        "\n";
+        Assertions.assertThat(tool.getStdout()).isEqualTo(help);
+    }
+
+    @Test
+    public void testTableStats()
+    {
+        ToolResult tool = ToolRunner.invokeNodetool("tablestats");
+
+        assertThat(tool.getStdout(), CoreMatchers.containsStringIgnoringCase("Keyspace : system_schema"));
+        assertTrue(StringUtils.countMatches(tool.getStdout(), "Table:") > 1);
+        assertTrue(tool.getCleanedStderr().isEmpty());
+        assertEquals(0, tool.getExitCode());
+
+        tool = ToolRunner.invokeNodetool("tablestats", "system_distributed");
+        assertThat(tool.getStdout(), CoreMatchers.containsStringIgnoringCase("Keyspace : system_distributed"));
+        assertThat(tool.getStdout(), CoreMatchers.not(CoreMatchers.containsStringIgnoringCase("Keyspace : system_schema")));
+        assertTrue(StringUtils.countMatches(tool.getStdout(), "Table:") > 1);
+        assertTrue(tool.getCleanedStderr().isEmpty());
+        assertEquals(0, tool.getExitCode());
+    }

Review comment:
       It seems that the pattern run tool - check stdout - check stderr - check exit code is repeated for most tests. Not saying that we should do it given that the tests are simpler enough, but we could use a common utility method like:
   ```suggestion
       private static void assertToolSuccess(Consumer<String> stdoutCheck, String... args)
       {
           ToolResult tool = ToolRunner.invokeNodetool(args);
           stdoutCheck.accept(tool.getStdout());
           assertTrue(tool.getCleanedStderr().isEmpty());
           assertEquals(0, tool.getExitCode());
       }
   
       @Test
       public void testTableStats()
       {
           assertToolSuccess(stdout -> {
               assertTrue(stdout.contains("Keyspace : system_schema"));
               assertTrue(StringUtils.countMatches(stdout, "Table:") > 1);
           }, "tablestats");
   
           assertToolSuccess(stdout -> {
               assertTrue(stdout.contains("Keyspace : system_distributed"));
               assertFalse(stdout.contains("Keyspace : system_schema"));
               assertTrue(StringUtils.countMatches(stdout, "Table:") > 1);
           }, "tablestats", "system_distributed");
       }
   ```
   That's just a suggestion, feel free to ignore.

##########
File path: test/unit/org/apache/cassandra/tools/NodetoolNetStatsTest.java
##########
@@ -0,0 +1,116 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.tools;
+
+import java.io.IOException;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.apache.cassandra.OrderedJUnit4ClassRunner;
+import org.apache.cassandra.cql3.CQLTester;
+import org.apache.cassandra.net.Message;
+import org.apache.cassandra.net.MessagingService;
+import org.apache.cassandra.net.NoPayload;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.tools.ToolRunner.ToolResult;
+import org.apache.cassandra.utils.FBUtilities;
+import org.assertj.core.api.Assertions;
+import org.hamcrest.CoreMatchers;
+
+import static org.apache.cassandra.net.Verb.ECHO_REQ;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+@RunWith(OrderedJUnit4ClassRunner.class)
+public class NodetoolNetStatsTest extends CQLTester
+{
+    private static NodeProbe probe;
+
+    @BeforeClass
+    public static void setup() throws Exception
+    {
+        StorageService.instance.initServer();
+        startJMXServer();
+        probe = new NodeProbe(jmxHost, jmxPort);
+    }
+
+    @AfterClass
+    public static void teardown() throws IOException
+    {
+        probe.close();
+    }
+
+    @Test
+    public void testMaybeChangeDocs()
+    {
+        // If you added, modified options or help, please update docs if necessary
+        ToolResult tool = ToolRunner.invokeNodetool("help", "netstats");
+        String help =   "NAME\n" + 
+                        "        nodetool netstats - Print network information on provided host\n" + 
+                        "        (connecting node by default)\n" + 
+                        "\n" + 
+                        "SYNOPSIS\n" + 
+                        "        nodetool [(-h <host> | --host <host>)] [(-p <port> | --port <port>)]\n" + 
+                        "                [(-pp | --print-port)] [(-pw <password> | --password <password>)]\n" + 
+                        "                [(-pwf <passwordFilePath> | --password-file <passwordFilePath>)]\n" + 
+                        "                [(-u <username> | --username <username>)] netstats\n" + 
+                        "                [(-H | --human-readable)]\n" + 
+                        "\n" + 
+                        "OPTIONS\n" + 
+                        "        -h <host>, --host <host>\n" + 
+                        "            Node hostname or ip address\n" + 
+                        "\n" + 
+                        "        -H, --human-readable\n" + 
+                        "            Display bytes in human readable form, i.e. KiB, MiB, GiB, TiB\n" + 
+                        "\n" + 
+                        "        -p <port>, --port <port>\n" + 
+                        "            Remote jmx agent port number\n" + 
+                        "\n" + 
+                        "        -pp, --print-port\n" + 
+                        "            Operate in 4.0 mode with hosts disambiguated by port number\n" + 
+                        "\n" + 
+                        "        -pw <password>, --password <password>\n" + 
+                        "            Remote jmx agent password\n" + 
+                        "\n" + 
+                        "        -pwf <passwordFilePath>, --password-file <passwordFilePath>\n" + 
+                        "            Path to the JMX password file\n" + 
+                        "\n" + 
+                        "        -u <username>, --username <username>\n" + 
+                        "            Remote jmx agent username\n" + 
+                        "\n" + 
+                        "\n";
+        Assertions.assertThat(tool.getStdout()).isEqualTo(help);
+    }
+
+    @Test
+    public void testNetStats() throws Throwable
+    {
+        Message<NoPayload> echoMessageOut = Message.out(ECHO_REQ, NoPayload.noPayload);
+        MessagingService.instance().send(echoMessageOut, FBUtilities.getBroadcastAddressAndPort());
+        
+        ToolResult tool = ToolRunner.invokeNodetool("netstats");
+        assertThat(tool.getStdout(), CoreMatchers.containsStringIgnoringCase("Gossip messages                 n/a         0              2         0"));

Review comment:
       We don't need `containsStringIgnoringCase`, the case seems correct so we can use either `CoreCatchers.contrainsString` or the simpler `tool.getStdout().contains()`. There are eight calls to `containsStringIgnoringCase` in `NodetoolTableStatsTest` in the same situation; none of them requires to ignore the letter case.

##########
File path: test/unit/org/apache/cassandra/tools/NodetoolNetStatsTest.java
##########
@@ -0,0 +1,116 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.tools;
+
+import java.io.IOException;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.apache.cassandra.OrderedJUnit4ClassRunner;
+import org.apache.cassandra.cql3.CQLTester;
+import org.apache.cassandra.net.Message;
+import org.apache.cassandra.net.MessagingService;
+import org.apache.cassandra.net.NoPayload;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.tools.ToolRunner.ToolResult;
+import org.apache.cassandra.utils.FBUtilities;
+import org.assertj.core.api.Assertions;
+import org.hamcrest.CoreMatchers;
+
+import static org.apache.cassandra.net.Verb.ECHO_REQ;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+@RunWith(OrderedJUnit4ClassRunner.class)
+public class NodetoolNetStatsTest extends CQLTester
+{
+    private static NodeProbe probe;
+
+    @BeforeClass
+    public static void setup() throws Exception
+    {
+        StorageService.instance.initServer();
+        startJMXServer();
+        probe = new NodeProbe(jmxHost, jmxPort);
+    }
+
+    @AfterClass
+    public static void teardown() throws IOException
+    {
+        probe.close();
+    }
+
+    @Test
+    public void testMaybeChangeDocs()
+    {
+        // If you added, modified options or help, please update docs if necessary
+        ToolResult tool = ToolRunner.invokeNodetool("help", "netstats");
+        String help =   "NAME\n" + 
+                        "        nodetool netstats - Print network information on provided host\n" + 
+                        "        (connecting node by default)\n" + 
+                        "\n" + 
+                        "SYNOPSIS\n" + 
+                        "        nodetool [(-h <host> | --host <host>)] [(-p <port> | --port <port>)]\n" + 
+                        "                [(-pp | --print-port)] [(-pw <password> | --password <password>)]\n" + 
+                        "                [(-pwf <passwordFilePath> | --password-file <passwordFilePath>)]\n" + 
+                        "                [(-u <username> | --username <username>)] netstats\n" + 
+                        "                [(-H | --human-readable)]\n" + 
+                        "\n" + 
+                        "OPTIONS\n" + 
+                        "        -h <host>, --host <host>\n" + 
+                        "            Node hostname or ip address\n" + 
+                        "\n" + 
+                        "        -H, --human-readable\n" + 
+                        "            Display bytes in human readable form, i.e. KiB, MiB, GiB, TiB\n" + 
+                        "\n" + 
+                        "        -p <port>, --port <port>\n" + 
+                        "            Remote jmx agent port number\n" + 
+                        "\n" + 
+                        "        -pp, --print-port\n" + 
+                        "            Operate in 4.0 mode with hosts disambiguated by port number\n" + 
+                        "\n" + 
+                        "        -pw <password>, --password <password>\n" + 
+                        "            Remote jmx agent password\n" + 
+                        "\n" + 
+                        "        -pwf <passwordFilePath>, --password-file <passwordFilePath>\n" + 
+                        "            Path to the JMX password file\n" + 
+                        "\n" + 
+                        "        -u <username>, --username <username>\n" + 
+                        "            Remote jmx agent username\n" + 
+                        "\n" + 
+                        "\n";
+        Assertions.assertThat(tool.getStdout()).isEqualTo(help);
+    }
+
+    @Test
+    public void testNetStats() throws Throwable

Review comment:
       Nit: the {{throws Throwable}} is not needed

##########
File path: test/unit/org/apache/cassandra/tools/nodetool/stats/NodetoolTableStatsTest.java
##########
@@ -0,0 +1,237 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.tools.nodetool.stats;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.commons.lang3.StringUtils;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.apache.cassandra.OrderedJUnit4ClassRunner;
+import org.apache.cassandra.cql3.CQLTester;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.tools.NodeProbe;
+import org.apache.cassandra.tools.ToolRunner;
+import org.apache.cassandra.tools.ToolRunner.ToolResult;
+import org.assertj.core.api.Assertions;
+import org.hamcrest.CoreMatchers;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+@RunWith(OrderedJUnit4ClassRunner.class)
+public class NodetoolTableStatsTest extends CQLTester
+{
+    private static NodeProbe probe;
+
+    @BeforeClass
+    public static void setup() throws Exception
+    {
+        StorageService.instance.initServer();
+        startJMXServer();
+        probe = new NodeProbe(jmxHost, jmxPort);
+    }
+
+    @AfterClass
+    public static void teardown() throws IOException
+    {
+        probe.close();
+    }
+
+    @Test
+    public void testMaybeChangeDocs()
+    {
+        // If you added, modified options or help, please update docs if necessary
+        ToolResult tool = ToolRunner.invokeNodetool("help", "tablestats");
+        String help =   "NAME\n" +
+                        "        nodetool tablestats - Print statistics on tables\n" + 
+                        "\n" + 
+                        "SYNOPSIS\n" + 
+                        "        nodetool [(-h <host> | --host <host>)] [(-p <port> | --port <port>)]\n" + 
+                        "                [(-pp | --print-port)] [(-pw <password> | --password <password>)]\n" + 
+                        "                [(-pwf <passwordFilePath> | --password-file <passwordFilePath>)]\n" + 
+                        "                [(-u <username> | --username <username>)] tablestats\n" + 
+                        "                [(-F <format> | --format <format>)] [(-H | --human-readable)] [-i]\n" + 
+                        "                [(-s <sort_key> | --sort <sort_key>)] [(-t <top> | --top <top>)] [--]\n" + 
+                        "                [<keyspace.table>...]\n" + 
+                        "\n" + 
+                        "OPTIONS\n" + 
+                        "        -F <format>, --format <format>\n" + 
+                        "            Output format (json, yaml)\n" + 
+                        "\n" + 
+                        "        -h <host>, --host <host>\n" + 
+                        "            Node hostname or ip address\n" + 
+                        "\n" + 
+                        "        -H, --human-readable\n" + 
+                        "            Display bytes in human readable form, i.e. KiB, MiB, GiB, TiB\n" + 
+                        "\n" + 
+                        "        -i\n" + 
+                        "            Ignore the list of tables and display the remaining tables\n" + 
+                        "\n" + 
+                        "        -p <port>, --port <port>\n" + 
+                        "            Remote jmx agent port number\n" + 
+                        "\n" + 
+                        "        -pp, --print-port\n" + 
+                        "            Operate in 4.0 mode with hosts disambiguated by port number\n" + 
+                        "\n" + 
+                        "        -pw <password>, --password <password>\n" + 
+                        "            Remote jmx agent password\n" + 
+                        "\n" + 
+                        "        -pwf <passwordFilePath>, --password-file <passwordFilePath>\n" + 
+                        "            Path to the JMX password file\n" + 
+                        "\n" + 
+                        "        -s <sort_key>, --sort <sort_key>\n" + 
+                        "            Sort tables by specified sort key\n" + 
+                        "            (average_live_cells_per_slice_last_five_minutes,\n" + 
+                        "            average_tombstones_per_slice_last_five_minutes,\n" + 
+                        "            bloom_filter_false_positives, bloom_filter_false_ratio,\n" + 
+                        "            bloom_filter_off_heap_memory_used, bloom_filter_space_used,\n" + 
+                        "            compacted_partition_maximum_bytes, compacted_partition_mean_bytes,\n" + 
+                        "            compacted_partition_minimum_bytes,\n" + 
+                        "            compression_metadata_off_heap_memory_used, dropped_mutations,\n" + 
+                        "            full_name, index_summary_off_heap_memory_used, local_read_count,\n" + 
+                        "            local_read_latency_ms, local_write_latency_ms,\n" + 
+                        "            maximum_live_cells_per_slice_last_five_minutes,\n" + 
+                        "            maximum_tombstones_per_slice_last_five_minutes, memtable_cell_count,\n" + 
+                        "            memtable_data_size, memtable_off_heap_memory_used,\n" + 
+                        "            memtable_switch_count, number_of_partitions_estimate,\n" + 
+                        "            off_heap_memory_used_total, pending_flushes, percent_repaired,\n" + 
+                        "            read_latency, reads, space_used_by_snapshots_total, space_used_live,\n" + 
+                        "            space_used_total, sstable_compression_ratio, sstable_count,\n" + 
+                        "            table_name, write_latency, writes)\n" + 
+                        "\n" + 
+                        "        -t <top>, --top <top>\n" + 
+                        "            Show only the top K tables for the sort key (specify the number K of\n" + 
+                        "            tables to be shown\n" + 
+                        "\n" + 
+                        "        -u <username>, --username <username>\n" + 
+                        "            Remote jmx agent username\n" + 
+                        "\n" + 
+                        "        --\n" + 
+                        "            This option can be used to separate command-line options from the\n" + 
+                        "            list of argument, (useful when arguments might be mistaken for\n" + 
+                        "            command-line options\n" + 
+                        "\n" + 
+                        "        [<keyspace.table>...]\n" + 
+                        "            List of tables (or keyspace) names\n" + 
+                        "\n" + 
+                        "\n";
+        Assertions.assertThat(tool.getStdout()).isEqualTo(help);
+    }
+
+    @Test
+    public void testTableStats()
+    {
+        ToolResult tool = ToolRunner.invokeNodetool("tablestats");
+
+        assertThat(tool.getStdout(), CoreMatchers.containsStringIgnoringCase("Keyspace : system_schema"));
+        assertTrue(StringUtils.countMatches(tool.getStdout(), "Table:") > 1);
+        assertTrue(tool.getCleanedStderr().isEmpty());
+        assertEquals(0, tool.getExitCode());
+
+        tool = ToolRunner.invokeNodetool("tablestats", "system_distributed");
+        assertThat(tool.getStdout(), CoreMatchers.containsStringIgnoringCase("Keyspace : system_distributed"));
+        assertThat(tool.getStdout(), CoreMatchers.not(CoreMatchers.containsStringIgnoringCase("Keyspace : system_schema")));
+        assertTrue(StringUtils.countMatches(tool.getStdout(), "Table:") > 1);
+        assertTrue(tool.getCleanedStderr().isEmpty());
+        assertEquals(0, tool.getExitCode());
+    }
+
+    @Test
+    public void testTableIgnoreArg()
+    {
+        ToolResult tool = ToolRunner.invokeNodetool("tablestats", "-i", "system_schema.aggregates");
+
+        assertThat(tool.getStdout(), CoreMatchers.containsStringIgnoringCase("Keyspace : system_schema"));
+        assertThat(tool.getStdout(), CoreMatchers.not(CoreMatchers.containsStringIgnoringCase("Table: system_schema.aggregates")));
+        assertTrue(StringUtils.countMatches(tool.getStdout(), "Table:") > 1);
+        assertTrue(tool.getCleanedStderr().isEmpty());
+        assertEquals(0, tool.getExitCode());
+    }
+
+    @Test
+    public void testHumanReadableArg()
+    {
+        Arrays.asList("-H", "--human-readable").forEach(arg -> {
+            ToolResult tool = ToolRunner.invokeNodetool("tablestats", arg);
+            assertThat("Arg: [" + arg + "]", tool.getStdout(), CoreMatchers.containsStringIgnoringCase(" KiB"));
+            assertTrue("Arg: [" + arg + "]", tool.getCleanedStderr().isEmpty());
+            assertEquals("Arg: [" + arg + "]", 0, tool.getExitCode());
+        });
+    }
+
+    @Test
+    public void testSortArg()
+    {
+        Pattern regExp = Pattern.compile("((?m)Table: .*$)");
+
+        Arrays.asList("-s", "--sort").forEach(arg -> {
+            ToolResult tool = ToolRunner.invokeNodetool("tablestats", arg, "table_name");
+            Matcher m = regExp.matcher(tool.getStdout());
+            ArrayList<String> orig = new ArrayList<>();
+            while (m.find())
+                orig.add(m.group(1));
+
+            tool = ToolRunner.invokeNodetool("tablestats", arg, "sstable_count");
+            m = regExp.matcher(tool.getStdout());
+            ArrayList<String> sorted = new ArrayList<>();
+            while (m.find())
+                sorted.add(m.group(1));
+
+            assertNotEquals("Arg: [" + arg + "]", orig, sorted);
+            Collections.sort(orig);
+            Collections.sort(sorted);
+            assertEquals("Arg: [" + arg + "]", orig, sorted);
+            assertTrue("Arg: [" + arg + "]", tool.getCleanedStderr().isEmpty());
+            assertEquals(0, tool.getExitCode());
+        });
+
+        ToolResult tool = ToolRunner.invokeNodetool("tablestats", "-s", "wrongSort");
+        assertThat(tool.getStdout(), CoreMatchers.containsStringIgnoringCase("argument for sort must be one of"));
+        assertTrue(tool.getCleanedStderr().isEmpty());
+        assertEquals(1, tool.getExitCode());
+    }
+
+    @Test
+    public void testTopArg()
+    {
+        Arrays.asList("-t", "--top").forEach(arg -> {
+            ToolResult tool = ToolRunner.invokeNodetool("tablestats", "-s", "table_name", arg, "1");
+            assertTrue("Arg: [" + arg + "]", StringUtils.countMatches(tool.getStdout(), "Table:") == 1);

Review comment:
       We could use `assertEquals` here.

##########
File path: test/unit/org/apache/cassandra/tools/nodetool/stats/NodetoolTableStatsTest.java
##########
@@ -0,0 +1,237 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.tools.nodetool.stats;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.commons.lang3.StringUtils;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.apache.cassandra.OrderedJUnit4ClassRunner;
+import org.apache.cassandra.cql3.CQLTester;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.tools.NodeProbe;
+import org.apache.cassandra.tools.ToolRunner;
+import org.apache.cassandra.tools.ToolRunner.ToolResult;
+import org.assertj.core.api.Assertions;
+import org.hamcrest.CoreMatchers;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+@RunWith(OrderedJUnit4ClassRunner.class)
+public class NodetoolTableStatsTest extends CQLTester
+{
+    private static NodeProbe probe;
+
+    @BeforeClass
+    public static void setup() throws Exception
+    {
+        StorageService.instance.initServer();
+        startJMXServer();
+        probe = new NodeProbe(jmxHost, jmxPort);
+    }
+
+    @AfterClass
+    public static void teardown() throws IOException
+    {
+        probe.close();
+    }
+
+    @Test
+    public void testMaybeChangeDocs()
+    {
+        // If you added, modified options or help, please update docs if necessary
+        ToolResult tool = ToolRunner.invokeNodetool("help", "tablestats");
+        String help =   "NAME\n" +
+                        "        nodetool tablestats - Print statistics on tables\n" + 
+                        "\n" + 
+                        "SYNOPSIS\n" + 
+                        "        nodetool [(-h <host> | --host <host>)] [(-p <port> | --port <port>)]\n" + 
+                        "                [(-pp | --print-port)] [(-pw <password> | --password <password>)]\n" + 
+                        "                [(-pwf <passwordFilePath> | --password-file <passwordFilePath>)]\n" + 
+                        "                [(-u <username> | --username <username>)] tablestats\n" + 
+                        "                [(-F <format> | --format <format>)] [(-H | --human-readable)] [-i]\n" + 
+                        "                [(-s <sort_key> | --sort <sort_key>)] [(-t <top> | --top <top>)] [--]\n" + 
+                        "                [<keyspace.table>...]\n" + 
+                        "\n" + 
+                        "OPTIONS\n" + 
+                        "        -F <format>, --format <format>\n" + 
+                        "            Output format (json, yaml)\n" + 
+                        "\n" + 
+                        "        -h <host>, --host <host>\n" + 
+                        "            Node hostname or ip address\n" + 
+                        "\n" + 
+                        "        -H, --human-readable\n" + 
+                        "            Display bytes in human readable form, i.e. KiB, MiB, GiB, TiB\n" + 
+                        "\n" + 
+                        "        -i\n" + 
+                        "            Ignore the list of tables and display the remaining tables\n" + 
+                        "\n" + 
+                        "        -p <port>, --port <port>\n" + 
+                        "            Remote jmx agent port number\n" + 
+                        "\n" + 
+                        "        -pp, --print-port\n" + 
+                        "            Operate in 4.0 mode with hosts disambiguated by port number\n" + 
+                        "\n" + 
+                        "        -pw <password>, --password <password>\n" + 
+                        "            Remote jmx agent password\n" + 
+                        "\n" + 
+                        "        -pwf <passwordFilePath>, --password-file <passwordFilePath>\n" + 
+                        "            Path to the JMX password file\n" + 
+                        "\n" + 
+                        "        -s <sort_key>, --sort <sort_key>\n" + 
+                        "            Sort tables by specified sort key\n" + 
+                        "            (average_live_cells_per_slice_last_five_minutes,\n" + 
+                        "            average_tombstones_per_slice_last_five_minutes,\n" + 
+                        "            bloom_filter_false_positives, bloom_filter_false_ratio,\n" + 
+                        "            bloom_filter_off_heap_memory_used, bloom_filter_space_used,\n" + 
+                        "            compacted_partition_maximum_bytes, compacted_partition_mean_bytes,\n" + 
+                        "            compacted_partition_minimum_bytes,\n" + 
+                        "            compression_metadata_off_heap_memory_used, dropped_mutations,\n" + 
+                        "            full_name, index_summary_off_heap_memory_used, local_read_count,\n" + 
+                        "            local_read_latency_ms, local_write_latency_ms,\n" + 
+                        "            maximum_live_cells_per_slice_last_five_minutes,\n" + 
+                        "            maximum_tombstones_per_slice_last_five_minutes, memtable_cell_count,\n" + 
+                        "            memtable_data_size, memtable_off_heap_memory_used,\n" + 
+                        "            memtable_switch_count, number_of_partitions_estimate,\n" + 
+                        "            off_heap_memory_used_total, pending_flushes, percent_repaired,\n" + 
+                        "            read_latency, reads, space_used_by_snapshots_total, space_used_live,\n" + 
+                        "            space_used_total, sstable_compression_ratio, sstable_count,\n" + 
+                        "            table_name, write_latency, writes)\n" + 
+                        "\n" + 
+                        "        -t <top>, --top <top>\n" + 
+                        "            Show only the top K tables for the sort key (specify the number K of\n" + 
+                        "            tables to be shown\n" + 
+                        "\n" + 
+                        "        -u <username>, --username <username>\n" + 
+                        "            Remote jmx agent username\n" + 
+                        "\n" + 
+                        "        --\n" + 
+                        "            This option can be used to separate command-line options from the\n" + 
+                        "            list of argument, (useful when arguments might be mistaken for\n" + 
+                        "            command-line options\n" + 
+                        "\n" + 
+                        "        [<keyspace.table>...]\n" + 
+                        "            List of tables (or keyspace) names\n" + 
+                        "\n" + 
+                        "\n";
+        Assertions.assertThat(tool.getStdout()).isEqualTo(help);
+    }
+
+    @Test
+    public void testTableStats()
+    {
+        ToolResult tool = ToolRunner.invokeNodetool("tablestats");
+
+        assertThat(tool.getStdout(), CoreMatchers.containsStringIgnoringCase("Keyspace : system_schema"));
+        assertTrue(StringUtils.countMatches(tool.getStdout(), "Table:") > 1);
+        assertTrue(tool.getCleanedStderr().isEmpty());
+        assertEquals(0, tool.getExitCode());
+
+        tool = ToolRunner.invokeNodetool("tablestats", "system_distributed");
+        assertThat(tool.getStdout(), CoreMatchers.containsStringIgnoringCase("Keyspace : system_distributed"));
+        assertThat(tool.getStdout(), CoreMatchers.not(CoreMatchers.containsStringIgnoringCase("Keyspace : system_schema")));
+        assertTrue(StringUtils.countMatches(tool.getStdout(), "Table:") > 1);
+        assertTrue(tool.getCleanedStderr().isEmpty());
+        assertEquals(0, tool.getExitCode());
+    }
+
+    @Test
+    public void testTableIgnoreArg()
+    {
+        ToolResult tool = ToolRunner.invokeNodetool("tablestats", "-i", "system_schema.aggregates");
+
+        assertThat(tool.getStdout(), CoreMatchers.containsStringIgnoringCase("Keyspace : system_schema"));
+        assertThat(tool.getStdout(), CoreMatchers.not(CoreMatchers.containsStringIgnoringCase("Table: system_schema.aggregates")));
+        assertTrue(StringUtils.countMatches(tool.getStdout(), "Table:") > 1);
+        assertTrue(tool.getCleanedStderr().isEmpty());
+        assertEquals(0, tool.getExitCode());
+    }
+
+    @Test
+    public void testHumanReadableArg()
+    {
+        Arrays.asList("-H", "--human-readable").forEach(arg -> {
+            ToolResult tool = ToolRunner.invokeNodetool("tablestats", arg);
+            assertThat("Arg: [" + arg + "]", tool.getStdout(), CoreMatchers.containsStringIgnoringCase(" KiB"));
+            assertTrue("Arg: [" + arg + "]", tool.getCleanedStderr().isEmpty());
+            assertEquals("Arg: [" + arg + "]", 0, tool.getExitCode());

Review comment:
       The `"Arg: [" + arg + "]"` error message is used multiple times here, and also in `testSortArg` and `testTopArg`. I think this is not very informative, so I' either remove it or replace it by a different message for each assertion.




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


[GitHub] [cassandra] bereng commented on pull request #788: CASSANDRA-16227 Add netstats and tablestats unit tests

Posted by GitBox <gi...@apache.org>.
bereng commented on pull request #788:
URL: https://github.com/apache/cassandra/pull/788#issuecomment-716491971


   Ci [j11](https://app.circleci.com/pipelines/github/bereng/cassandra/159/workflows/9fd8cd04-4864-4d23-a070-38f51abad1c5)
   CI [j8](https://app.circleci.com/pipelines/github/bereng/cassandra/159/workflows/52f66fda-217f-4325-b4c7-e96bea405e06)


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


[GitHub] [cassandra] adelapena commented on a change in pull request #788: CASSANDRA-16227 Add netstats and tablestats unit tests

Posted by GitBox <gi...@apache.org>.
adelapena commented on a change in pull request #788:
URL: https://github.com/apache/cassandra/pull/788#discussion_r514178531



##########
File path: test/unit/org/apache/cassandra/tools/NodetoolNetStatsTest.java
##########
@@ -0,0 +1,170 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.tools;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.PrintStream;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+import java.util.List;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.apache.cassandra.OrderedJUnit4ClassRunner;
+import org.apache.cassandra.cql3.CQLTester;
+import org.apache.cassandra.locator.InetAddressAndPort;
+import org.apache.cassandra.net.Message;
+import org.apache.cassandra.net.MessagingService;
+import org.apache.cassandra.net.NoPayload;
+import org.apache.cassandra.schema.TableId;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.streaming.SessionInfo;
+import org.apache.cassandra.streaming.StreamSession.State;
+import org.apache.cassandra.streaming.StreamSummary;
+import org.apache.cassandra.tools.ToolRunner.ToolResult;
+import org.apache.cassandra.tools.nodetool.NetStats;
+import org.apache.cassandra.utils.FBUtilities;
+import org.assertj.core.api.Assertions;
+import org.hamcrest.CoreMatchers;
+
+import static org.apache.cassandra.net.Verb.ECHO_REQ;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+@RunWith(OrderedJUnit4ClassRunner.class)
+public class NodetoolNetStatsTest extends CQLTester
+{
+    private static NodeProbe probe;
+
+    @BeforeClass
+    public static void setup() throws Exception
+    {
+        StorageService.instance.initServer();
+        startJMXServer();
+        probe = new NodeProbe(jmxHost, jmxPort);
+    }
+
+    @AfterClass
+    public static void teardown() throws IOException
+    {
+        probe.close();
+    }
+
+    @Test
+    public void testMaybeChangeDocs()
+    {
+        // If you added, modified options or help, please update docs if necessary
+        ToolResult tool = ToolRunner.invokeNodetool("help", "netstats");
+        String help =   "NAME\n" + 
+                        "        nodetool netstats - Print network information on provided host\n" + 
+                        "        (connecting node by default)\n" + 
+                        "\n" + 
+                        "SYNOPSIS\n" + 
+                        "        nodetool [(-h <host> | --host <host>)] [(-p <port> | --port <port>)]\n" + 
+                        "                [(-pp | --print-port)] [(-pw <password> | --password <password>)]\n" + 
+                        "                [(-pwf <passwordFilePath> | --password-file <passwordFilePath>)]\n" + 
+                        "                [(-u <username> | --username <username>)] netstats\n" + 
+                        "                [(-H | --human-readable)]\n" + 
+                        "\n" + 
+                        "OPTIONS\n" + 
+                        "        -h <host>, --host <host>\n" + 
+                        "            Node hostname or ip address\n" + 
+                        "\n" + 
+                        "        -H, --human-readable\n" + 
+                        "            Display bytes in human readable form, i.e. KiB, MiB, GiB, TiB\n" + 
+                        "\n" + 
+                        "        -p <port>, --port <port>\n" + 
+                        "            Remote jmx agent port number\n" + 
+                        "\n" + 
+                        "        -pp, --print-port\n" + 
+                        "            Operate in 4.0 mode with hosts disambiguated by port number\n" + 
+                        "\n" + 
+                        "        -pw <password>, --password <password>\n" + 
+                        "            Remote jmx agent password\n" + 
+                        "\n" + 
+                        "        -pwf <passwordFilePath>, --password-file <passwordFilePath>\n" + 
+                        "            Path to the JMX password file\n" + 
+                        "\n" + 
+                        "        -u <username>, --username <username>\n" + 
+                        "            Remote jmx agent username\n" + 
+                        "\n" + 
+                        "\n";
+        Assertions.assertThat(tool.getStdout()).isEqualTo(help);
+    }
+
+    @Test
+    public void testNetStats()
+    {
+        Message<NoPayload> echoMessageOut = Message.out(ECHO_REQ, NoPayload.noPayload);
+        MessagingService.instance().send(echoMessageOut, FBUtilities.getBroadcastAddressAndPort());
+        
+        ToolResult tool = ToolRunner.invokeNodetool("netstats");
+        assertThat(tool.getStdout(), CoreMatchers.containsString("Gossip messages                 n/a         0              2         0"));
+        assertTrue(tool.getCleanedStderr().isEmpty());
+        assertEquals(0, tool.getExitCode());
+    }
+
+    @Test
+    public void testHumanReadable() throws IOException
+    {
+        List<StreamSummary> streamSummaries = Arrays.asList(new StreamSummary(TableId.generate(), 1, 1024));

Review comment:
       ```suggestion
           List<StreamSummary> streamSummaries = Collections.singletonList(new StreamSummary(TableId.generate(), 1, 1024));
   ```




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


[GitHub] [cassandra] bereng commented on a change in pull request #788: CASSANDRA-16227 Add netstats and tablestats unit tests

Posted by GitBox <gi...@apache.org>.
bereng commented on a change in pull request #788:
URL: https://github.com/apache/cassandra/pull/788#discussion_r513983768



##########
File path: test/unit/org/apache/cassandra/tools/nodetool/stats/NodetoolTableStatsTest.java
##########
@@ -0,0 +1,237 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.tools.nodetool.stats;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.commons.lang3.StringUtils;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.apache.cassandra.OrderedJUnit4ClassRunner;
+import org.apache.cassandra.cql3.CQLTester;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.tools.NodeProbe;
+import org.apache.cassandra.tools.ToolRunner;
+import org.apache.cassandra.tools.ToolRunner.ToolResult;
+import org.assertj.core.api.Assertions;
+import org.hamcrest.CoreMatchers;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+@RunWith(OrderedJUnit4ClassRunner.class)
+public class NodetoolTableStatsTest extends CQLTester
+{
+    private static NodeProbe probe;
+
+    @BeforeClass
+    public static void setup() throws Exception
+    {
+        StorageService.instance.initServer();
+        startJMXServer();
+        probe = new NodeProbe(jmxHost, jmxPort);
+    }
+
+    @AfterClass
+    public static void teardown() throws IOException
+    {
+        probe.close();
+    }
+
+    @Test
+    public void testMaybeChangeDocs()
+    {
+        // If you added, modified options or help, please update docs if necessary
+        ToolResult tool = ToolRunner.invokeNodetool("help", "tablestats");
+        String help =   "NAME\n" +
+                        "        nodetool tablestats - Print statistics on tables\n" + 
+                        "\n" + 
+                        "SYNOPSIS\n" + 
+                        "        nodetool [(-h <host> | --host <host>)] [(-p <port> | --port <port>)]\n" + 
+                        "                [(-pp | --print-port)] [(-pw <password> | --password <password>)]\n" + 
+                        "                [(-pwf <passwordFilePath> | --password-file <passwordFilePath>)]\n" + 
+                        "                [(-u <username> | --username <username>)] tablestats\n" + 
+                        "                [(-F <format> | --format <format>)] [(-H | --human-readable)] [-i]\n" + 
+                        "                [(-s <sort_key> | --sort <sort_key>)] [(-t <top> | --top <top>)] [--]\n" + 
+                        "                [<keyspace.table>...]\n" + 
+                        "\n" + 
+                        "OPTIONS\n" + 
+                        "        -F <format>, --format <format>\n" + 
+                        "            Output format (json, yaml)\n" + 
+                        "\n" + 
+                        "        -h <host>, --host <host>\n" + 
+                        "            Node hostname or ip address\n" + 
+                        "\n" + 
+                        "        -H, --human-readable\n" + 
+                        "            Display bytes in human readable form, i.e. KiB, MiB, GiB, TiB\n" + 
+                        "\n" + 
+                        "        -i\n" + 
+                        "            Ignore the list of tables and display the remaining tables\n" + 
+                        "\n" + 
+                        "        -p <port>, --port <port>\n" + 
+                        "            Remote jmx agent port number\n" + 
+                        "\n" + 
+                        "        -pp, --print-port\n" + 
+                        "            Operate in 4.0 mode with hosts disambiguated by port number\n" + 
+                        "\n" + 
+                        "        -pw <password>, --password <password>\n" + 
+                        "            Remote jmx agent password\n" + 
+                        "\n" + 
+                        "        -pwf <passwordFilePath>, --password-file <passwordFilePath>\n" + 
+                        "            Path to the JMX password file\n" + 
+                        "\n" + 
+                        "        -s <sort_key>, --sort <sort_key>\n" + 
+                        "            Sort tables by specified sort key\n" + 
+                        "            (average_live_cells_per_slice_last_five_minutes,\n" + 
+                        "            average_tombstones_per_slice_last_five_minutes,\n" + 
+                        "            bloom_filter_false_positives, bloom_filter_false_ratio,\n" + 
+                        "            bloom_filter_off_heap_memory_used, bloom_filter_space_used,\n" + 
+                        "            compacted_partition_maximum_bytes, compacted_partition_mean_bytes,\n" + 
+                        "            compacted_partition_minimum_bytes,\n" + 
+                        "            compression_metadata_off_heap_memory_used, dropped_mutations,\n" + 
+                        "            full_name, index_summary_off_heap_memory_used, local_read_count,\n" + 
+                        "            local_read_latency_ms, local_write_latency_ms,\n" + 
+                        "            maximum_live_cells_per_slice_last_five_minutes,\n" + 
+                        "            maximum_tombstones_per_slice_last_five_minutes, memtable_cell_count,\n" + 
+                        "            memtable_data_size, memtable_off_heap_memory_used,\n" + 
+                        "            memtable_switch_count, number_of_partitions_estimate,\n" + 
+                        "            off_heap_memory_used_total, pending_flushes, percent_repaired,\n" + 
+                        "            read_latency, reads, space_used_by_snapshots_total, space_used_live,\n" + 
+                        "            space_used_total, sstable_compression_ratio, sstable_count,\n" + 
+                        "            table_name, write_latency, writes)\n" + 
+                        "\n" + 
+                        "        -t <top>, --top <top>\n" + 
+                        "            Show only the top K tables for the sort key (specify the number K of\n" + 
+                        "            tables to be shown\n" + 
+                        "\n" + 
+                        "        -u <username>, --username <username>\n" + 
+                        "            Remote jmx agent username\n" + 
+                        "\n" + 
+                        "        --\n" + 
+                        "            This option can be used to separate command-line options from the\n" + 
+                        "            list of argument, (useful when arguments might be mistaken for\n" + 
+                        "            command-line options\n" + 
+                        "\n" + 
+                        "        [<keyspace.table>...]\n" + 
+                        "            List of tables (or keyspace) names\n" + 
+                        "\n" + 
+                        "\n";
+        Assertions.assertThat(tool.getStdout()).isEqualTo(help);
+    }
+
+    @Test
+    public void testTableStats()
+    {
+        ToolResult tool = ToolRunner.invokeNodetool("tablestats");
+
+        assertThat(tool.getStdout(), CoreMatchers.containsStringIgnoringCase("Keyspace : system_schema"));
+        assertTrue(StringUtils.countMatches(tool.getStdout(), "Table:") > 1);
+        assertTrue(tool.getCleanedStderr().isEmpty());
+        assertEquals(0, tool.getExitCode());
+
+        tool = ToolRunner.invokeNodetool("tablestats", "system_distributed");
+        assertThat(tool.getStdout(), CoreMatchers.containsStringIgnoringCase("Keyspace : system_distributed"));
+        assertThat(tool.getStdout(), CoreMatchers.not(CoreMatchers.containsStringIgnoringCase("Keyspace : system_schema")));
+        assertTrue(StringUtils.countMatches(tool.getStdout(), "Table:") > 1);
+        assertTrue(tool.getCleanedStderr().isEmpty());
+        assertEquals(0, tool.getExitCode());
+    }
+
+    @Test
+    public void testTableIgnoreArg()
+    {
+        ToolResult tool = ToolRunner.invokeNodetool("tablestats", "-i", "system_schema.aggregates");
+
+        assertThat(tool.getStdout(), CoreMatchers.containsStringIgnoringCase("Keyspace : system_schema"));
+        assertThat(tool.getStdout(), CoreMatchers.not(CoreMatchers.containsStringIgnoringCase("Table: system_schema.aggregates")));
+        assertTrue(StringUtils.countMatches(tool.getStdout(), "Table:") > 1);
+        assertTrue(tool.getCleanedStderr().isEmpty());
+        assertEquals(0, tool.getExitCode());
+    }
+
+    @Test
+    public void testHumanReadableArg()
+    {
+        Arrays.asList("-H", "--human-readable").forEach(arg -> {
+            ToolResult tool = ToolRunner.invokeNodetool("tablestats", arg);
+            assertThat("Arg: [" + arg + "]", tool.getStdout(), CoreMatchers.containsStringIgnoringCase(" KiB"));
+            assertTrue("Arg: [" + arg + "]", tool.getCleanedStderr().isEmpty());
+            assertEquals("Arg: [" + arg + "]", 0, tool.getExitCode());

Review comment:
       This has turned out to be very useful in some cases. If you accidentally change `-sort` to `-sorta` in the code bc of a typo you need to know which of the args is failing. In commands that accept many combinations of values this has been extremely useful as well. So I am happy to change the message but I don't want to remove it. wdyt, what message would you put?




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


[GitHub] [cassandra] bereng commented on a change in pull request #788: CASSANDRA-16227 Add netstats and tablestats unit tests

Posted by GitBox <gi...@apache.org>.
bereng commented on a change in pull request #788:
URL: https://github.com/apache/cassandra/pull/788#discussion_r513985982



##########
File path: test/unit/org/apache/cassandra/tools/nodetool/stats/NodetoolTableStatsTest.java
##########
@@ -0,0 +1,237 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.tools.nodetool.stats;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.commons.lang3.StringUtils;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.apache.cassandra.OrderedJUnit4ClassRunner;
+import org.apache.cassandra.cql3.CQLTester;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.tools.NodeProbe;
+import org.apache.cassandra.tools.ToolRunner;
+import org.apache.cassandra.tools.ToolRunner.ToolResult;
+import org.assertj.core.api.Assertions;
+import org.hamcrest.CoreMatchers;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+@RunWith(OrderedJUnit4ClassRunner.class)
+public class NodetoolTableStatsTest extends CQLTester
+{
+    private static NodeProbe probe;
+
+    @BeforeClass
+    public static void setup() throws Exception
+    {
+        StorageService.instance.initServer();
+        startJMXServer();
+        probe = new NodeProbe(jmxHost, jmxPort);
+    }
+
+    @AfterClass
+    public static void teardown() throws IOException
+    {
+        probe.close();
+    }
+
+    @Test
+    public void testMaybeChangeDocs()
+    {
+        // If you added, modified options or help, please update docs if necessary
+        ToolResult tool = ToolRunner.invokeNodetool("help", "tablestats");
+        String help =   "NAME\n" +
+                        "        nodetool tablestats - Print statistics on tables\n" + 
+                        "\n" + 
+                        "SYNOPSIS\n" + 
+                        "        nodetool [(-h <host> | --host <host>)] [(-p <port> | --port <port>)]\n" + 
+                        "                [(-pp | --print-port)] [(-pw <password> | --password <password>)]\n" + 
+                        "                [(-pwf <passwordFilePath> | --password-file <passwordFilePath>)]\n" + 
+                        "                [(-u <username> | --username <username>)] tablestats\n" + 
+                        "                [(-F <format> | --format <format>)] [(-H | --human-readable)] [-i]\n" + 
+                        "                [(-s <sort_key> | --sort <sort_key>)] [(-t <top> | --top <top>)] [--]\n" + 
+                        "                [<keyspace.table>...]\n" + 
+                        "\n" + 
+                        "OPTIONS\n" + 
+                        "        -F <format>, --format <format>\n" + 
+                        "            Output format (json, yaml)\n" + 
+                        "\n" + 
+                        "        -h <host>, --host <host>\n" + 
+                        "            Node hostname or ip address\n" + 
+                        "\n" + 
+                        "        -H, --human-readable\n" + 
+                        "            Display bytes in human readable form, i.e. KiB, MiB, GiB, TiB\n" + 
+                        "\n" + 
+                        "        -i\n" + 
+                        "            Ignore the list of tables and display the remaining tables\n" + 
+                        "\n" + 
+                        "        -p <port>, --port <port>\n" + 
+                        "            Remote jmx agent port number\n" + 
+                        "\n" + 
+                        "        -pp, --print-port\n" + 
+                        "            Operate in 4.0 mode with hosts disambiguated by port number\n" + 
+                        "\n" + 
+                        "        -pw <password>, --password <password>\n" + 
+                        "            Remote jmx agent password\n" + 
+                        "\n" + 
+                        "        -pwf <passwordFilePath>, --password-file <passwordFilePath>\n" + 
+                        "            Path to the JMX password file\n" + 
+                        "\n" + 
+                        "        -s <sort_key>, --sort <sort_key>\n" + 
+                        "            Sort tables by specified sort key\n" + 
+                        "            (average_live_cells_per_slice_last_five_minutes,\n" + 
+                        "            average_tombstones_per_slice_last_five_minutes,\n" + 
+                        "            bloom_filter_false_positives, bloom_filter_false_ratio,\n" + 
+                        "            bloom_filter_off_heap_memory_used, bloom_filter_space_used,\n" + 
+                        "            compacted_partition_maximum_bytes, compacted_partition_mean_bytes,\n" + 
+                        "            compacted_partition_minimum_bytes,\n" + 
+                        "            compression_metadata_off_heap_memory_used, dropped_mutations,\n" + 
+                        "            full_name, index_summary_off_heap_memory_used, local_read_count,\n" + 
+                        "            local_read_latency_ms, local_write_latency_ms,\n" + 
+                        "            maximum_live_cells_per_slice_last_five_minutes,\n" + 
+                        "            maximum_tombstones_per_slice_last_five_minutes, memtable_cell_count,\n" + 
+                        "            memtable_data_size, memtable_off_heap_memory_used,\n" + 
+                        "            memtable_switch_count, number_of_partitions_estimate,\n" + 
+                        "            off_heap_memory_used_total, pending_flushes, percent_repaired,\n" + 
+                        "            read_latency, reads, space_used_by_snapshots_total, space_used_live,\n" + 
+                        "            space_used_total, sstable_compression_ratio, sstable_count,\n" + 
+                        "            table_name, write_latency, writes)\n" + 
+                        "\n" + 
+                        "        -t <top>, --top <top>\n" + 
+                        "            Show only the top K tables for the sort key (specify the number K of\n" + 
+                        "            tables to be shown\n" + 
+                        "\n" + 
+                        "        -u <username>, --username <username>\n" + 
+                        "            Remote jmx agent username\n" + 
+                        "\n" + 
+                        "        --\n" + 
+                        "            This option can be used to separate command-line options from the\n" + 
+                        "            list of argument, (useful when arguments might be mistaken for\n" + 
+                        "            command-line options\n" + 
+                        "\n" + 
+                        "        [<keyspace.table>...]\n" + 
+                        "            List of tables (or keyspace) names\n" + 
+                        "\n" + 
+                        "\n";
+        Assertions.assertThat(tool.getStdout()).isEqualTo(help);
+    }
+
+    @Test
+    public void testTableStats()
+    {
+        ToolResult tool = ToolRunner.invokeNodetool("tablestats");
+
+        assertThat(tool.getStdout(), CoreMatchers.containsStringIgnoringCase("Keyspace : system_schema"));
+        assertTrue(StringUtils.countMatches(tool.getStdout(), "Table:") > 1);
+        assertTrue(tool.getCleanedStderr().isEmpty());
+        assertEquals(0, tool.getExitCode());
+
+        tool = ToolRunner.invokeNodetool("tablestats", "system_distributed");
+        assertThat(tool.getStdout(), CoreMatchers.containsStringIgnoringCase("Keyspace : system_distributed"));
+        assertThat(tool.getStdout(), CoreMatchers.not(CoreMatchers.containsStringIgnoringCase("Keyspace : system_schema")));
+        assertTrue(StringUtils.countMatches(tool.getStdout(), "Table:") > 1);
+        assertTrue(tool.getCleanedStderr().isEmpty());
+        assertEquals(0, tool.getExitCode());
+    }

Review comment:
       Multiple checks on stdout are not the norm but the exception on the other many tooling tests we have just been introducing. So I see little added value and I would have to change a gazillion other tests to make things consistent. I would rather leave it as it is if you don't mind.




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


[GitHub] [cassandra] bereng commented on pull request #788: CASSANDRA-16227 Add netstats and tablestats unit tests

Posted by GitBox <gi...@apache.org>.
bereng commented on pull request #788:
URL: https://github.com/apache/cassandra/pull/788#issuecomment-718780708


   I had forgotten about `assertCleanExit()` +1
   
   CI [j11](https://app.circleci.com/pipelines/github/bereng/cassandra/168/workflows/211dfe89-a4a4-4260-bcef-96c72e2621fa)
   CI [j8](https://app.circleci.com/pipelines/github/bereng/cassandra/168/workflows/1db60055-87b6-497f-9021-b58b924158ae)


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


[GitHub] [cassandra] bereng commented on a change in pull request #788: CASSANDRA-16227 Add netstats and tablestats unit tests

Posted by GitBox <gi...@apache.org>.
bereng commented on a change in pull request #788:
URL: https://github.com/apache/cassandra/pull/788#discussion_r514199787



##########
File path: test/unit/org/apache/cassandra/tools/nodetool/stats/NodetoolTableStatsTest.java
##########
@@ -0,0 +1,237 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.tools.nodetool.stats;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.commons.lang3.StringUtils;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.apache.cassandra.OrderedJUnit4ClassRunner;
+import org.apache.cassandra.cql3.CQLTester;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.tools.NodeProbe;
+import org.apache.cassandra.tools.ToolRunner;
+import org.apache.cassandra.tools.ToolRunner.ToolResult;
+import org.assertj.core.api.Assertions;
+import org.hamcrest.CoreMatchers;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+@RunWith(OrderedJUnit4ClassRunner.class)
+public class NodetoolTableStatsTest extends CQLTester
+{
+    private static NodeProbe probe;
+
+    @BeforeClass
+    public static void setup() throws Exception
+    {
+        StorageService.instance.initServer();
+        startJMXServer();
+        probe = new NodeProbe(jmxHost, jmxPort);
+    }
+
+    @AfterClass
+    public static void teardown() throws IOException
+    {
+        probe.close();
+    }
+
+    @Test
+    public void testMaybeChangeDocs()
+    {
+        // If you added, modified options or help, please update docs if necessary
+        ToolResult tool = ToolRunner.invokeNodetool("help", "tablestats");
+        String help =   "NAME\n" +
+                        "        nodetool tablestats - Print statistics on tables\n" + 
+                        "\n" + 
+                        "SYNOPSIS\n" + 
+                        "        nodetool [(-h <host> | --host <host>)] [(-p <port> | --port <port>)]\n" + 
+                        "                [(-pp | --print-port)] [(-pw <password> | --password <password>)]\n" + 
+                        "                [(-pwf <passwordFilePath> | --password-file <passwordFilePath>)]\n" + 
+                        "                [(-u <username> | --username <username>)] tablestats\n" + 
+                        "                [(-F <format> | --format <format>)] [(-H | --human-readable)] [-i]\n" + 
+                        "                [(-s <sort_key> | --sort <sort_key>)] [(-t <top> | --top <top>)] [--]\n" + 
+                        "                [<keyspace.table>...]\n" + 
+                        "\n" + 
+                        "OPTIONS\n" + 
+                        "        -F <format>, --format <format>\n" + 
+                        "            Output format (json, yaml)\n" + 
+                        "\n" + 
+                        "        -h <host>, --host <host>\n" + 
+                        "            Node hostname or ip address\n" + 
+                        "\n" + 
+                        "        -H, --human-readable\n" + 
+                        "            Display bytes in human readable form, i.e. KiB, MiB, GiB, TiB\n" + 
+                        "\n" + 
+                        "        -i\n" + 
+                        "            Ignore the list of tables and display the remaining tables\n" + 
+                        "\n" + 
+                        "        -p <port>, --port <port>\n" + 
+                        "            Remote jmx agent port number\n" + 
+                        "\n" + 
+                        "        -pp, --print-port\n" + 
+                        "            Operate in 4.0 mode with hosts disambiguated by port number\n" + 
+                        "\n" + 
+                        "        -pw <password>, --password <password>\n" + 
+                        "            Remote jmx agent password\n" + 
+                        "\n" + 
+                        "        -pwf <passwordFilePath>, --password-file <passwordFilePath>\n" + 
+                        "            Path to the JMX password file\n" + 
+                        "\n" + 
+                        "        -s <sort_key>, --sort <sort_key>\n" + 
+                        "            Sort tables by specified sort key\n" + 
+                        "            (average_live_cells_per_slice_last_five_minutes,\n" + 
+                        "            average_tombstones_per_slice_last_five_minutes,\n" + 
+                        "            bloom_filter_false_positives, bloom_filter_false_ratio,\n" + 
+                        "            bloom_filter_off_heap_memory_used, bloom_filter_space_used,\n" + 
+                        "            compacted_partition_maximum_bytes, compacted_partition_mean_bytes,\n" + 
+                        "            compacted_partition_minimum_bytes,\n" + 
+                        "            compression_metadata_off_heap_memory_used, dropped_mutations,\n" + 
+                        "            full_name, index_summary_off_heap_memory_used, local_read_count,\n" + 
+                        "            local_read_latency_ms, local_write_latency_ms,\n" + 
+                        "            maximum_live_cells_per_slice_last_five_minutes,\n" + 
+                        "            maximum_tombstones_per_slice_last_five_minutes, memtable_cell_count,\n" + 
+                        "            memtable_data_size, memtable_off_heap_memory_used,\n" + 
+                        "            memtable_switch_count, number_of_partitions_estimate,\n" + 
+                        "            off_heap_memory_used_total, pending_flushes, percent_repaired,\n" + 
+                        "            read_latency, reads, space_used_by_snapshots_total, space_used_live,\n" + 
+                        "            space_used_total, sstable_compression_ratio, sstable_count,\n" + 
+                        "            table_name, write_latency, writes)\n" + 
+                        "\n" + 
+                        "        -t <top>, --top <top>\n" + 
+                        "            Show only the top K tables for the sort key (specify the number K of\n" + 
+                        "            tables to be shown\n" + 
+                        "\n" + 
+                        "        -u <username>, --username <username>\n" + 
+                        "            Remote jmx agent username\n" + 
+                        "\n" + 
+                        "        --\n" + 
+                        "            This option can be used to separate command-line options from the\n" + 
+                        "            list of argument, (useful when arguments might be mistaken for\n" + 
+                        "            command-line options\n" + 
+                        "\n" + 
+                        "        [<keyspace.table>...]\n" + 
+                        "            List of tables (or keyspace) names\n" + 
+                        "\n" + 
+                        "\n";
+        Assertions.assertThat(tool.getStdout()).isEqualTo(help);
+    }
+
+    @Test
+    public void testTableStats()
+    {
+        ToolResult tool = ToolRunner.invokeNodetool("tablestats");
+
+        assertThat(tool.getStdout(), CoreMatchers.containsStringIgnoringCase("Keyspace : system_schema"));
+        assertTrue(StringUtils.countMatches(tool.getStdout(), "Table:") > 1);
+        assertTrue(tool.getCleanedStderr().isEmpty());
+        assertEquals(0, tool.getExitCode());
+
+        tool = ToolRunner.invokeNodetool("tablestats", "system_distributed");
+        assertThat(tool.getStdout(), CoreMatchers.containsStringIgnoringCase("Keyspace : system_distributed"));
+        assertThat(tool.getStdout(), CoreMatchers.not(CoreMatchers.containsStringIgnoringCase("Keyspace : system_schema")));
+        assertTrue(StringUtils.countMatches(tool.getStdout(), "Table:") > 1);
+        assertTrue(tool.getCleanedStderr().isEmpty());
+        assertEquals(0, tool.getExitCode());
+    }
+
+    @Test
+    public void testTableIgnoreArg()
+    {
+        ToolResult tool = ToolRunner.invokeNodetool("tablestats", "-i", "system_schema.aggregates");
+
+        assertThat(tool.getStdout(), CoreMatchers.containsStringIgnoringCase("Keyspace : system_schema"));
+        assertThat(tool.getStdout(), CoreMatchers.not(CoreMatchers.containsStringIgnoringCase("Table: system_schema.aggregates")));
+        assertTrue(StringUtils.countMatches(tool.getStdout(), "Table:") > 1);
+        assertTrue(tool.getCleanedStderr().isEmpty());
+        assertEquals(0, tool.getExitCode());
+    }
+
+    @Test
+    public void testHumanReadableArg()
+    {
+        Arrays.asList("-H", "--human-readable").forEach(arg -> {
+            ToolResult tool = ToolRunner.invokeNodetool("tablestats", arg);
+            assertThat("Arg: [" + arg + "]", tool.getStdout(), CoreMatchers.containsStringIgnoringCase(" KiB"));
+            assertTrue("Arg: [" + arg + "]", tool.getCleanedStderr().isEmpty());
+            assertEquals("Arg: [" + arg + "]", 0, tool.getExitCode());

Review comment:
       Unless I am misundertsnading notice the usage of `assertThat` which should give you a very nice description when it fails with exactly what you're proposing. That is one of the advantages of `asserThat` afaik.
   
   The other 2 I can change.
   
   




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


[GitHub] [cassandra] adelapena commented on a change in pull request #788: CASSANDRA-16227 Add netstats and tablestats unit tests

Posted by GitBox <gi...@apache.org>.
adelapena commented on a change in pull request #788:
URL: https://github.com/apache/cassandra/pull/788#discussion_r514231216



##########
File path: test/unit/org/apache/cassandra/tools/NodetoolNetStatsTest.java
##########
@@ -0,0 +1,171 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.tools;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.PrintStream;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;

Review comment:
       Nit: unusedImport

##########
File path: test/unit/org/apache/cassandra/tools/NodetoolNetStatsTest.java
##########
@@ -0,0 +1,171 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.tools;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.PrintStream;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;

Review comment:
       Nit: unused import




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


[GitHub] [cassandra] adelapena commented on pull request #788: CASSANDRA-16227 Add netstats and tablestats unit tests

Posted by GitBox <gi...@apache.org>.
adelapena commented on pull request #788:
URL: https://github.com/apache/cassandra/pull/788#issuecomment-718727890


   > I have gone through unused imports in `test/unit/org/apache/cassandra/tools/nodetool/stats` if that is what you were referring to. I also tweaked generics usage but [this](https://github.com/bereng/cassandra/blob/CASSANDRA-16227/test/unit/org/apache/cassandra/tools/nodetool/stats/StatsTableComparatorTest.java#L53) guy is giving me a warning despite both the List and the Comparator are both typed 🤷. I must be missing sthg...
   
   Yes, I meant that. I thought that given that we are not going to add more tests for TableStats under this ticket at least we can do some quick and easy cleaning of warnings on its tests classes. Regarding that warning on `StatsTableComparatorTest`, I think that it is suggesting to use the instance method `List#sort` instead of the static `Collections.sort`:
   ```
   vector.sort(new StatsTableComparator(sortKey, humanReadable, ascending));
   ```


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


[GitHub] [cassandra] bereng commented on pull request #788: CASSANDRA-16227 Add netstats and tablestats unit tests

Posted by GitBox <gi...@apache.org>.
bereng commented on pull request #788:
URL: https://github.com/apache/cassandra/pull/788#issuecomment-718826631


   CI [j11](https://app.circleci.com/pipelines/github/bereng/cassandra/173/workflows/92327e9e-6797-48ca-ac86-be68d7e6f344)
   Ci [j8](https://app.circleci.com/pipelines/github/bereng/cassandra/173/workflows/56178638-303c-4d1e-9347-8f706eba21ac)


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


[GitHub] [cassandra] adelapena commented on a change in pull request #788: CASSANDRA-16227 Add netstats and tablestats unit tests

Posted by GitBox <gi...@apache.org>.
adelapena commented on a change in pull request #788:
URL: https://github.com/apache/cassandra/pull/788#discussion_r514223321



##########
File path: test/unit/org/apache/cassandra/tools/nodetool/stats/NodetoolTableStatsTest.java
##########
@@ -0,0 +1,237 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.tools.nodetool.stats;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.commons.lang3.StringUtils;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.apache.cassandra.OrderedJUnit4ClassRunner;
+import org.apache.cassandra.cql3.CQLTester;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.tools.NodeProbe;
+import org.apache.cassandra.tools.ToolRunner;
+import org.apache.cassandra.tools.ToolRunner.ToolResult;
+import org.assertj.core.api.Assertions;
+import org.hamcrest.CoreMatchers;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+@RunWith(OrderedJUnit4ClassRunner.class)
+public class NodetoolTableStatsTest extends CQLTester
+{
+    private static NodeProbe probe;
+
+    @BeforeClass
+    public static void setup() throws Exception
+    {
+        StorageService.instance.initServer();
+        startJMXServer();
+        probe = new NodeProbe(jmxHost, jmxPort);
+    }
+
+    @AfterClass
+    public static void teardown() throws IOException
+    {
+        probe.close();
+    }
+
+    @Test
+    public void testMaybeChangeDocs()
+    {
+        // If you added, modified options or help, please update docs if necessary
+        ToolResult tool = ToolRunner.invokeNodetool("help", "tablestats");
+        String help =   "NAME\n" +
+                        "        nodetool tablestats - Print statistics on tables\n" + 
+                        "\n" + 
+                        "SYNOPSIS\n" + 
+                        "        nodetool [(-h <host> | --host <host>)] [(-p <port> | --port <port>)]\n" + 
+                        "                [(-pp | --print-port)] [(-pw <password> | --password <password>)]\n" + 
+                        "                [(-pwf <passwordFilePath> | --password-file <passwordFilePath>)]\n" + 
+                        "                [(-u <username> | --username <username>)] tablestats\n" + 
+                        "                [(-F <format> | --format <format>)] [(-H | --human-readable)] [-i]\n" + 
+                        "                [(-s <sort_key> | --sort <sort_key>)] [(-t <top> | --top <top>)] [--]\n" + 
+                        "                [<keyspace.table>...]\n" + 
+                        "\n" + 
+                        "OPTIONS\n" + 
+                        "        -F <format>, --format <format>\n" + 
+                        "            Output format (json, yaml)\n" + 
+                        "\n" + 
+                        "        -h <host>, --host <host>\n" + 
+                        "            Node hostname or ip address\n" + 
+                        "\n" + 
+                        "        -H, --human-readable\n" + 
+                        "            Display bytes in human readable form, i.e. KiB, MiB, GiB, TiB\n" + 
+                        "\n" + 
+                        "        -i\n" + 
+                        "            Ignore the list of tables and display the remaining tables\n" + 
+                        "\n" + 
+                        "        -p <port>, --port <port>\n" + 
+                        "            Remote jmx agent port number\n" + 
+                        "\n" + 
+                        "        -pp, --print-port\n" + 
+                        "            Operate in 4.0 mode with hosts disambiguated by port number\n" + 
+                        "\n" + 
+                        "        -pw <password>, --password <password>\n" + 
+                        "            Remote jmx agent password\n" + 
+                        "\n" + 
+                        "        -pwf <passwordFilePath>, --password-file <passwordFilePath>\n" + 
+                        "            Path to the JMX password file\n" + 
+                        "\n" + 
+                        "        -s <sort_key>, --sort <sort_key>\n" + 
+                        "            Sort tables by specified sort key\n" + 
+                        "            (average_live_cells_per_slice_last_five_minutes,\n" + 
+                        "            average_tombstones_per_slice_last_five_minutes,\n" + 
+                        "            bloom_filter_false_positives, bloom_filter_false_ratio,\n" + 
+                        "            bloom_filter_off_heap_memory_used, bloom_filter_space_used,\n" + 
+                        "            compacted_partition_maximum_bytes, compacted_partition_mean_bytes,\n" + 
+                        "            compacted_partition_minimum_bytes,\n" + 
+                        "            compression_metadata_off_heap_memory_used, dropped_mutations,\n" + 
+                        "            full_name, index_summary_off_heap_memory_used, local_read_count,\n" + 
+                        "            local_read_latency_ms, local_write_latency_ms,\n" + 
+                        "            maximum_live_cells_per_slice_last_five_minutes,\n" + 
+                        "            maximum_tombstones_per_slice_last_five_minutes, memtable_cell_count,\n" + 
+                        "            memtable_data_size, memtable_off_heap_memory_used,\n" + 
+                        "            memtable_switch_count, number_of_partitions_estimate,\n" + 
+                        "            off_heap_memory_used_total, pending_flushes, percent_repaired,\n" + 
+                        "            read_latency, reads, space_used_by_snapshots_total, space_used_live,\n" + 
+                        "            space_used_total, sstable_compression_ratio, sstable_count,\n" + 
+                        "            table_name, write_latency, writes)\n" + 
+                        "\n" + 
+                        "        -t <top>, --top <top>\n" + 
+                        "            Show only the top K tables for the sort key (specify the number K of\n" + 
+                        "            tables to be shown\n" + 
+                        "\n" + 
+                        "        -u <username>, --username <username>\n" + 
+                        "            Remote jmx agent username\n" + 
+                        "\n" + 
+                        "        --\n" + 
+                        "            This option can be used to separate command-line options from the\n" + 
+                        "            list of argument, (useful when arguments might be mistaken for\n" + 
+                        "            command-line options\n" + 
+                        "\n" + 
+                        "        [<keyspace.table>...]\n" + 
+                        "            List of tables (or keyspace) names\n" + 
+                        "\n" + 
+                        "\n";
+        Assertions.assertThat(tool.getStdout()).isEqualTo(help);
+    }
+
+    @Test
+    public void testTableStats()
+    {
+        ToolResult tool = ToolRunner.invokeNodetool("tablestats");
+
+        assertThat(tool.getStdout(), CoreMatchers.containsStringIgnoringCase("Keyspace : system_schema"));
+        assertTrue(StringUtils.countMatches(tool.getStdout(), "Table:") > 1);
+        assertTrue(tool.getCleanedStderr().isEmpty());
+        assertEquals(0, tool.getExitCode());
+
+        tool = ToolRunner.invokeNodetool("tablestats", "system_distributed");
+        assertThat(tool.getStdout(), CoreMatchers.containsStringIgnoringCase("Keyspace : system_distributed"));
+        assertThat(tool.getStdout(), CoreMatchers.not(CoreMatchers.containsStringIgnoringCase("Keyspace : system_schema")));
+        assertTrue(StringUtils.countMatches(tool.getStdout(), "Table:") > 1);
+        assertTrue(tool.getCleanedStderr().isEmpty());
+        assertEquals(0, tool.getExitCode());
+    }

Review comment:
       Well, that would remove a gazillion other code duplications :). I understand that's why `ToolRunner#assertOnCleanExit` was added during CASSANDRA-15942 (in slightly different way to its current state), I propose we use that method instead of the two assertions. I can count 12 places where it can be used, so not that many.
   




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


[GitHub] [cassandra] bereng commented on pull request #788: CASSANDRA-16227 Add netstats and tablestats unit tests

Posted by GitBox <gi...@apache.org>.
bereng commented on pull request #788:
URL: https://github.com/apache/cassandra/pull/788#issuecomment-718588205


   Ci [j11](https://app.circleci.com/pipelines/github/bereng/cassandra/161/workflows/6159e485-68b8-4e76-a69a-058fd2e86846)
   CI [j8](https://app.circleci.com/pipelines/github/bereng/cassandra/161/workflows/6fc745ae-42d5-4662-842f-b16800978954)


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


[GitHub] [cassandra] adelapena commented on a change in pull request #788: CASSANDRA-16227 Add netstats and tablestats unit tests

Posted by GitBox <gi...@apache.org>.
adelapena commented on a change in pull request #788:
URL: https://github.com/apache/cassandra/pull/788#discussion_r513418226



##########
File path: test/unit/org/apache/cassandra/tools/NodetoolNetStatsTest.java
##########
@@ -0,0 +1,116 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.tools;
+
+import java.io.IOException;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.apache.cassandra.OrderedJUnit4ClassRunner;
+import org.apache.cassandra.cql3.CQLTester;
+import org.apache.cassandra.net.Message;
+import org.apache.cassandra.net.MessagingService;
+import org.apache.cassandra.net.NoPayload;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.tools.ToolRunner.ToolResult;
+import org.apache.cassandra.utils.FBUtilities;
+import org.assertj.core.api.Assertions;
+import org.hamcrest.CoreMatchers;
+
+import static org.apache.cassandra.net.Verb.ECHO_REQ;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+@RunWith(OrderedJUnit4ClassRunner.class)
+public class NodetoolNetStatsTest extends CQLTester
+{
+    private static NodeProbe probe;
+
+    @BeforeClass
+    public static void setup() throws Exception
+    {
+        StorageService.instance.initServer();
+        startJMXServer();
+        probe = new NodeProbe(jmxHost, jmxPort);
+    }
+
+    @AfterClass
+    public static void teardown() throws IOException
+    {
+        probe.close();
+    }
+
+    @Test
+    public void testMaybeChangeDocs()
+    {
+        // If you added, modified options or help, please update docs if necessary
+        ToolResult tool = ToolRunner.invokeNodetool("help", "netstats");
+        String help =   "NAME\n" + 
+                        "        nodetool netstats - Print network information on provided host\n" + 
+                        "        (connecting node by default)\n" + 
+                        "\n" + 
+                        "SYNOPSIS\n" + 
+                        "        nodetool [(-h <host> | --host <host>)] [(-p <port> | --port <port>)]\n" + 
+                        "                [(-pp | --print-port)] [(-pw <password> | --password <password>)]\n" + 
+                        "                [(-pwf <passwordFilePath> | --password-file <passwordFilePath>)]\n" + 
+                        "                [(-u <username> | --username <username>)] netstats\n" + 
+                        "                [(-H | --human-readable)]\n" + 
+                        "\n" + 
+                        "OPTIONS\n" + 
+                        "        -h <host>, --host <host>\n" + 
+                        "            Node hostname or ip address\n" + 
+                        "\n" + 
+                        "        -H, --human-readable\n" + 
+                        "            Display bytes in human readable form, i.e. KiB, MiB, GiB, TiB\n" + 
+                        "\n" + 
+                        "        -p <port>, --port <port>\n" + 
+                        "            Remote jmx agent port number\n" + 
+                        "\n" + 
+                        "        -pp, --print-port\n" + 
+                        "            Operate in 4.0 mode with hosts disambiguated by port number\n" + 
+                        "\n" + 
+                        "        -pw <password>, --password <password>\n" + 
+                        "            Remote jmx agent password\n" + 
+                        "\n" + 
+                        "        -pwf <passwordFilePath>, --password-file <passwordFilePath>\n" + 
+                        "            Path to the JMX password file\n" + 
+                        "\n" + 
+                        "        -u <username>, --username <username>\n" + 
+                        "            Remote jmx agent username\n" + 
+                        "\n" + 
+                        "\n";
+        Assertions.assertThat(tool.getStdout()).isEqualTo(help);
+    }
+
+    @Test
+    public void testNetStats() throws Throwable

Review comment:
       Nit: the `throws Throwable` is not needed




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


[GitHub] [cassandra] adelapena commented on a change in pull request #788: CASSANDRA-16227 Add netstats and tablestats unit tests

Posted by GitBox <gi...@apache.org>.
adelapena commented on a change in pull request #788:
URL: https://github.com/apache/cassandra/pull/788#discussion_r514315580



##########
File path: test/unit/org/apache/cassandra/tools/nodetool/stats/NodetoolTableStatsTest.java
##########
@@ -0,0 +1,239 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.tools.nodetool.stats;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.commons.lang3.StringUtils;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.apache.cassandra.OrderedJUnit4ClassRunner;
+import org.apache.cassandra.cql3.CQLTester;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.tools.NodeProbe;
+import org.apache.cassandra.tools.ToolRunner;
+import org.apache.cassandra.tools.ToolRunner.ToolResult;
+import org.assertj.core.api.Assertions;
+import org.hamcrest.CoreMatchers;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+@RunWith(OrderedJUnit4ClassRunner.class)
+public class NodetoolTableStatsTest extends CQLTester
+{
+    private static NodeProbe probe;
+
+    @BeforeClass
+    public static void setup() throws Exception
+    {
+        StorageService.instance.initServer();
+        startJMXServer();
+        probe = new NodeProbe(jmxHost, jmxPort);
+    }
+
+    @AfterClass
+    public static void teardown() throws IOException
+    {
+        probe.close();
+    }
+
+    @Test
+    public void testMaybeChangeDocs()
+    {
+        // If you added, modified options or help, please update docs if necessary
+        ToolResult tool = ToolRunner.invokeNodetool("help", "tablestats");
+        String help =   "NAME\n" +
+                        "        nodetool tablestats - Print statistics on tables\n" + 
+                        "\n" + 
+                        "SYNOPSIS\n" + 
+                        "        nodetool [(-h <host> | --host <host>)] [(-p <port> | --port <port>)]\n" + 
+                        "                [(-pp | --print-port)] [(-pw <password> | --password <password>)]\n" + 
+                        "                [(-pwf <passwordFilePath> | --password-file <passwordFilePath>)]\n" + 
+                        "                [(-u <username> | --username <username>)] tablestats\n" + 
+                        "                [(-F <format> | --format <format>)] [(-H | --human-readable)] [-i]\n" + 
+                        "                [(-s <sort_key> | --sort <sort_key>)] [(-t <top> | --top <top>)] [--]\n" + 
+                        "                [<keyspace.table>...]\n" + 
+                        "\n" + 
+                        "OPTIONS\n" + 
+                        "        -F <format>, --format <format>\n" + 
+                        "            Output format (json, yaml)\n" + 
+                        "\n" + 
+                        "        -h <host>, --host <host>\n" + 
+                        "            Node hostname or ip address\n" + 
+                        "\n" + 
+                        "        -H, --human-readable\n" + 
+                        "            Display bytes in human readable form, i.e. KiB, MiB, GiB, TiB\n" + 
+                        "\n" + 
+                        "        -i\n" + 
+                        "            Ignore the list of tables and display the remaining tables\n" + 
+                        "\n" + 
+                        "        -p <port>, --port <port>\n" + 
+                        "            Remote jmx agent port number\n" + 
+                        "\n" + 
+                        "        -pp, --print-port\n" + 
+                        "            Operate in 4.0 mode with hosts disambiguated by port number\n" + 
+                        "\n" + 
+                        "        -pw <password>, --password <password>\n" + 
+                        "            Remote jmx agent password\n" + 
+                        "\n" + 
+                        "        -pwf <passwordFilePath>, --password-file <passwordFilePath>\n" + 
+                        "            Path to the JMX password file\n" + 
+                        "\n" + 
+                        "        -s <sort_key>, --sort <sort_key>\n" + 
+                        "            Sort tables by specified sort key\n" + 
+                        "            (average_live_cells_per_slice_last_five_minutes,\n" + 
+                        "            average_tombstones_per_slice_last_five_minutes,\n" + 
+                        "            bloom_filter_false_positives, bloom_filter_false_ratio,\n" + 
+                        "            bloom_filter_off_heap_memory_used, bloom_filter_space_used,\n" + 
+                        "            compacted_partition_maximum_bytes, compacted_partition_mean_bytes,\n" + 
+                        "            compacted_partition_minimum_bytes,\n" + 
+                        "            compression_metadata_off_heap_memory_used, dropped_mutations,\n" + 
+                        "            full_name, index_summary_off_heap_memory_used, local_read_count,\n" + 
+                        "            local_read_latency_ms, local_write_latency_ms,\n" + 
+                        "            maximum_live_cells_per_slice_last_five_minutes,\n" + 
+                        "            maximum_tombstones_per_slice_last_five_minutes, memtable_cell_count,\n" + 
+                        "            memtable_data_size, memtable_off_heap_memory_used,\n" + 
+                        "            memtable_switch_count, number_of_partitions_estimate,\n" + 
+                        "            off_heap_memory_used_total, pending_flushes, percent_repaired,\n" + 
+                        "            read_latency, reads, space_used_by_snapshots_total, space_used_live,\n" + 
+                        "            space_used_total, sstable_compression_ratio, sstable_count,\n" + 
+                        "            table_name, write_latency, writes)\n" + 
+                        "\n" + 
+                        "        -t <top>, --top <top>\n" + 
+                        "            Show only the top K tables for the sort key (specify the number K of\n" + 
+                        "            tables to be shown\n" + 
+                        "\n" + 
+                        "        -u <username>, --username <username>\n" + 
+                        "            Remote jmx agent username\n" + 
+                        "\n" + 
+                        "        --\n" + 
+                        "            This option can be used to separate command-line options from the\n" + 
+                        "            list of argument, (useful when arguments might be mistaken for\n" + 
+                        "            command-line options\n" + 
+                        "\n" + 
+                        "        [<keyspace.table>...]\n" + 
+                        "            List of tables (or keyspace) names\n" + 
+                        "\n" + 
+                        "\n";
+        Assertions.assertThat(tool.getStdout()).isEqualTo(help);
+    }
+
+    @Test
+    public void testTableStats()
+    {
+        ToolResult tool = ToolRunner.invokeNodetool("tablestats");
+
+        assertThat(tool.getStdout(), CoreMatchers.containsString("Keyspace : system_schema"));
+        assertTrue(StringUtils.countMatches(tool.getStdout(), "Table:") > 1);
+        tool.assertOnCleanExit();
+
+        tool = ToolRunner.invokeNodetool("tablestats", "system_distributed");
+        assertThat(tool.getStdout(), CoreMatchers.containsString("Keyspace : system_distributed"));
+        assertThat(tool.getStdout(), CoreMatchers.not(CoreMatchers.containsString("Keyspace : system_schema")));
+        assertTrue(StringUtils.countMatches(tool.getStdout(), "Table:") > 1);
+        tool.assertOnCleanExit();
+    }
+
+    @Test
+    public void testTableIgnoreArg()
+    {
+        ToolResult tool = ToolRunner.invokeNodetool("tablestats", "-i", "system_schema.aggregates");
+
+        assertThat(tool.getStdout(), CoreMatchers.containsString("Keyspace : system_schema"));
+        assertThat(tool.getStdout(), CoreMatchers.not(CoreMatchers.containsString("Table: system_schema.aggregates")));
+        assertTrue(StringUtils.countMatches(tool.getStdout(), "Table:") > 1);
+        tool.assertOnCleanExit();
+    }
+
+    @Test
+    public void testHumanReadableArg()
+    {
+        Arrays.asList("-H", "--human-readable").forEach(arg -> {
+            ToolResult tool = ToolRunner.invokeNodetool("tablestats", arg);
+            assertThat("Arg: [" + arg + "]", tool.getStdout(), CoreMatchers.containsString(" KiB"));
+            assertTrue(String.format("Expected empty stderr for option [%s] but found: %s",
+                                     arg,
+                                     tool.getCleanedStderr()),
+                       tool.getCleanedStderr().isEmpty());
+            assertEquals(String.format("Expected exit code 0 for option [%s] but found: %s", arg, tool.getExitCode()),
+                         0,
+                         tool.getExitCode());
+        });
+    }
+
+    @Test
+    public void testSortArg()
+    {
+        Pattern regExp = Pattern.compile("((?m)Table: .*$)");
+
+        Arrays.asList("-s", "--sort").forEach(arg -> {
+            ToolResult tool = ToolRunner.invokeNodetool("tablestats", arg, "table_name");
+            Matcher m = regExp.matcher(tool.getStdout());
+            ArrayList<String> orig = new ArrayList<>();
+            while (m.find())
+                orig.add(m.group(1));
+
+            tool = ToolRunner.invokeNodetool("tablestats", arg, "sstable_count");
+            m = regExp.matcher(tool.getStdout());
+            ArrayList<String> sorted = new ArrayList<>();
+            while (m.find())
+                sorted.add(m.group(1));
+
+            assertNotEquals("Arg: [" + arg + "]", orig, sorted);
+            Collections.sort(orig);
+            Collections.sort(sorted);
+            assertEquals("Arg: [" + arg + "]", orig, sorted);
+            assertTrue("Arg: [" + arg + "]", tool.getCleanedStderr().isEmpty());
+            assertEquals(0, tool.getExitCode());
+        });
+
+        ToolResult tool = ToolRunner.invokeNodetool("tablestats", "-s", "wrongSort");
+        assertThat(tool.getStdout(), CoreMatchers.containsString("argument for sort must be one of"));
+        assertTrue(tool.getCleanedStderr().isEmpty());

Review comment:
       ```suggestion
           tool.assertCleanStdErr();
   ```

##########
File path: test/unit/org/apache/cassandra/tools/nodetool/stats/NodetoolTableStatsTest.java
##########
@@ -0,0 +1,239 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.tools.nodetool.stats;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.commons.lang3.StringUtils;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.apache.cassandra.OrderedJUnit4ClassRunner;
+import org.apache.cassandra.cql3.CQLTester;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.tools.NodeProbe;
+import org.apache.cassandra.tools.ToolRunner;
+import org.apache.cassandra.tools.ToolRunner.ToolResult;
+import org.assertj.core.api.Assertions;
+import org.hamcrest.CoreMatchers;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+@RunWith(OrderedJUnit4ClassRunner.class)
+public class NodetoolTableStatsTest extends CQLTester
+{
+    private static NodeProbe probe;
+
+    @BeforeClass
+    public static void setup() throws Exception
+    {
+        StorageService.instance.initServer();
+        startJMXServer();
+        probe = new NodeProbe(jmxHost, jmxPort);
+    }
+
+    @AfterClass
+    public static void teardown() throws IOException
+    {
+        probe.close();
+    }
+
+    @Test
+    public void testMaybeChangeDocs()
+    {
+        // If you added, modified options or help, please update docs if necessary
+        ToolResult tool = ToolRunner.invokeNodetool("help", "tablestats");
+        String help =   "NAME\n" +
+                        "        nodetool tablestats - Print statistics on tables\n" + 
+                        "\n" + 
+                        "SYNOPSIS\n" + 
+                        "        nodetool [(-h <host> | --host <host>)] [(-p <port> | --port <port>)]\n" + 
+                        "                [(-pp | --print-port)] [(-pw <password> | --password <password>)]\n" + 
+                        "                [(-pwf <passwordFilePath> | --password-file <passwordFilePath>)]\n" + 
+                        "                [(-u <username> | --username <username>)] tablestats\n" + 
+                        "                [(-F <format> | --format <format>)] [(-H | --human-readable)] [-i]\n" + 
+                        "                [(-s <sort_key> | --sort <sort_key>)] [(-t <top> | --top <top>)] [--]\n" + 
+                        "                [<keyspace.table>...]\n" + 
+                        "\n" + 
+                        "OPTIONS\n" + 
+                        "        -F <format>, --format <format>\n" + 
+                        "            Output format (json, yaml)\n" + 
+                        "\n" + 
+                        "        -h <host>, --host <host>\n" + 
+                        "            Node hostname or ip address\n" + 
+                        "\n" + 
+                        "        -H, --human-readable\n" + 
+                        "            Display bytes in human readable form, i.e. KiB, MiB, GiB, TiB\n" + 
+                        "\n" + 
+                        "        -i\n" + 
+                        "            Ignore the list of tables and display the remaining tables\n" + 
+                        "\n" + 
+                        "        -p <port>, --port <port>\n" + 
+                        "            Remote jmx agent port number\n" + 
+                        "\n" + 
+                        "        -pp, --print-port\n" + 
+                        "            Operate in 4.0 mode with hosts disambiguated by port number\n" + 
+                        "\n" + 
+                        "        -pw <password>, --password <password>\n" + 
+                        "            Remote jmx agent password\n" + 
+                        "\n" + 
+                        "        -pwf <passwordFilePath>, --password-file <passwordFilePath>\n" + 
+                        "            Path to the JMX password file\n" + 
+                        "\n" + 
+                        "        -s <sort_key>, --sort <sort_key>\n" + 
+                        "            Sort tables by specified sort key\n" + 
+                        "            (average_live_cells_per_slice_last_five_minutes,\n" + 
+                        "            average_tombstones_per_slice_last_five_minutes,\n" + 
+                        "            bloom_filter_false_positives, bloom_filter_false_ratio,\n" + 
+                        "            bloom_filter_off_heap_memory_used, bloom_filter_space_used,\n" + 
+                        "            compacted_partition_maximum_bytes, compacted_partition_mean_bytes,\n" + 
+                        "            compacted_partition_minimum_bytes,\n" + 
+                        "            compression_metadata_off_heap_memory_used, dropped_mutations,\n" + 
+                        "            full_name, index_summary_off_heap_memory_used, local_read_count,\n" + 
+                        "            local_read_latency_ms, local_write_latency_ms,\n" + 
+                        "            maximum_live_cells_per_slice_last_five_minutes,\n" + 
+                        "            maximum_tombstones_per_slice_last_five_minutes, memtable_cell_count,\n" + 
+                        "            memtable_data_size, memtable_off_heap_memory_used,\n" + 
+                        "            memtable_switch_count, number_of_partitions_estimate,\n" + 
+                        "            off_heap_memory_used_total, pending_flushes, percent_repaired,\n" + 
+                        "            read_latency, reads, space_used_by_snapshots_total, space_used_live,\n" + 
+                        "            space_used_total, sstable_compression_ratio, sstable_count,\n" + 
+                        "            table_name, write_latency, writes)\n" + 
+                        "\n" + 
+                        "        -t <top>, --top <top>\n" + 
+                        "            Show only the top K tables for the sort key (specify the number K of\n" + 
+                        "            tables to be shown\n" + 
+                        "\n" + 
+                        "        -u <username>, --username <username>\n" + 
+                        "            Remote jmx agent username\n" + 
+                        "\n" + 
+                        "        --\n" + 
+                        "            This option can be used to separate command-line options from the\n" + 
+                        "            list of argument, (useful when arguments might be mistaken for\n" + 
+                        "            command-line options\n" + 
+                        "\n" + 
+                        "        [<keyspace.table>...]\n" + 
+                        "            List of tables (or keyspace) names\n" + 
+                        "\n" + 
+                        "\n";
+        Assertions.assertThat(tool.getStdout()).isEqualTo(help);
+    }
+
+    @Test
+    public void testTableStats()
+    {
+        ToolResult tool = ToolRunner.invokeNodetool("tablestats");
+
+        assertThat(tool.getStdout(), CoreMatchers.containsString("Keyspace : system_schema"));
+        assertTrue(StringUtils.countMatches(tool.getStdout(), "Table:") > 1);
+        tool.assertOnCleanExit();
+
+        tool = ToolRunner.invokeNodetool("tablestats", "system_distributed");
+        assertThat(tool.getStdout(), CoreMatchers.containsString("Keyspace : system_distributed"));
+        assertThat(tool.getStdout(), CoreMatchers.not(CoreMatchers.containsString("Keyspace : system_schema")));
+        assertTrue(StringUtils.countMatches(tool.getStdout(), "Table:") > 1);
+        tool.assertOnCleanExit();
+    }
+
+    @Test
+    public void testTableIgnoreArg()
+    {
+        ToolResult tool = ToolRunner.invokeNodetool("tablestats", "-i", "system_schema.aggregates");
+
+        assertThat(tool.getStdout(), CoreMatchers.containsString("Keyspace : system_schema"));
+        assertThat(tool.getStdout(), CoreMatchers.not(CoreMatchers.containsString("Table: system_schema.aggregates")));
+        assertTrue(StringUtils.countMatches(tool.getStdout(), "Table:") > 1);
+        tool.assertOnCleanExit();
+    }
+
+    @Test
+    public void testHumanReadableArg()
+    {
+        Arrays.asList("-H", "--human-readable").forEach(arg -> {
+            ToolResult tool = ToolRunner.invokeNodetool("tablestats", arg);
+            assertThat("Arg: [" + arg + "]", tool.getStdout(), CoreMatchers.containsString(" KiB"));
+            assertTrue(String.format("Expected empty stderr for option [%s] but found: %s",
+                                     arg,
+                                     tool.getCleanedStderr()),
+                       tool.getCleanedStderr().isEmpty());
+            assertEquals(String.format("Expected exit code 0 for option [%s] but found: %s", arg, tool.getExitCode()),
+                         0,
+                         tool.getExitCode());
+        });
+    }
+
+    @Test
+    public void testSortArg()
+    {
+        Pattern regExp = Pattern.compile("((?m)Table: .*$)");
+
+        Arrays.asList("-s", "--sort").forEach(arg -> {
+            ToolResult tool = ToolRunner.invokeNodetool("tablestats", arg, "table_name");
+            Matcher m = regExp.matcher(tool.getStdout());
+            ArrayList<String> orig = new ArrayList<>();
+            while (m.find())
+                orig.add(m.group(1));
+
+            tool = ToolRunner.invokeNodetool("tablestats", arg, "sstable_count");
+            m = regExp.matcher(tool.getStdout());
+            ArrayList<String> sorted = new ArrayList<>();
+            while (m.find())
+                sorted.add(m.group(1));
+
+            assertNotEquals("Arg: [" + arg + "]", orig, sorted);
+            Collections.sort(orig);
+            Collections.sort(sorted);
+            assertEquals("Arg: [" + arg + "]", orig, sorted);
+            assertTrue("Arg: [" + arg + "]", tool.getCleanedStderr().isEmpty());
+            assertEquals(0, tool.getExitCode());
+        });
+
+        ToolResult tool = ToolRunner.invokeNodetool("tablestats", "-s", "wrongSort");
+        assertThat(tool.getStdout(), CoreMatchers.containsString("argument for sort must be one of"));
+        assertTrue(tool.getCleanedStderr().isEmpty());
+        assertEquals(1, tool.getExitCode());
+    }
+
+    @Test
+    public void testTopArg()
+    {
+        Arrays.asList("-t", "--top").forEach(arg -> {
+            ToolResult tool = ToolRunner.invokeNodetool("tablestats", "-s", "table_name", arg, "1");
+            assertEquals("Arg: [" + arg + "]", StringUtils.countMatches(tool.getStdout(), "Table:"), 1);
+            assertTrue("Arg: [" + arg + "]", tool.getCleanedStderr().isEmpty());
+            assertEquals("Arg: [" + arg + "]", 0, tool.getExitCode());
+        });
+
+        ToolResult tool = ToolRunner.invokeNodetool("tablestats", "-s", "table_name", "-t", "-1");
+        assertThat(tool.getStdout(), CoreMatchers.containsString("argument for top must be a positive integer"));
+        assertTrue(tool.getCleanedStderr().isEmpty());

Review comment:
       ```suggestion
           tool.assertCleanStdErr();
   ```

##########
File path: test/unit/org/apache/cassandra/tools/NodetoolNetStatsTest.java
##########
@@ -0,0 +1,169 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.tools;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.PrintStream;
+import java.nio.charset.StandardCharsets;
+import java.util.Collections;
+import java.util.List;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.apache.cassandra.OrderedJUnit4ClassRunner;
+import org.apache.cassandra.cql3.CQLTester;
+import org.apache.cassandra.locator.InetAddressAndPort;
+import org.apache.cassandra.net.Message;
+import org.apache.cassandra.net.MessagingService;
+import org.apache.cassandra.net.NoPayload;
+import org.apache.cassandra.schema.TableId;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.streaming.SessionInfo;
+import org.apache.cassandra.streaming.StreamSession.State;
+import org.apache.cassandra.streaming.StreamSummary;
+import org.apache.cassandra.tools.ToolRunner.ToolResult;
+import org.apache.cassandra.tools.nodetool.NetStats;
+import org.apache.cassandra.utils.FBUtilities;
+import org.assertj.core.api.Assertions;
+import org.hamcrest.CoreMatchers;
+
+import static org.apache.cassandra.net.Verb.ECHO_REQ;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+@RunWith(OrderedJUnit4ClassRunner.class)
+public class NodetoolNetStatsTest extends CQLTester
+{
+    private static NodeProbe probe;
+
+    @BeforeClass
+    public static void setup() throws Exception
+    {
+        StorageService.instance.initServer();
+        startJMXServer();
+        probe = new NodeProbe(jmxHost, jmxPort);
+    }
+
+    @AfterClass
+    public static void teardown() throws IOException
+    {
+        probe.close();
+    }
+
+    @Test
+    public void testMaybeChangeDocs()
+    {
+        // If you added, modified options or help, please update docs if necessary
+        ToolResult tool = ToolRunner.invokeNodetool("help", "netstats");
+        String help =   "NAME\n" + 
+                        "        nodetool netstats - Print network information on provided host\n" + 
+                        "        (connecting node by default)\n" + 
+                        "\n" + 
+                        "SYNOPSIS\n" + 
+                        "        nodetool [(-h <host> | --host <host>)] [(-p <port> | --port <port>)]\n" + 
+                        "                [(-pp | --print-port)] [(-pw <password> | --password <password>)]\n" + 
+                        "                [(-pwf <passwordFilePath> | --password-file <passwordFilePath>)]\n" + 
+                        "                [(-u <username> | --username <username>)] netstats\n" + 
+                        "                [(-H | --human-readable)]\n" + 
+                        "\n" + 
+                        "OPTIONS\n" + 
+                        "        -h <host>, --host <host>\n" + 
+                        "            Node hostname or ip address\n" + 
+                        "\n" + 
+                        "        -H, --human-readable\n" + 
+                        "            Display bytes in human readable form, i.e. KiB, MiB, GiB, TiB\n" + 
+                        "\n" + 
+                        "        -p <port>, --port <port>\n" + 
+                        "            Remote jmx agent port number\n" + 
+                        "\n" + 
+                        "        -pp, --print-port\n" + 
+                        "            Operate in 4.0 mode with hosts disambiguated by port number\n" + 
+                        "\n" + 
+                        "        -pw <password>, --password <password>\n" + 
+                        "            Remote jmx agent password\n" + 
+                        "\n" + 
+                        "        -pwf <passwordFilePath>, --password-file <passwordFilePath>\n" + 
+                        "            Path to the JMX password file\n" + 
+                        "\n" + 
+                        "        -u <username>, --username <username>\n" + 
+                        "            Remote jmx agent username\n" + 
+                        "\n" + 
+                        "\n";
+        Assertions.assertThat(tool.getStdout()).isEqualTo(help);

Review comment:
       ```suggestion
           Assertions.assertThat(tool.getStdout()).isEqualTo(help);
           tool.assertOnCleanExit();
   ```

##########
File path: test/unit/org/apache/cassandra/tools/NodetoolNetStatsTest.java
##########
@@ -0,0 +1,169 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.tools;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.PrintStream;
+import java.nio.charset.StandardCharsets;
+import java.util.Collections;
+import java.util.List;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.apache.cassandra.OrderedJUnit4ClassRunner;
+import org.apache.cassandra.cql3.CQLTester;
+import org.apache.cassandra.locator.InetAddressAndPort;
+import org.apache.cassandra.net.Message;
+import org.apache.cassandra.net.MessagingService;
+import org.apache.cassandra.net.NoPayload;
+import org.apache.cassandra.schema.TableId;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.streaming.SessionInfo;
+import org.apache.cassandra.streaming.StreamSession.State;
+import org.apache.cassandra.streaming.StreamSummary;
+import org.apache.cassandra.tools.ToolRunner.ToolResult;
+import org.apache.cassandra.tools.nodetool.NetStats;
+import org.apache.cassandra.utils.FBUtilities;
+import org.assertj.core.api.Assertions;
+import org.hamcrest.CoreMatchers;
+
+import static org.apache.cassandra.net.Verb.ECHO_REQ;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;

Review comment:
       Nit: unused import

##########
File path: test/unit/org/apache/cassandra/tools/nodetool/stats/NodetoolTableStatsTest.java
##########
@@ -0,0 +1,239 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.tools.nodetool.stats;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.commons.lang3.StringUtils;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.apache.cassandra.OrderedJUnit4ClassRunner;
+import org.apache.cassandra.cql3.CQLTester;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.tools.NodeProbe;
+import org.apache.cassandra.tools.ToolRunner;
+import org.apache.cassandra.tools.ToolRunner.ToolResult;
+import org.assertj.core.api.Assertions;
+import org.hamcrest.CoreMatchers;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+@RunWith(OrderedJUnit4ClassRunner.class)
+public class NodetoolTableStatsTest extends CQLTester
+{
+    private static NodeProbe probe;
+
+    @BeforeClass
+    public static void setup() throws Exception
+    {
+        StorageService.instance.initServer();
+        startJMXServer();
+        probe = new NodeProbe(jmxHost, jmxPort);
+    }
+
+    @AfterClass
+    public static void teardown() throws IOException
+    {
+        probe.close();
+    }
+
+    @Test
+    public void testMaybeChangeDocs()
+    {
+        // If you added, modified options or help, please update docs if necessary
+        ToolResult tool = ToolRunner.invokeNodetool("help", "tablestats");
+        String help =   "NAME\n" +
+                        "        nodetool tablestats - Print statistics on tables\n" + 
+                        "\n" + 
+                        "SYNOPSIS\n" + 
+                        "        nodetool [(-h <host> | --host <host>)] [(-p <port> | --port <port>)]\n" + 
+                        "                [(-pp | --print-port)] [(-pw <password> | --password <password>)]\n" + 
+                        "                [(-pwf <passwordFilePath> | --password-file <passwordFilePath>)]\n" + 
+                        "                [(-u <username> | --username <username>)] tablestats\n" + 
+                        "                [(-F <format> | --format <format>)] [(-H | --human-readable)] [-i]\n" + 
+                        "                [(-s <sort_key> | --sort <sort_key>)] [(-t <top> | --top <top>)] [--]\n" + 
+                        "                [<keyspace.table>...]\n" + 
+                        "\n" + 
+                        "OPTIONS\n" + 
+                        "        -F <format>, --format <format>\n" + 
+                        "            Output format (json, yaml)\n" + 
+                        "\n" + 
+                        "        -h <host>, --host <host>\n" + 
+                        "            Node hostname or ip address\n" + 
+                        "\n" + 
+                        "        -H, --human-readable\n" + 
+                        "            Display bytes in human readable form, i.e. KiB, MiB, GiB, TiB\n" + 
+                        "\n" + 
+                        "        -i\n" + 
+                        "            Ignore the list of tables and display the remaining tables\n" + 
+                        "\n" + 
+                        "        -p <port>, --port <port>\n" + 
+                        "            Remote jmx agent port number\n" + 
+                        "\n" + 
+                        "        -pp, --print-port\n" + 
+                        "            Operate in 4.0 mode with hosts disambiguated by port number\n" + 
+                        "\n" + 
+                        "        -pw <password>, --password <password>\n" + 
+                        "            Remote jmx agent password\n" + 
+                        "\n" + 
+                        "        -pwf <passwordFilePath>, --password-file <passwordFilePath>\n" + 
+                        "            Path to the JMX password file\n" + 
+                        "\n" + 
+                        "        -s <sort_key>, --sort <sort_key>\n" + 
+                        "            Sort tables by specified sort key\n" + 
+                        "            (average_live_cells_per_slice_last_five_minutes,\n" + 
+                        "            average_tombstones_per_slice_last_five_minutes,\n" + 
+                        "            bloom_filter_false_positives, bloom_filter_false_ratio,\n" + 
+                        "            bloom_filter_off_heap_memory_used, bloom_filter_space_used,\n" + 
+                        "            compacted_partition_maximum_bytes, compacted_partition_mean_bytes,\n" + 
+                        "            compacted_partition_minimum_bytes,\n" + 
+                        "            compression_metadata_off_heap_memory_used, dropped_mutations,\n" + 
+                        "            full_name, index_summary_off_heap_memory_used, local_read_count,\n" + 
+                        "            local_read_latency_ms, local_write_latency_ms,\n" + 
+                        "            maximum_live_cells_per_slice_last_five_minutes,\n" + 
+                        "            maximum_tombstones_per_slice_last_five_minutes, memtable_cell_count,\n" + 
+                        "            memtable_data_size, memtable_off_heap_memory_used,\n" + 
+                        "            memtable_switch_count, number_of_partitions_estimate,\n" + 
+                        "            off_heap_memory_used_total, pending_flushes, percent_repaired,\n" + 
+                        "            read_latency, reads, space_used_by_snapshots_total, space_used_live,\n" + 
+                        "            space_used_total, sstable_compression_ratio, sstable_count,\n" + 
+                        "            table_name, write_latency, writes)\n" + 
+                        "\n" + 
+                        "        -t <top>, --top <top>\n" + 
+                        "            Show only the top K tables for the sort key (specify the number K of\n" + 
+                        "            tables to be shown\n" + 
+                        "\n" + 
+                        "        -u <username>, --username <username>\n" + 
+                        "            Remote jmx agent username\n" + 
+                        "\n" + 
+                        "        --\n" + 
+                        "            This option can be used to separate command-line options from the\n" + 
+                        "            list of argument, (useful when arguments might be mistaken for\n" + 
+                        "            command-line options\n" + 
+                        "\n" + 
+                        "        [<keyspace.table>...]\n" + 
+                        "            List of tables (or keyspace) names\n" + 
+                        "\n" + 
+                        "\n";
+        Assertions.assertThat(tool.getStdout()).isEqualTo(help);

Review comment:
       Just in case:
   ```suggestion
           Assertions.assertThat(tool.getStdout()).isEqualTo(help);
           tool.assertOnCleanExit();
   ```

##########
File path: test/unit/org/apache/cassandra/tools/NodetoolNetStatsTest.java
##########
@@ -0,0 +1,169 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.tools;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.PrintStream;
+import java.nio.charset.StandardCharsets;
+import java.util.Collections;
+import java.util.List;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.apache.cassandra.OrderedJUnit4ClassRunner;
+import org.apache.cassandra.cql3.CQLTester;
+import org.apache.cassandra.locator.InetAddressAndPort;
+import org.apache.cassandra.net.Message;
+import org.apache.cassandra.net.MessagingService;
+import org.apache.cassandra.net.NoPayload;
+import org.apache.cassandra.schema.TableId;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.streaming.SessionInfo;
+import org.apache.cassandra.streaming.StreamSession.State;
+import org.apache.cassandra.streaming.StreamSummary;
+import org.apache.cassandra.tools.ToolRunner.ToolResult;
+import org.apache.cassandra.tools.nodetool.NetStats;
+import org.apache.cassandra.utils.FBUtilities;
+import org.assertj.core.api.Assertions;
+import org.hamcrest.CoreMatchers;
+
+import static org.apache.cassandra.net.Verb.ECHO_REQ;
+import static org.junit.Assert.assertEquals;

Review comment:
       Nit: unused import




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


[GitHub] [cassandra] adelapena commented on a change in pull request #788: CASSANDRA-16227 Add netstats and tablestats unit tests

Posted by GitBox <gi...@apache.org>.
adelapena commented on a change in pull request #788:
URL: https://github.com/apache/cassandra/pull/788#discussion_r514211047



##########
File path: test/unit/org/apache/cassandra/tools/nodetool/stats/NodetoolTableStatsTest.java
##########
@@ -0,0 +1,237 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.tools.nodetool.stats;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.commons.lang3.StringUtils;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.apache.cassandra.OrderedJUnit4ClassRunner;
+import org.apache.cassandra.cql3.CQLTester;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.tools.NodeProbe;
+import org.apache.cassandra.tools.ToolRunner;
+import org.apache.cassandra.tools.ToolRunner.ToolResult;
+import org.assertj.core.api.Assertions;
+import org.hamcrest.CoreMatchers;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+@RunWith(OrderedJUnit4ClassRunner.class)
+public class NodetoolTableStatsTest extends CQLTester
+{
+    private static NodeProbe probe;
+
+    @BeforeClass
+    public static void setup() throws Exception
+    {
+        StorageService.instance.initServer();
+        startJMXServer();
+        probe = new NodeProbe(jmxHost, jmxPort);
+    }
+
+    @AfterClass
+    public static void teardown() throws IOException
+    {
+        probe.close();
+    }
+
+    @Test
+    public void testMaybeChangeDocs()
+    {
+        // If you added, modified options or help, please update docs if necessary
+        ToolResult tool = ToolRunner.invokeNodetool("help", "tablestats");
+        String help =   "NAME\n" +
+                        "        nodetool tablestats - Print statistics on tables\n" + 
+                        "\n" + 
+                        "SYNOPSIS\n" + 
+                        "        nodetool [(-h <host> | --host <host>)] [(-p <port> | --port <port>)]\n" + 
+                        "                [(-pp | --print-port)] [(-pw <password> | --password <password>)]\n" + 
+                        "                [(-pwf <passwordFilePath> | --password-file <passwordFilePath>)]\n" + 
+                        "                [(-u <username> | --username <username>)] tablestats\n" + 
+                        "                [(-F <format> | --format <format>)] [(-H | --human-readable)] [-i]\n" + 
+                        "                [(-s <sort_key> | --sort <sort_key>)] [(-t <top> | --top <top>)] [--]\n" + 
+                        "                [<keyspace.table>...]\n" + 
+                        "\n" + 
+                        "OPTIONS\n" + 
+                        "        -F <format>, --format <format>\n" + 
+                        "            Output format (json, yaml)\n" + 
+                        "\n" + 
+                        "        -h <host>, --host <host>\n" + 
+                        "            Node hostname or ip address\n" + 
+                        "\n" + 
+                        "        -H, --human-readable\n" + 
+                        "            Display bytes in human readable form, i.e. KiB, MiB, GiB, TiB\n" + 
+                        "\n" + 
+                        "        -i\n" + 
+                        "            Ignore the list of tables and display the remaining tables\n" + 
+                        "\n" + 
+                        "        -p <port>, --port <port>\n" + 
+                        "            Remote jmx agent port number\n" + 
+                        "\n" + 
+                        "        -pp, --print-port\n" + 
+                        "            Operate in 4.0 mode with hosts disambiguated by port number\n" + 
+                        "\n" + 
+                        "        -pw <password>, --password <password>\n" + 
+                        "            Remote jmx agent password\n" + 
+                        "\n" + 
+                        "        -pwf <passwordFilePath>, --password-file <passwordFilePath>\n" + 
+                        "            Path to the JMX password file\n" + 
+                        "\n" + 
+                        "        -s <sort_key>, --sort <sort_key>\n" + 
+                        "            Sort tables by specified sort key\n" + 
+                        "            (average_live_cells_per_slice_last_five_minutes,\n" + 
+                        "            average_tombstones_per_slice_last_five_minutes,\n" + 
+                        "            bloom_filter_false_positives, bloom_filter_false_ratio,\n" + 
+                        "            bloom_filter_off_heap_memory_used, bloom_filter_space_used,\n" + 
+                        "            compacted_partition_maximum_bytes, compacted_partition_mean_bytes,\n" + 
+                        "            compacted_partition_minimum_bytes,\n" + 
+                        "            compression_metadata_off_heap_memory_used, dropped_mutations,\n" + 
+                        "            full_name, index_summary_off_heap_memory_used, local_read_count,\n" + 
+                        "            local_read_latency_ms, local_write_latency_ms,\n" + 
+                        "            maximum_live_cells_per_slice_last_five_minutes,\n" + 
+                        "            maximum_tombstones_per_slice_last_five_minutes, memtable_cell_count,\n" + 
+                        "            memtable_data_size, memtable_off_heap_memory_used,\n" + 
+                        "            memtable_switch_count, number_of_partitions_estimate,\n" + 
+                        "            off_heap_memory_used_total, pending_flushes, percent_repaired,\n" + 
+                        "            read_latency, reads, space_used_by_snapshots_total, space_used_live,\n" + 
+                        "            space_used_total, sstable_compression_ratio, sstable_count,\n" + 
+                        "            table_name, write_latency, writes)\n" + 
+                        "\n" + 
+                        "        -t <top>, --top <top>\n" + 
+                        "            Show only the top K tables for the sort key (specify the number K of\n" + 
+                        "            tables to be shown\n" + 
+                        "\n" + 
+                        "        -u <username>, --username <username>\n" + 
+                        "            Remote jmx agent username\n" + 
+                        "\n" + 
+                        "        --\n" + 
+                        "            This option can be used to separate command-line options from the\n" + 
+                        "            list of argument, (useful when arguments might be mistaken for\n" + 
+                        "            command-line options\n" + 
+                        "\n" + 
+                        "        [<keyspace.table>...]\n" + 
+                        "            List of tables (or keyspace) names\n" + 
+                        "\n" + 
+                        "\n";
+        Assertions.assertThat(tool.getStdout()).isEqualTo(help);
+    }
+
+    @Test
+    public void testTableStats()
+    {
+        ToolResult tool = ToolRunner.invokeNodetool("tablestats");
+
+        assertThat(tool.getStdout(), CoreMatchers.containsStringIgnoringCase("Keyspace : system_schema"));
+        assertTrue(StringUtils.countMatches(tool.getStdout(), "Table:") > 1);
+        assertTrue(tool.getCleanedStderr().isEmpty());
+        assertEquals(0, tool.getExitCode());
+
+        tool = ToolRunner.invokeNodetool("tablestats", "system_distributed");
+        assertThat(tool.getStdout(), CoreMatchers.containsStringIgnoringCase("Keyspace : system_distributed"));
+        assertThat(tool.getStdout(), CoreMatchers.not(CoreMatchers.containsStringIgnoringCase("Keyspace : system_schema")));
+        assertTrue(StringUtils.countMatches(tool.getStdout(), "Table:") > 1);
+        assertTrue(tool.getCleanedStderr().isEmpty());
+        assertEquals(0, tool.getExitCode());
+    }
+
+    @Test
+    public void testTableIgnoreArg()
+    {
+        ToolResult tool = ToolRunner.invokeNodetool("tablestats", "-i", "system_schema.aggregates");
+
+        assertThat(tool.getStdout(), CoreMatchers.containsStringIgnoringCase("Keyspace : system_schema"));
+        assertThat(tool.getStdout(), CoreMatchers.not(CoreMatchers.containsStringIgnoringCase("Table: system_schema.aggregates")));
+        assertTrue(StringUtils.countMatches(tool.getStdout(), "Table:") > 1);
+        assertTrue(tool.getCleanedStderr().isEmpty());
+        assertEquals(0, tool.getExitCode());
+    }
+
+    @Test
+    public void testHumanReadableArg()
+    {
+        Arrays.asList("-H", "--human-readable").forEach(arg -> {
+            ToolResult tool = ToolRunner.invokeNodetool("tablestats", arg);
+            assertThat("Arg: [" + arg + "]", tool.getStdout(), CoreMatchers.containsStringIgnoringCase(" KiB"));
+            assertTrue("Arg: [" + arg + "]", tool.getCleanedStderr().isEmpty());
+            assertEquals("Arg: [" + arg + "]", 0, tool.getExitCode());

Review comment:
       You are totally right, I thought that passing the message to `assertThat` was going to override the assertion message, but that's not the case.




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