frankgh commented on code in PR #135: URL: https://github.com/apache/cassandra-analytics/pull/135#discussion_r2252692872
########## cassandra-four-zero-bridge/src/test/java/org/apache/cassandra/spark/data/converter/types/DateTypeTests.java: ########## @@ -38,21 +37,21 @@ public class DateTypeTests public void testDateConversion() { int cassandraDate = SimpleDateSerializer.dateStringToDays("2021-07-16"); - assertTrue(cassandraDate < 0); - assertEquals("2021-07-16", SimpleDateSerializer.instance.toString(cassandraDate)); + assertThat(cassandraDate < 0).isTrue(); Review Comment: ```suggestion assertThat(cassandraDate).isLessThan(0); ``` ########## cassandra-four-zero-bridge/src/test/java/org/apache/cassandra/bridge/SSTableWriterImplementationTest.java: ########## @@ -86,38 +85,38 @@ void testGetProducedSSTables() throws IOException Collections.emptySet(), 1); writer.setSSTablesProducedListener(produced::addAll); - assertTrue(produced.isEmpty()); + assertThat(produced.isEmpty()).isTrue(); Review Comment: ```suggestion assertThat(produced).isEmpty(); ``` ########## cassandra-analytics-core/src/test/java/org/apache/cassandra/spark/data/partitioner/MultipleReplicasTests.java: ########## @@ -162,7 +158,7 @@ private static void runTest(int numInstances, int rfFactor, int numDownPrimaryIn // if insufficient primary or backup replicas available to meet consistency level MultipleReplicas replicas = new MultipleReplicas(primaryReplicas, backupReplicas, Stats.DoNothingStats.INSTANCE); Set<TestSSTableReader> readers = replicas.openAll((ssTable, isRepairPrimary) -> new TestSSTableReader(ssTable)); - assertEquals(expectedSSTables, readers.size()); + assertThat(readers.size()).isEqualTo(expectedSSTables); Review Comment: ```suggestion assertThat(readers).hasSize(expectedSSTables); ``` ########## cassandra-analytics-core/src/test/java/org/apache/cassandra/spark/data/DataLayerUnsupportedPushDownFiltersTest.java: ########## @@ -194,30 +192,30 @@ public void testSchemaWithCompositePartitionKey() // a is part of a composite partition column Filter[] allFilters = {new EqualTo("a", 5)}; Filter[] unsupportedFilters = dataLayer.unsupportedPushDownFilters(allFilters); - assertNotNull(unsupportedFilters); + assertThat(unsupportedFilters).isNotNull(); // Filter push-down is disabled because not all partition columns are in the filter array - assertEquals(1, unsupportedFilters.length); + assertThat(unsupportedFilters).hasSize(1); // a and b are part of a composite partition column allFilters = new Filter[]{new EqualTo("a", 5), new EqualTo("b", 10)}; unsupportedFilters = dataLayer.unsupportedPushDownFilters(allFilters); - assertNotNull(unsupportedFilters); + assertThat(unsupportedFilters).isNotNull(); // Filter push-down is disabled because not all partition columns are in the filter array - assertEquals(2, unsupportedFilters.length); + assertThat(unsupportedFilters).hasSize(2); // a and b are part of a composite partition column, but d is not allFilters = new Filter[]{new EqualTo("a", 5), new EqualTo("b", 10), new EqualTo("d", 20)}; unsupportedFilters = dataLayer.unsupportedPushDownFilters(allFilters); - assertNotNull(unsupportedFilters); + assertThat(unsupportedFilters).isNotNull(); // Filter push-down is disabled because not all partition columns are in the filter array - assertEquals(3, unsupportedFilters.length); + assertThat(unsupportedFilters).hasSize(3); // a and b are part of a composite partition column allFilters = new Filter[]{new EqualTo("a", 5), new EqualTo("b", 10), new EqualTo("c", 15)}; unsupportedFilters = dataLayer.unsupportedPushDownFilters(allFilters); - assertNotNull(unsupportedFilters); + assertThat(unsupportedFilters).isNotNull(); // Filter push-down is enabled because all the partition columns are part of the filter array - assertEquals(0, unsupportedFilters.length); + assertThat(unsupportedFilters).hasSize(0); Review Comment: ```suggestion assertThat(unsupportedFilters).isEmpty(); ``` ########## cassandra-analytics-core/src/test/java/org/apache/cassandra/spark/bulkwriter/RecordWriterTest.java: ########## @@ -123,10 +117,10 @@ void symmetricDifferenceTest() Set<Integer> s3 = new HashSet<>(Arrays.asList(5, 6, 7)); Set<Integer> s4 = new HashSet<>(Arrays.asList(5, 6, 7)); Set<Integer> s5 = new HashSet<>(); - assertThat(RecordWriter.symmetricDifference(s1, s2), is(new HashSet<>(Arrays.asList(1, 4)))); - assertThat(RecordWriter.symmetricDifference(s2, s3), is(new HashSet<>(Arrays.asList(2, 3, 4, 5, 6, 7)))); - assertThat(RecordWriter.symmetricDifference(s3, s4), empty()); - assertThat(RecordWriter.symmetricDifference(s4, s5), is(s4)); + assertThat(RecordWriter.symmetricDifference(s1, s2)).isEqualTo(new HashSet<>(Arrays.asList(1, 4))); + assertThat(RecordWriter.symmetricDifference(s2, s3)).isEqualTo(new HashSet<>(Arrays.asList(2, 3, 4, 5, 6, 7))); Review Comment: ```suggestion assertThat(RecordWriter.symmetricDifference(s1, s2)).containsExactly(1,4); assertThat(RecordWriter.symmetricDifference(s2, s3)).containsExactly(2, 3, 4, 5, 6, 7); ``` ########## cassandra-analytics-core/src/test/java/org/apache/cassandra/spark/bulkwriter/cloudstorage/SSTablesBundlerTest.java: ########## @@ -141,15 +140,15 @@ void testManifestWritten() throws IOException, URISyntaxException + "}"; Path bundle0Manifest = outputDir.resolve("0").resolve("manifest.json"); Path bundle1Manifest = outputDir.resolve("1").resolve("manifest.json"); - assertTrue(Files.exists(bundle0Manifest)); - assertTrue(Files.exists(bundle1Manifest)); + assertThat(Files.exists(bundle0Manifest)).isTrue(); + assertThat(Files.exists(bundle1Manifest)).isTrue(); Review Comment: ```suggestion assertThat(bundle0Manifest).exists(); assertThat(bundle1Manifest).exists(); ``` ########## cassandra-analytics-core/src/test/java/org/apache/cassandra/spark/utils/FilterUtilsTests.java: ########## @@ -31,55 +31,51 @@ import org.apache.spark.sql.sources.Filter; import org.apache.spark.sql.sources.In; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; public class FilterUtilsTests { @Test() public void testPartialPartitionKeyFilter() { Filter[] filters = new Filter[]{new EqualTo("a", "1")}; - assertThrows(IllegalArgumentException.class, - () -> FilterUtils.extractPartitionKeyValues(filters, new HashSet<>(Arrays.asList("a", "b"))) - ); + assertThatThrownBy(() -> FilterUtils.extractPartitionKeyValues(filters, new HashSet<>(Arrays.asList("a", "b")))) + .isInstanceOf(IllegalArgumentException.class); } @Test public void testValidPartitionKeyValuesExtracted() { Filter[] filters = new Filter[]{new EqualTo("a", "1"), new In("b", new String[]{"2", "3"}), new EqualTo("c", "2")}; Map<String, List<String>> partitionKeyValues = FilterUtils.extractPartitionKeyValues(filters, new HashSet<>(Arrays.asList("a", "b"))); - assertFalse(partitionKeyValues.containsKey("c")); - assertTrue(partitionKeyValues.containsKey("a")); - assertTrue(partitionKeyValues.containsKey("b")); + assertThat(partitionKeyValues.containsKey("c")).isFalse(); + assertThat(partitionKeyValues.containsKey("a")).isTrue(); + assertThat(partitionKeyValues.containsKey("b")).isTrue(); } @Test() public void testCartesianProductOfInValidValues() { List<List<String>> orderedValues = Arrays.asList(Arrays.asList("1", "2"), Arrays.asList("a", "b", "c"), Collections.emptyList()); - assertThrows(IllegalArgumentException.class, - () -> FilterUtils.cartesianProduct(orderedValues) - ); + assertThatThrownBy(() -> FilterUtils.cartesianProduct(orderedValues)) + .isInstanceOf(IllegalArgumentException.class); } @Test public void testCartesianProductOfEmptyList() { List<List<String>> orderedValues = Collections.emptyList(); List<List<String>> product = FilterUtils.cartesianProduct(orderedValues); - assertFalse(product.isEmpty()); - assertEquals(1, product.size()); - assertTrue(product.get(0).isEmpty()); + assertThat(product.isEmpty()).isFalse(); + assertThat(product).hasSize(1); + assertThat(product.get(0).isEmpty()).isTrue(); Review Comment: ```suggestion assertThat(product).isNotEmpty(); assertThat(product).hasSize(1); assertThat(product.get(0)).isEmpty(); ``` ########## cassandra-four-zero-bridge/src/test/java/org/apache/cassandra/bridge/SSTableWriterImplementationTest.java: ########## @@ -86,38 +85,38 @@ void testGetProducedSSTables() throws IOException Collections.emptySet(), 1); writer.setSSTablesProducedListener(produced::addAll); - assertTrue(produced.isEmpty()); + assertThat(produced.isEmpty()).isTrue(); for (int i = 0; i < 300_000; i++) { writer.addRow(ImmutableMap.of("a", i, "b", "val_" + i)); } - assertEquals(2, produced.size()); + assertThat(produced.size()).isEqualTo(2); Review Comment: ```suggestion assertThat(produced).hasSize(2); ``` ########## cassandra-analytics-core/src/test/java/org/apache/cassandra/spark/data/CqlFieldComparatorTests.java: ########## Review Comment: a lot of comparisons in this PR should use `isLessThan(0)` | `isGreaterThan(0)` instead of `result < 0).isTrue()` | `result > 0).isTrue()`. ########## cassandra-analytics-core/src/test/java/org/apache/cassandra/spark/utils/FilterUtilsTests.java: ########## @@ -31,55 +31,51 @@ import org.apache.spark.sql.sources.Filter; import org.apache.spark.sql.sources.In; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; public class FilterUtilsTests { @Test() public void testPartialPartitionKeyFilter() { Filter[] filters = new Filter[]{new EqualTo("a", "1")}; - assertThrows(IllegalArgumentException.class, - () -> FilterUtils.extractPartitionKeyValues(filters, new HashSet<>(Arrays.asList("a", "b"))) - ); + assertThatThrownBy(() -> FilterUtils.extractPartitionKeyValues(filters, new HashSet<>(Arrays.asList("a", "b")))) + .isInstanceOf(IllegalArgumentException.class); } @Test public void testValidPartitionKeyValuesExtracted() { Filter[] filters = new Filter[]{new EqualTo("a", "1"), new In("b", new String[]{"2", "3"}), new EqualTo("c", "2")}; Map<String, List<String>> partitionKeyValues = FilterUtils.extractPartitionKeyValues(filters, new HashSet<>(Arrays.asList("a", "b"))); - assertFalse(partitionKeyValues.containsKey("c")); - assertTrue(partitionKeyValues.containsKey("a")); - assertTrue(partitionKeyValues.containsKey("b")); + assertThat(partitionKeyValues.containsKey("c")).isFalse(); + assertThat(partitionKeyValues.containsKey("a")).isTrue(); + assertThat(partitionKeyValues.containsKey("b")).isTrue(); Review Comment: ```suggestion assertThat(partitionKeyValues).containsKey("c"); assertThat(partitionKeyValues).containsKey("a"); assertThat(partitionKeyValues).containsKey("b"); ``` ########## analytics-sidecar-client-common/src/test/java/org/apache/cassandra/sidecar/common/utils/TimeUtilsTest.java: ########## @@ -55,8 +54,8 @@ public void testRandomDurationWithMinimumBelowMaximum() Duration maximum = Duration.ofSeconds(test + 1); Duration random = TimeUtils.randomDuration(minimum, maximum); - assertTrue(minimum.compareTo(random) <= 0); - assertTrue(random.compareTo(maximum) <= 0); + assertThat(minimum.compareTo(random) <= 0).isTrue(); + assertThat(random.compareTo(maximum) <= 0).isTrue(); Review Comment: "You" missed this one ```suggestion assertThat(minimum.compareTo(random)).isLessThanOrEqualTo(0); assertThat(random.compareTo(maximum)).isLessThanOrEqualTo(0); ``` ########## cassandra-four-zero-bridge/src/test/java/org/apache/cassandra/spark/data/converter/types/DateTypeTests.java: ########## @@ -38,21 +37,21 @@ public class DateTypeTests public void testDateConversion() { int cassandraDate = SimpleDateSerializer.dateStringToDays("2021-07-16"); - assertTrue(cassandraDate < 0); - assertEquals("2021-07-16", SimpleDateSerializer.instance.toString(cassandraDate)); + assertThat(cassandraDate < 0).isTrue(); + assertThat(SimpleDateSerializer.instance.toString(cassandraDate)).isEqualTo("2021-07-16"); Object sparkSqlDate = SparkDate.INSTANCE.toSparkSqlType(cassandraDate, false); - assertTrue(sparkSqlDate instanceof Integer); + assertThat(sparkSqlDate instanceof Integer).isTrue(); int numDays = (int) sparkSqlDate; - assertTrue(numDays > 0); + assertThat(numDays > 0).isTrue(); Review Comment: ```suggestion assertThat(numDays).isGreaterThan(0); ``` ########## cassandra-analytics-core/src/test/java/org/apache/cassandra/spark/utils/IOUtilsTest.java: ########## @@ -53,25 +51,25 @@ void testZip() throws Exception } File targetZip = tempFolder.resolve("zip").toFile(); long zipFileSize = IOUtils.zip(zipSourceDir.toPath(), targetZip.toPath()); - assertTrue(targetZip.exists()); - assertTrue(zipFileSize > 0); + assertThat(targetZip.exists()).isTrue(); + assertThat(zipFileSize > 0).isTrue(); Review Comment: ```suggestion assertThat(targetZip).exists(); assertThat(zipFileSize).isGreaterThan(0); ``` ########## cassandra-analytics-core/src/test/java/org/apache/cassandra/spark/utils/BufferingInputStreamHttpTest.java: ########## @@ -280,7 +279,7 @@ private void runHttpTest(long size, Long maxBufferSize, Long chunkBufferSize) } } byte[] expectedMD5 = digest.digest(); - assertEquals(size, Files.size(path)); + assertThat(Files.size(path)).isEqualTo(size); Review Comment: ```suggestion assertThat(path).hasSize(size); ``` ########## cassandra-analytics-core/src/test/java/org/apache/cassandra/spark/bulkwriter/cloudstorage/SSTablesBundlerTest.java: ########## @@ -74,16 +73,16 @@ void testNumberOfBundlesGenerated() throws IOException, URISyntaxException ssTablesBundler.includeDirectory(outputDir); ssTablesBundler.finish(); List<Bundle> bundles = ImmutableList.copyOf(ssTablesBundler); - assertEquals(2, bundles.size()); + assertThat(bundles).hasSize(2); Path bundle0 = outputDir.resolve("0"); Path bundle1 = outputDir.resolve("1"); - assertTrue(Files.exists(bundle0) && Files.isDirectory(bundle0)); - assertTrue(Files.exists(bundle1) && Files.isDirectory(bundle1)); + assertThat(Files.exists(bundle0) && Files.isDirectory(bundle0)).isTrue(); + assertThat(Files.exists(bundle1) && Files.isDirectory(bundle1)).isTrue(); Review Comment: ```suggestion assertThat(bundle0).exists().isDirectory(); assertThat(bundle1).exists().isDirectory(); ``` ########## cassandra-analytics-core/src/test/java/org/apache/cassandra/spark/bulkwriter/StreamSessionConsistencyTest.java: ########## @@ -139,12 +135,12 @@ public void testConsistencyLevelAndFailureInCommit(ConsistencyLevel.CL consisten executor.assertFuturesCalled(); assertThat(writerContext.getUploads().values().stream() .mapToInt(Collection::size) - .sum(), - equalTo(REPLICATION_FACTOR * NUMBER_DCS * FILES_PER_SSTABLE)); + .sum()) + .isEqualTo(REPLICATION_FACTOR * NUMBER_DCS * FILES_PER_SSTABLE); List<String> instances = writerContext.getUploads().keySet().stream() .map(CassandraInstance::nodeName) .collect(Collectors.toList()); - assertThat(instances, containsInAnyOrder(EXPECTED_INSTANCES.toArray())); + assertThat(instances).containsExactlyInAnyOrder(EXPECTED_INSTANCES.toArray(new String[0])); Review Comment: ```suggestion assertThat(instances).containsAll(EXPECTED_INSTANCES); ``` ########## cassandra-analytics-core/src/test/java/org/apache/cassandra/spark/bulkwriter/cloudstorage/SSTablesBundlerTest.java: ########## @@ -74,16 +73,16 @@ void testNumberOfBundlesGenerated() throws IOException, URISyntaxException ssTablesBundler.includeDirectory(outputDir); ssTablesBundler.finish(); List<Bundle> bundles = ImmutableList.copyOf(ssTablesBundler); - assertEquals(2, bundles.size()); + assertThat(bundles).hasSize(2); Path bundle0 = outputDir.resolve("0"); Path bundle1 = outputDir.resolve("1"); - assertTrue(Files.exists(bundle0) && Files.isDirectory(bundle0)); - assertTrue(Files.exists(bundle1) && Files.isDirectory(bundle1)); + assertThat(Files.exists(bundle0) && Files.isDirectory(bundle0)).isTrue(); + assertThat(Files.exists(bundle1) && Files.isDirectory(bundle1)).isTrue(); String expectedZippedBundlePath1 = "b_" + jobId + "_" + sessionId + "_1_3"; String expectedZippedBundlePath2 = "e_" + jobId + "_" + sessionId + "_4_6"; - assertTrue(Files.exists(outputDir.resolve(expectedZippedBundlePath1))); - assertTrue(Files.exists(outputDir.resolve(expectedZippedBundlePath2))); + assertThat(Files.exists(outputDir.resolve(expectedZippedBundlePath1))).isTrue(); + assertThat(Files.exists(outputDir.resolve(expectedZippedBundlePath2))).isTrue(); Review Comment: ```suggestion assertThat(outputDir.resolve(expectedZippedBundlePath1)).exists(); assertThat(outputDir.resolve(expectedZippedBundlePath2)).exists(); ``` ########## cassandra-analytics-core/src/test/java/org/apache/cassandra/spark/data/CqlFieldComparatorTests.java: ########## @@ -268,9 +267,9 @@ public void testByteComparator(CassandraBridge bridge) { byte byte1 = 101; byte byte2 = 102; - assertTrue(toSparkType(bridge, bridge.tinyint()).compare(byte1, byte2) < 0); - assertEquals(0, toSparkType(bridge, bridge.tinyint()).compare(byte1, byte1)); - assertEquals(0, toSparkType(bridge, bridge.tinyint()).compare(byte2, byte2)); - assertTrue(toSparkType(bridge, bridge.tinyint()).compare(byte2, byte1) > 0); + assertThat(toSparkType(bridge, bridge.tinyint()).compare(byte1, byte2) < 0).isTrue(); + assertThat(toSparkType(bridge, bridge.tinyint()).compare(byte1, byte1)).isEqualTo(0); + assertThat(toSparkType(bridge, bridge.tinyint()).compare(byte2, byte2)).isEqualTo(0); + assertThat(toSparkType(bridge, bridge.tinyint()).compare(byte2, byte1) > 0).isTrue(); Review Comment: ```suggestion assertThat(toSparkType(bridge, bridge.tinyint()).compare(byte1, byte2)).isLessThan(0); assertThat(toSparkType(bridge, bridge.tinyint()).compare(byte1, byte1)).isEqualTo(0); assertThat(toSparkType(bridge, bridge.tinyint()).compare(byte2, byte2)).isEqualTo(0); assertThat(toSparkType(bridge, bridge.tinyint()).compare(byte2, byte1)).isGreaterThan(0); ``` ########## cassandra-analytics-core/src/test/java/org/apache/cassandra/spark/bulkwriter/StreamSessionConsistencyTest.java: ########## @@ -183,12 +178,12 @@ public void testConsistencyLevelAndFailureInUpload(ConsistencyLevel.CL consisten int filesSkipped = (numFailures * (FILES_PER_SSTABLE - 1)); assertThat(writerContext.getUploads().values().stream() .mapToInt(Collection::size) - .sum(), - equalTo(totalFilesToUpload - filesSkipped)); + .sum()) + .isEqualTo(totalFilesToUpload - filesSkipped); List<String> instances = writerContext.getUploads().keySet().stream() .map(CassandraInstance::nodeName) .collect(Collectors.toList()); - assertThat(instances, containsInAnyOrder(EXPECTED_INSTANCES.toArray())); + assertThat(instances).containsExactlyInAnyOrder(EXPECTED_INSTANCES.toArray(new String[0])); Review Comment: ```suggestion assertThat(instances).containsAll(EXPECTED_INSTANCES); ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org