From 36ce2e2a0a4d229d1805766b0e9534b1f708929f Mon Sep 17 00:00:00 2001 From: Alan Paxton Date: Thu, 17 Feb 2022 13:28:09 -0800 Subject: [PATCH] Update build files for java8 build (#9541) Summary: For RocksJava 7 we will move from requiring Java 7 to Java 8. * This simplifies the `Makefile` as we no longer need to deal with Java 7; so we no longer use `javah`. * Added a java-version target which is invoked by the java target, and which exits if the version of java being used is not 8 or greater. * Enforces java 8 as a minimum. * Fixed CMake build. * Fixed broken java event listener test, as the test was broken and the assertions in the callbacks were not causing assertions in the tests. The callbacks now queue up assertion errors for the main thread of the tests to check. * Fixed C++ dangling pointers in the test code. Pull Request resolved: https://github.com/facebook/rocksdb/pull/9541 Reviewed By: pdillinger Differential Revision: D34214929 Pulled By: jay-zhuang fbshipit-source-id: fdff348758d0a23a742e83c87d5f54073ce16ca6 --- build_tools/build_detect_platform | 2 +- java/CMakeLists.txt | 16 +- java/Makefile | 44 +-- java/rocksjni/testable_event_listener.cc | 15 +- .../java/org/rocksdb/EventListenerTest.java | 366 ++++++++---------- 5 files changed, 202 insertions(+), 241 deletions(-) diff --git a/build_tools/build_detect_platform b/build_tools/build_detect_platform index a99f3a368..1d89cba79 100755 --- a/build_tools/build_detect_platform +++ b/build_tools/build_detect_platform @@ -274,7 +274,7 @@ esac PLATFORM_CXXFLAGS="$PLATFORM_CXXFLAGS ${CXXFLAGS}" JAVA_LDFLAGS="$PLATFORM_LDFLAGS" JAVA_STATIC_LDFLAGS="$PLATFORM_LDFLAGS" -JAVAC_ARGS="-source 7" +JAVAC_ARGS="-source 8" if [ "$CROSS_COMPILE" = "true" -o "$FBCODE_BUILD" = "true" ]; then # Cross-compiling; do not try any compilation tests. diff --git a/java/CMakeLists.txt b/java/CMakeLists.txt index b399b1749..872d27911 100644 --- a/java/CMakeLists.txt +++ b/java/CMakeLists.txt @@ -309,13 +309,17 @@ set(JAVA_TESTCLASSPATH ${JAVA_JUNIT_JAR} ${JAVA_HAMCR_JAR} ${JAVA_MOCKITO_JAR} $ set(JNI_OUTPUT_DIR ${PROJECT_SOURCE_DIR}/java/include) file(MAKE_DIRECTORY ${JNI_OUTPUT_DIR}) +if(${Java_VERSION_MINOR} VERSION_LESS_EQUAL "7" AND ${Java_VERSION_MAJOR} STREQUAL "1") + message(FATAL_ERROR "Detected Java 7 or older (${Java_VERSION_STRING}), minimum required version in now Java 8") +endif() + if(${Java_VERSION_MAJOR} VERSION_GREATER_EQUAL "10" AND ${CMAKE_VERSION} VERSION_LESS "3.11.4") # Java 10 and newer don't have javah, but the alternative GENERATE_NATIVE_HEADERS requires CMake 3.11.4 or newer message(FATAL_ERROR "Detected Java 10 or newer (${Java_VERSION_STRING}), to build with CMake please upgrade CMake to 3.11.4 or newer") -elseif(${CMAKE_VERSION} VERSION_LESS "3.11.4" OR (${Java_VERSION_MINOR} STREQUAL "7" AND ${Java_VERSION_MAJOR} STREQUAL "1")) - # Old CMake or Java 1.7 prepare the JAR... - message("Preparing Jar for Java 7") +elseif(${CMAKE_VERSION} VERSION_LESS "3.11.4") + # Old CMake + message("Using an old CMAKE (${CMAKE_VERSION}) - JNI headers generated in separate step") add_jar( rocksdbjni_classes SOURCES @@ -405,9 +409,9 @@ if(NOT EXISTS ${JAVA_ASSERTJ_JAR}) file(RENAME ${JAVA_TMP_JAR} ${JAVA_ASSERTJ_JAR}) endif() -if(${CMAKE_VERSION} VERSION_LESS "3.11.4" OR (${Java_VERSION_MINOR} STREQUAL "7" AND ${Java_VERSION_MAJOR} STREQUAL "1")) - # Old CMake or Java 1.7 ONLY generate JNI headers, Java 1.8+ JNI is handled in add_jar step above - message("Preparing JNI headers for Java 7") +if(${CMAKE_VERSION} VERSION_LESS "3.11.4") + # Old CMake ONLY generate JNI headers, otherwise JNI is handled in add_jar step above + message("Preparing JNI headers for old CMake (${CMAKE_VERSION})") set(NATIVE_JAVA_CLASSES org.rocksdb.AbstractCompactionFilter org.rocksdb.AbstractCompactionFilterFactory diff --git a/java/Makefile b/java/Makefile index 87d513b95..d294c0ce0 100644 --- a/java/Makefile +++ b/java/Makefile @@ -252,14 +252,6 @@ JAVAC_CMD := javac endif endif -ifeq ($(JAVAH_CMD),) -ifneq ($(JAVA_HOME),) -JAVAH_CMD := $(JAVA_HOME)/bin/javah -else -JAVAH_CMD := javah -endif -endif - ifeq ($(JAVADOC_CMD),) ifneq ($(JAVA_HOME),) JAVADOC_CMD := $(JAVA_HOME)/bin/javadoc @@ -268,6 +260,17 @@ JAVADOC_CMD := javadoc endif endif +# Look for the Java version (1.6->6, 1.7->7, 1.8->8, 11.0->11, 13.0->13, 15.0->15 etc..) +JAVAC_VERSION := $(shell $(JAVAC_CMD) -version 2>&1) +JAVAC_MAJOR_VERSION := $(word 2,$(subst ., ,$(JAVAC_VERSION))) +ifeq ($(JAVAC_MAJOR_VERSION),1) +JAVAC_MAJOR_VERSION := $(word 3,$(subst ., ,$(JAVAC_VERSION))) +endif + +# Test whether the version we see meets our minimum +MIN_JAVAC_MAJOR_VERSION := 8 +JAVAC_VERSION_GE_MIN := $(shell [ $(JAVAC_MAJOR_VERSION) -ge $(MIN_JAVAC_MAJOR_VERSION) ] > /dev/null 2>&1 && echo true) + # Set the default JAVA_ARGS to "" for DEBUG_LEVEL=0 JAVA_ARGS ?= @@ -283,6 +286,12 @@ endif # of failing in Travis builds.) DEPS_URL?=https://rocksdb-deps.s3-us-west-2.amazonaws.com/jars +java-version: +ifneq ($(JAVAC_VERSION_GE_MIN),true) + echo 'Java version is $(JAVAC_VERSION), minimum required version is $(MIN_JAVAC_MAJOR_VERSION)' + exit 1 +endif + clean: clean-not-downloaded clean-downloaded clean-not-downloaded: @@ -301,22 +310,13 @@ javadocs: java javalib: java java_test javadocs -java: +java: java-version $(AM_V_GEN)mkdir -p $(MAIN_CLASSES) -ifeq ($(shell $(JAVAC_CMD) -version 2>&1 | grep 1.7.0 > /dev/null; printf $$?), 0) - $(AM_V_at)$(JAVAC_CMD) $(JAVAC_ARGS) -d $(MAIN_CLASSES)\ - $(MAIN_SRC)/org/rocksdb/util/*.java\ - $(MAIN_SRC)/org/rocksdb/*.java -else $(AM_V_at)$(JAVAC_CMD) $(JAVAC_ARGS) -h $(NATIVE_INCLUDE) -d $(MAIN_CLASSES)\ $(MAIN_SRC)/org/rocksdb/util/*.java\ $(MAIN_SRC)/org/rocksdb/*.java -endif $(AM_V_at)@cp ../HISTORY.md ./HISTORY-CPP.md $(AM_V_at)@rm -f ./HISTORY-CPP.md -ifeq ($(shell $(JAVAH_CMD) -version 2>&1 | grep 1.7.0 > /dev/null; printf $$?), 0) - $(AM_V_at)$(JAVAH_CMD) -cp $(MAIN_CLASSES) -d $(NATIVE_INCLUDE) -jni $(NATIVE_JAVA_CLASSES) -endif sample: java $(AM_V_GEN)mkdir -p $(SAMPLES_MAIN_CLASSES) @@ -415,18 +415,10 @@ resolve_test_deps: $(JAVA_JUNIT_JAR_PATH) $(JAVA_HAMCREST_JAR_PATH) $(JAVA_MOCKI java_test: java resolve_test_deps $(AM_V_GEN)mkdir -p $(TEST_CLASSES) -ifeq ($(shell $(JAVAC_CMD) -version 2>&1|grep 1.7.0 >/dev/null; printf $$?),0) - $(AM_V_at)$(JAVAC_CMD) $(JAVAC_ARGS) -cp $(MAIN_CLASSES):$(JAVA_TESTCLASSPATH) -d $(TEST_CLASSES)\ - $(TEST_SRC)/org/rocksdb/test/*.java\ - $(TEST_SRC)/org/rocksdb/util/*.java\ - $(TEST_SRC)/org/rocksdb/*.java - $(AM_V_at)$(JAVAH_CMD) -cp $(MAIN_CLASSES):$(TEST_CLASSES) -d $(NATIVE_INCLUDE) -jni $(NATIVE_JAVA_TEST_CLASSES) -else $(AM_V_at)$(JAVAC_CMD) $(JAVAC_ARGS) -cp $(MAIN_CLASSES):$(JAVA_TESTCLASSPATH) -h $(NATIVE_INCLUDE) -d $(TEST_CLASSES)\ $(TEST_SRC)/org/rocksdb/test/*.java\ $(TEST_SRC)/org/rocksdb/util/*.java\ $(TEST_SRC)/org/rocksdb/*.java -endif test: java java_test run_test diff --git a/java/rocksjni/testable_event_listener.cc b/java/rocksjni/testable_event_listener.cc index 9fc79c5d3..71188bc3c 100644 --- a/java/rocksjni/testable_event_listener.cc +++ b/java/rocksjni/testable_event_listener.cc @@ -4,6 +4,7 @@ // (found in the LICENSE.Apache file in the root directory). #include #include +#include #include #include "include/org_rocksdb_test_TestableEventListener.h" @@ -184,21 +185,23 @@ void Java_org_rocksdb_test_TestableEventListener_invokeAllCallbacks( write_stall_info.condition.prev = WriteStallCondition::kStopped; el->OnStallConditionsChanged(write_stall_info); - FileOperationInfo op_info = FileOperationInfo( - FileOperationType::kRead, "/file/path", + const std::string file_path = "/file/path"; + const auto start_timestamp = std::make_pair(std::chrono::time_point( std::chrono::nanoseconds(1600699420000000000ll)), std::chrono::time_point( - std::chrono::nanoseconds(1600699420000000000ll))), + std::chrono::nanoseconds(1600699420000000000ll))); + const auto finish_timestamp = std::chrono::time_point( - std::chrono::nanoseconds(1600699425000000000ll)), - status); + std::chrono::nanoseconds(1600699425000000000ll)); + FileOperationInfo op_info = + FileOperationInfo(FileOperationType::kRead, file_path, start_timestamp, + finish_timestamp, status); op_info.offset = UINT64_MAX; op_info.length = SIZE_MAX; - op_info.status = status; el->OnFileReadFinish(op_info); el->OnFileWriteFinish(op_info); diff --git a/java/src/test/java/org/rocksdb/EventListenerTest.java b/java/src/test/java/org/rocksdb/EventListenerTest.java index 92d0c736b..aec0af617 100644 --- a/java/src/test/java/org/rocksdb/EventListenerTest.java +++ b/java/src/test/java/org/rocksdb/EventListenerTest.java @@ -5,12 +5,14 @@ package org.rocksdb; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.Assert.*; +import java.nio.charset.StandardCharsets; import java.nio.file.Path; import java.nio.file.Paths; import java.util.*; import java.util.concurrent.atomic.AtomicBoolean; +import org.assertj.core.api.AbstractObjectAssert; +import org.assertj.core.api.ObjectAssert; import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; @@ -37,7 +39,7 @@ public class EventListenerTest { rand.nextBytes(value); db.put("testKey".getBytes(), value); db.flush(new FlushOptions()); - assertTrue(wasCbCalled.get()); + assertThat(wasCbCalled.get()).isTrue(); } } @@ -47,8 +49,8 @@ public class EventListenerTest { final AbstractEventListener onFlushCompletedListener = new AbstractEventListener() { @Override public void onFlushCompleted(final RocksDB rocksDb, final FlushJobInfo flushJobInfo) { - assertNotNull(flushJobInfo.getColumnFamilyName()); - assertEquals(FlushReason.MANUAL_FLUSH, flushJobInfo.getFlushReason()); + assertThat(flushJobInfo.getColumnFamilyName()).isNotNull(); + assertThat(flushJobInfo.getFlushReason()).isEqualTo(FlushReason.MANUAL_FLUSH); wasCbCalled.set(true); } }; @@ -61,8 +63,8 @@ public class EventListenerTest { final AbstractEventListener onFlushBeginListener = new AbstractEventListener() { @Override public void onFlushBegin(final RocksDB rocksDb, final FlushJobInfo flushJobInfo) { - assertNotNull(flushJobInfo.getColumnFamilyName()); - assertEquals(FlushReason.MANUAL_FLUSH, flushJobInfo.getFlushReason()); + assertThat(flushJobInfo.getColumnFamilyName()).isNotNull(); + assertThat(flushJobInfo.getFlushReason()).isEqualTo(FlushReason.MANUAL_FLUSH); wasCbCalled.set(true); } }; @@ -79,21 +81,21 @@ public class EventListenerTest { rand.nextBytes(value); db.put("testKey".getBytes(), value); final RocksDB.LiveFiles liveFiles = db.getLiveFiles(); - assertNotNull(liveFiles); - assertNotNull(liveFiles.files); - assertFalse(liveFiles.files.isEmpty()); + assertThat(liveFiles).isNotNull(); + assertThat(liveFiles.files).isNotNull(); + assertThat(liveFiles.files.isEmpty()).isFalse(); db.deleteFile(liveFiles.files.get(0)); - assertTrue(wasCbCalled.get()); + assertThat(wasCbCalled.get()).isTrue(); } } @Test - public void onTableFileDeleted() throws RocksDBException, InterruptedException { + public void onTableFileDeleted() throws RocksDBException { final AtomicBoolean wasCbCalled = new AtomicBoolean(); final AbstractEventListener onTableFileDeletedListener = new AbstractEventListener() { @Override public void onTableFileDeleted(final TableFileDeletionInfo tableFileDeletionInfo) { - assertNotNull(tableFileDeletionInfo.getDbName()); + assertThat(tableFileDeletionInfo.getDbName()).isNotNull(); wasCbCalled.set(true); } }; @@ -110,7 +112,7 @@ public class EventListenerTest { rand.nextBytes(value); db.put("testKey".getBytes(), value); db.compactRange(); - assertTrue(wasCbCalled.get()); + assertThat(wasCbCalled.get()).isTrue(); } } @@ -120,7 +122,8 @@ public class EventListenerTest { final AbstractEventListener onCompactionBeginListener = new AbstractEventListener() { @Override public void onCompactionBegin(final RocksDB db, final CompactionJobInfo compactionJobInfo) { - assertEquals(CompactionReason.kManualCompaction, compactionJobInfo.compactionReason()); + assertThat(compactionJobInfo.compactionReason()) + .isEqualTo(CompactionReason.kManualCompaction); wasCbCalled.set(true); } }; @@ -134,7 +137,8 @@ public class EventListenerTest { @Override public void onCompactionCompleted( final RocksDB db, final CompactionJobInfo compactionJobInfo) { - assertEquals(CompactionReason.kManualCompaction, compactionJobInfo.compactionReason()); + assertThat(compactionJobInfo.compactionReason()) + .isEqualTo(CompactionReason.kManualCompaction); wasCbCalled.set(true); } }; @@ -147,7 +151,7 @@ public class EventListenerTest { final AbstractEventListener onTableFileCreatedListener = new AbstractEventListener() { @Override public void onTableFileCreated(final TableFileCreationInfo tableFileCreationInfo) { - assertEquals(TableFileCreationReason.FLUSH, tableFileCreationInfo.getReason()); + assertThat(tableFileCreationInfo.getReason()).isEqualTo(TableFileCreationReason.FLUSH); wasCbCalled.set(true); } }; @@ -161,7 +165,7 @@ public class EventListenerTest { @Override public void onTableFileCreationStarted( final TableFileCreationBriefInfo tableFileCreationBriefInfo) { - assertEquals(TableFileCreationReason.FLUSH, tableFileCreationBriefInfo.getReason()); + assertThat(tableFileCreationBriefInfo.getReason()).isEqualTo(TableFileCreationReason.FLUSH); wasCbCalled.set(true); } }; @@ -179,7 +183,7 @@ public class EventListenerTest { db.put("testKey".getBytes(), value); ColumnFamilyHandle columnFamilyHandle = db.getDefaultColumnFamily(); columnFamilyHandle.close(); - assertTrue(wasCbCalled.get()); + assertThat(wasCbCalled.get()).isTrue(); } } @@ -191,7 +195,7 @@ public class EventListenerTest { @Override public void onColumnFamilyHandleDeletionStarted( final ColumnFamilyHandle columnFamilyHandle) { - assertNotNull(columnFamilyHandle); + assertThat(columnFamilyHandle).isNotNull(); wasCbCalled.set(true); } }; @@ -212,7 +216,7 @@ public class EventListenerTest { sstFileWriter.finish(); db.ingestExternalFile( Collections.singletonList(externalFilePath.toString()), new IngestExternalFileOptions()); - assertTrue(wasCbCalled.get()); + assertThat(wasCbCalled.get()).isTrue(); } } @@ -223,7 +227,7 @@ public class EventListenerTest { @Override public void onExternalFileIngested( final RocksDB db, final ExternalFileIngestionInfo externalFileIngestionInfo) { - assertNotNull(db); + assertThat(db).isNotNull(); wasCbCalled.set(true); } }; @@ -232,7 +236,6 @@ public class EventListenerTest { @Test public void testAllCallbacksInvocation() { - final int TEST_INT_VAL = -1; final long TEST_LONG_VAL = -1; // Expected test data objects final Map userCollectedPropertiesTestData = @@ -272,75 +275,79 @@ public class EventListenerTest { @Override public void onFlushCompleted(final RocksDB db, final FlushJobInfo flushJobInfo) { super.onFlushCompleted(db, flushJobInfo); - assertEquals(flushJobInfoTestData, flushJobInfo); + assertThat(flushJobInfo).isEqualTo(flushJobInfoTestData); } @Override public void onFlushBegin(final RocksDB db, final FlushJobInfo flushJobInfo) { super.onFlushBegin(db, flushJobInfo); - assertEquals(flushJobInfoTestData, flushJobInfo); + assertThat(flushJobInfo).isEqualTo(flushJobInfoTestData); } @Override public void onTableFileDeleted(final TableFileDeletionInfo tableFileDeletionInfo) { super.onTableFileDeleted(tableFileDeletionInfo); - assertEquals(tableFileDeletionInfoTestData, tableFileDeletionInfo); + assertThat(tableFileDeletionInfo).isEqualTo(tableFileDeletionInfoTestData); } @Override public void onCompactionBegin(final RocksDB db, final CompactionJobInfo compactionJobInfo) { super.onCompactionBegin(db, compactionJobInfo); - assertArrayEquals( - "compactionColumnFamily".getBytes(), compactionJobInfo.columnFamilyName()); - assertEquals(statusTestData, compactionJobInfo.status()); - assertEquals(TEST_LONG_VAL, compactionJobInfo.threadId()); - assertEquals(Integer.MAX_VALUE, compactionJobInfo.jobId()); - assertEquals(Integer.MAX_VALUE, compactionJobInfo.baseInputLevel()); - assertEquals(Integer.MAX_VALUE, compactionJobInfo.outputLevel()); - assertEquals(Collections.singletonList("inputFile.sst"), compactionJobInfo.inputFiles()); - assertEquals(Collections.singletonList("outputFile.sst"), compactionJobInfo.outputFiles()); - assertEquals(Collections.singletonMap("tableProperties", tablePropertiesTestData), - compactionJobInfo.tableProperties()); - assertEquals(CompactionReason.kFlush, compactionJobInfo.compactionReason()); - assertEquals(CompressionType.SNAPPY_COMPRESSION, compactionJobInfo.compression()); + assertThat(new String(compactionJobInfo.columnFamilyName(), StandardCharsets.UTF_8)) + .isEqualTo("compactionColumnFamily"); + assertThat(compactionJobInfo.status()).isEqualTo(statusTestData); + assertThat(compactionJobInfo.threadId()).isEqualTo(TEST_LONG_VAL); + assertThat(compactionJobInfo.jobId()).isEqualTo(Integer.MAX_VALUE); + assertThat(compactionJobInfo.baseInputLevel()).isEqualTo(Integer.MAX_VALUE); + assertThat(compactionJobInfo.outputLevel()).isEqualTo(Integer.MAX_VALUE); + assertThat(compactionJobInfo.inputFiles()) + .isEqualTo(Collections.singletonList("inputFile.sst")); + assertThat(compactionJobInfo.outputFiles()) + .isEqualTo(Collections.singletonList("outputFile.sst")); + assertThat(compactionJobInfo.tableProperties()) + .isEqualTo(Collections.singletonMap("tableProperties", tablePropertiesTestData)); + assertThat(compactionJobInfo.compactionReason()).isEqualTo(CompactionReason.kFlush); + assertThat(compactionJobInfo.compression()).isEqualTo(CompressionType.SNAPPY_COMPRESSION); } @Override public void onCompactionCompleted( final RocksDB db, final CompactionJobInfo compactionJobInfo) { super.onCompactionCompleted(db, compactionJobInfo); - assertArrayEquals( - "compactionColumnFamily".getBytes(), compactionJobInfo.columnFamilyName()); - assertEquals(statusTestData, compactionJobInfo.status()); - assertEquals(TEST_LONG_VAL, compactionJobInfo.threadId()); - assertEquals(Integer.MAX_VALUE, compactionJobInfo.jobId()); - assertEquals(Integer.MAX_VALUE, compactionJobInfo.baseInputLevel()); - assertEquals(Integer.MAX_VALUE, compactionJobInfo.outputLevel()); - assertEquals(Collections.singletonList("inputFile.sst"), compactionJobInfo.inputFiles()); - assertEquals(Collections.singletonList("outputFile.sst"), compactionJobInfo.outputFiles()); - assertEquals(Collections.singletonMap("tableProperties", tablePropertiesTestData), - compactionJobInfo.tableProperties()); - assertEquals(CompactionReason.kFlush, compactionJobInfo.compactionReason()); - assertEquals(CompressionType.SNAPPY_COMPRESSION, compactionJobInfo.compression()); + assertThat(new String(compactionJobInfo.columnFamilyName())) + .isEqualTo("compactionColumnFamily"); + assertThat(compactionJobInfo.status()).isEqualTo(statusTestData); + assertThat(compactionJobInfo.threadId()).isEqualTo(TEST_LONG_VAL); + assertThat(compactionJobInfo.jobId()).isEqualTo(Integer.MAX_VALUE); + assertThat(compactionJobInfo.baseInputLevel()).isEqualTo(Integer.MAX_VALUE); + assertThat(compactionJobInfo.outputLevel()).isEqualTo(Integer.MAX_VALUE); + assertThat(compactionJobInfo.inputFiles()) + .isEqualTo(Collections.singletonList("inputFile.sst")); + assertThat(compactionJobInfo.outputFiles()) + .isEqualTo(Collections.singletonList("outputFile.sst")); + assertThat(compactionJobInfo.tableProperties()) + .isEqualTo(Collections.singletonMap("tableProperties", tablePropertiesTestData)); + assertThat(compactionJobInfo.compactionReason()).isEqualTo(CompactionReason.kFlush); + assertThat(compactionJobInfo.compression()).isEqualTo(CompressionType.SNAPPY_COMPRESSION); } @Override public void onTableFileCreated(final TableFileCreationInfo tableFileCreationInfo) { super.onTableFileCreated(tableFileCreationInfo); - assertEquals(tableFileCreationInfoTestData, tableFileCreationInfo); + assertThat(tableFileCreationInfo).isEqualTo(tableFileCreationInfoTestData); } @Override public void onTableFileCreationStarted( final TableFileCreationBriefInfo tableFileCreationBriefInfo) { super.onTableFileCreationStarted(tableFileCreationBriefInfo); - assertEquals(tableFileCreationBriefInfoTestData, tableFileCreationBriefInfo); + assertThat(tableFileCreationBriefInfo).isEqualTo(tableFileCreationBriefInfoTestData); } @Override public void onMemTableSealed(final MemTableInfo memTableInfo) { super.onMemTableSealed(memTableInfo); - assertEquals(memTableInfoTestData, memTableInfo); + assertThat(memTableInfo).isEqualTo(memTableInfoTestData); } @Override @@ -352,7 +359,7 @@ public class EventListenerTest { public void onExternalFileIngested( final RocksDB db, final ExternalFileIngestionInfo externalFileIngestionInfo) { super.onExternalFileIngested(db, externalFileIngestionInfo); - assertEquals(externalFileIngestionInfoTestData, externalFileIngestionInfo); + assertThat(externalFileIngestionInfo).isEqualTo(externalFileIngestionInfoTestData); } @Override @@ -364,49 +371,49 @@ public class EventListenerTest { @Override public void onStallConditionsChanged(final WriteStallInfo writeStallInfo) { super.onStallConditionsChanged(writeStallInfo); - assertEquals(writeStallInfoTestData, writeStallInfo); + assertThat(writeStallInfo).isEqualTo(writeStallInfoTestData); } @Override public void onFileReadFinish(final FileOperationInfo fileOperationInfo) { super.onFileReadFinish(fileOperationInfo); - assertEquals(fileOperationInfoTestData, fileOperationInfo); + assertThat(fileOperationInfo).isEqualTo(fileOperationInfoTestData); } @Override public void onFileWriteFinish(final FileOperationInfo fileOperationInfo) { super.onFileWriteFinish(fileOperationInfo); - assertEquals(fileOperationInfoTestData, fileOperationInfo); + assertThat(fileOperationInfo).isEqualTo(fileOperationInfoTestData); } @Override public void onFileFlushFinish(final FileOperationInfo fileOperationInfo) { super.onFileFlushFinish(fileOperationInfo); - assertEquals(fileOperationInfoTestData, fileOperationInfo); + assertThat(fileOperationInfo).isEqualTo(fileOperationInfoTestData); } @Override public void onFileSyncFinish(final FileOperationInfo fileOperationInfo) { super.onFileSyncFinish(fileOperationInfo); - assertEquals(fileOperationInfoTestData, fileOperationInfo); + assertThat(fileOperationInfo).isEqualTo(fileOperationInfoTestData); } @Override public void onFileRangeSyncFinish(final FileOperationInfo fileOperationInfo) { super.onFileRangeSyncFinish(fileOperationInfo); - assertEquals(fileOperationInfoTestData, fileOperationInfo); + assertThat(fileOperationInfo).isEqualTo(fileOperationInfoTestData); } @Override public void onFileTruncateFinish(final FileOperationInfo fileOperationInfo) { super.onFileTruncateFinish(fileOperationInfo); - assertEquals(fileOperationInfoTestData, fileOperationInfo); + assertThat(fileOperationInfo).isEqualTo(fileOperationInfoTestData); } @Override public void onFileCloseFinish(final FileOperationInfo fileOperationInfo) { super.onFileCloseFinish(fileOperationInfo); - assertEquals(fileOperationInfoTestData, fileOperationInfo); + assertThat(fileOperationInfo).isEqualTo(fileOperationInfoTestData); } @Override @@ -419,15 +426,15 @@ public class EventListenerTest { public boolean onErrorRecoveryBegin( final BackgroundErrorReason backgroundErrorReason, final Status backgroundError) { super.onErrorRecoveryBegin(backgroundErrorReason, backgroundError); - assertEquals(BackgroundErrorReason.FLUSH, backgroundErrorReason); - assertEquals(statusTestData, backgroundError); + assertThat(backgroundErrorReason).isEqualTo(BackgroundErrorReason.FLUSH); + assertThat(backgroundError).isEqualTo(statusTestData); return true; } @Override public void onErrorRecoveryCompleted(final Status oldBackgroundError) { super.onErrorRecoveryCompleted(oldBackgroundError); - assertEquals(statusTestData, oldBackgroundError); + assertThat(oldBackgroundError).isEqualTo(statusTestData); } }; @@ -436,11 +443,13 @@ public class EventListenerTest { // assert assertAllEventsCalled(listener); + + assertNoCallbackErrors(listener); } @Test public void testEnabledCallbacks() { - final EnabledEventCallback enabledEvents[] = { + final EnabledEventCallback[] enabledEvents = { EnabledEventCallback.ON_MEMTABLE_SEALED, EnabledEventCallback.ON_ERROR_RECOVERY_COMPLETED}; final CapturingTestableEventListener listener = @@ -464,146 +473,65 @@ public class EventListenerTest { assertEventsCalled(capturingTestableEventListener, EnumSet.copyOf(Arrays.asList(expected))); } + private static void assertNoCallbackErrors( + final CapturingTestableEventListener capturingTestableEventListener) { + for (AssertionError error : capturingTestableEventListener.capturedAssertionErrors) { + throw new Error("An assertion failed in callback", error); + } + } + private static void assertEventsCalled( final CapturingTestableEventListener capturingTestableEventListener, final EnumSet expected) { final ListenerEvents capturedEvents = capturingTestableEventListener.capturedListenerEvents; - if (expected.contains(EnabledEventCallback.ON_FLUSH_COMPLETED)) { - assertTrue("onFlushCompleted was not called", capturedEvents.flushCompleted); - } else { - assertFalse("onFlushCompleted was not called", capturedEvents.flushCompleted); - } - - if (expected.contains(EnabledEventCallback.ON_FLUSH_BEGIN)) { - assertTrue("onFlushBegin was not called", capturedEvents.flushBegin); - } else { - assertFalse("onFlushBegin was called", capturedEvents.flushBegin); - } - - if (expected.contains(EnabledEventCallback.ON_TABLE_FILE_DELETED)) { - assertTrue("onTableFileDeleted was not called", capturedEvents.tableFileDeleted); - } else { - assertFalse("onTableFileDeleted was called", capturedEvents.tableFileDeleted); - } - - if (expected.contains(EnabledEventCallback.ON_COMPACTION_BEGIN)) { - assertTrue("onCompactionBegin was not called", capturedEvents.compactionBegin); - } else { - assertFalse("onCompactionBegin was called", capturedEvents.compactionBegin); - } - - if (expected.contains(EnabledEventCallback.ON_COMPACTION_COMPLETED)) { - assertTrue("onCompactionCompleted was not called", capturedEvents.compactionCompleted); - } else { - assertFalse("onCompactionCompleted was called", capturedEvents.compactionCompleted); - } - - if (expected.contains(EnabledEventCallback.ON_TABLE_FILE_CREATED)) { - assertTrue("onTableFileCreated was not called", capturedEvents.tableFileCreated); - } else { - assertFalse("onTableFileCreated was called", capturedEvents.tableFileCreated); - } - - if (expected.contains(EnabledEventCallback.ON_TABLE_FILE_CREATION_STARTED)) { - assertTrue( - "onTableFileCreationStarted was not called", capturedEvents.tableFileCreationStarted); - } else { - assertFalse("onTableFileCreationStarted was called", capturedEvents.tableFileCreationStarted); - } - - if (expected.contains(EnabledEventCallback.ON_MEMTABLE_SEALED)) { - assertTrue("onMemTableSealed was not called", capturedEvents.memTableSealed); - } else { - assertFalse("onMemTableSealed was called", capturedEvents.memTableSealed); - } - - if (expected.contains(EnabledEventCallback.ON_COLUMN_FAMILY_HANDLE_DELETION_STARTED)) { - assertTrue("onColumnFamilyHandleDeletionStarted was not called", - capturedEvents.columnFamilyHandleDeletionStarted); - } else { - assertFalse("onColumnFamilyHandleDeletionStarted was called", - capturedEvents.columnFamilyHandleDeletionStarted); - } - - if (expected.contains(EnabledEventCallback.ON_EXTERNAL_FILE_INGESTED)) { - assertTrue("onExternalFileIngested was not called", capturedEvents.externalFileIngested); - } else { - assertFalse("onExternalFileIngested was called", capturedEvents.externalFileIngested); - } - - if (expected.contains(EnabledEventCallback.ON_BACKGROUND_ERROR)) { - assertTrue("onBackgroundError was not called", capturedEvents.backgroundError); - } else { - assertFalse("onBackgroundError was called", capturedEvents.backgroundError); - } - - if (expected.contains(EnabledEventCallback.ON_STALL_CONDITIONS_CHANGED)) { - assertTrue("onStallConditionsChanged was not called", capturedEvents.stallConditionsChanged); - } else { - assertFalse("onStallConditionsChanged was called", capturedEvents.stallConditionsChanged); - } - - if (expected.contains(EnabledEventCallback.ON_FILE_READ_FINISH)) { - assertTrue("onFileReadFinish was not called", capturedEvents.fileReadFinish); - } else { - assertFalse("onFileReadFinish was called", capturedEvents.fileReadFinish); - } - - if (expected.contains(EnabledEventCallback.ON_FILE_WRITE_FINISH)) { - assertTrue("onFileWriteFinish was not called", capturedEvents.fileWriteFinish); - } else { - assertFalse("onFileWriteFinish was called", capturedEvents.fileWriteFinish); - } - - if (expected.contains(EnabledEventCallback.ON_FILE_FLUSH_FINISH)) { - assertTrue("onFileFlushFinish was not called", capturedEvents.fileFlushFinish); - } else { - assertFalse("onFileFlushFinish was called", capturedEvents.fileFlushFinish); - } - - if (expected.contains(EnabledEventCallback.ON_FILE_SYNC_FINISH)) { - assertTrue("onFileSyncFinish was not called", capturedEvents.fileSyncFinish); - } else { - assertFalse("onFileSyncFinish was called", capturedEvents.fileSyncFinish); - } - - if (expected.contains(EnabledEventCallback.ON_FILE_RANGE_SYNC_FINISH)) { - assertTrue("onFileRangeSyncFinish was not called", capturedEvents.fileRangeSyncFinish); - } else { - assertFalse("onFileRangeSyncFinish was called", capturedEvents.fileRangeSyncFinish); - } - - if (expected.contains(EnabledEventCallback.ON_FILE_TRUNCATE_FINISH)) { - assertTrue("onFileTruncateFinish was not called", capturedEvents.fileTruncateFinish); - } else { - assertFalse("onFileTruncateFinish was called", capturedEvents.fileTruncateFinish); - } - - if (expected.contains(EnabledEventCallback.ON_FILE_CLOSE_FINISH)) { - assertTrue("onFileCloseFinish was not called", capturedEvents.fileCloseFinish); - } else { - assertFalse("onFileCloseFinish was called", capturedEvents.fileCloseFinish); - } - - if (expected.contains(EnabledEventCallback.SHOULD_BE_NOTIFIED_ON_FILE_IO)) { - assertTrue( - "shouldBeNotifiedOnFileIO was not called", capturedEvents.shouldBeNotifiedOnFileIO); - } else { - assertFalse("shouldBeNotifiedOnFileIO was called", capturedEvents.shouldBeNotifiedOnFileIO); - } - - if (expected.contains(EnabledEventCallback.ON_ERROR_RECOVERY_BEGIN)) { - assertTrue("onErrorRecoveryBegin was not called", capturedEvents.errorRecoveryBegin); - } else { - assertFalse("onErrorRecoveryBegin was called", capturedEvents.errorRecoveryBegin); - } - - if (expected.contains(EnabledEventCallback.ON_ERROR_RECOVERY_COMPLETED)) { - assertTrue("onErrorRecoveryCompleted was not called", capturedEvents.errorRecoveryCompleted); - } else { - assertFalse("onErrorRecoveryCompleted was called", capturedEvents.errorRecoveryCompleted); - } + assertThat(capturedEvents.flushCompleted) + .isEqualTo(expected.contains(EnabledEventCallback.ON_FLUSH_COMPLETED)); + assertThat(capturedEvents.flushBegin) + .isEqualTo(expected.contains(EnabledEventCallback.ON_FLUSH_BEGIN)); + assertThat(capturedEvents.tableFileDeleted) + .isEqualTo(expected.contains(EnabledEventCallback.ON_TABLE_FILE_DELETED)); + assertThat(capturedEvents.compactionBegin) + .isEqualTo(expected.contains(EnabledEventCallback.ON_COMPACTION_BEGIN)); + assertThat(capturedEvents.compactionCompleted) + .isEqualTo(expected.contains(EnabledEventCallback.ON_COMPACTION_COMPLETED)); + assertThat(capturedEvents.tableFileCreated) + .isEqualTo(expected.contains(EnabledEventCallback.ON_TABLE_FILE_CREATED)); + assertThat(capturedEvents.tableFileCreationStarted) + .isEqualTo(expected.contains(EnabledEventCallback.ON_TABLE_FILE_CREATION_STARTED)); + assertThat(capturedEvents.memTableSealed) + .isEqualTo(expected.contains(EnabledEventCallback.ON_MEMTABLE_SEALED)); + assertThat(capturedEvents.columnFamilyHandleDeletionStarted) + .isEqualTo( + expected.contains(EnabledEventCallback.ON_COLUMN_FAMILY_HANDLE_DELETION_STARTED)); + assertThat(capturedEvents.externalFileIngested) + .isEqualTo(expected.contains(EnabledEventCallback.ON_EXTERNAL_FILE_INGESTED)); + assertThat(capturedEvents.backgroundError) + .isEqualTo(expected.contains(EnabledEventCallback.ON_BACKGROUND_ERROR)); + assertThat(capturedEvents.stallConditionsChanged) + .isEqualTo(expected.contains(EnabledEventCallback.ON_STALL_CONDITIONS_CHANGED)); + assertThat(capturedEvents.fileReadFinish) + .isEqualTo(expected.contains(EnabledEventCallback.ON_FILE_READ_FINISH)); + assertThat(capturedEvents.fileWriteFinish) + .isEqualTo(expected.contains(EnabledEventCallback.ON_FILE_WRITE_FINISH)); + assertThat(capturedEvents.fileFlushFinish) + .isEqualTo(expected.contains(EnabledEventCallback.ON_FILE_FLUSH_FINISH)); + assertThat(capturedEvents.fileSyncFinish) + .isEqualTo(expected.contains(EnabledEventCallback.ON_FILE_SYNC_FINISH)); + assertThat(capturedEvents.fileRangeSyncFinish) + .isEqualTo(expected.contains(EnabledEventCallback.ON_FILE_RANGE_SYNC_FINISH)); + assertThat(capturedEvents.fileTruncateFinish) + .isEqualTo(expected.contains(EnabledEventCallback.ON_FILE_TRUNCATE_FINISH)); + assertThat(capturedEvents.fileCloseFinish) + .isEqualTo(expected.contains(EnabledEventCallback.ON_FILE_CLOSE_FINISH)); + assertThat(capturedEvents.shouldBeNotifiedOnFileIO) + .isEqualTo(expected.contains(EnabledEventCallback.SHOULD_BE_NOTIFIED_ON_FILE_IO)); + assertThat(capturedEvents.errorRecoveryBegin) + .isEqualTo(expected.contains(EnabledEventCallback.ON_ERROR_RECOVERY_BEGIN)); + assertThat(capturedEvents.errorRecoveryCompleted) + .isEqualTo(expected.contains(EnabledEventCallback.ON_ERROR_RECOVERY_COMPLETED)); + assertThat(capturedEvents.errorRecoveryCompleted) + .isEqualTo(expected.contains(EnabledEventCallback.ON_ERROR_RECOVERY_COMPLETED)); } /** @@ -635,9 +563,43 @@ public class EventListenerTest { volatile boolean errorRecoveryCompleted; } + private static class CapturingObjectAssert extends ObjectAssert { + private final List assertionErrors; + public CapturingObjectAssert(T t, List assertionErrors) { + super(t); + this.assertionErrors = assertionErrors; + } + + @Override + public ObjectAssert isEqualTo(Object other) { + try { + return super.isEqualTo(other); + } catch (AssertionError error) { + assertionErrors.add(error); + throw error; + } + } + + @Override + public ObjectAssert isNotNull() { + try { + return super.isNotNull(); + } catch (AssertionError error) { + assertionErrors.add(error); + throw error; + } + } + } + private static class CapturingTestableEventListener extends TestableEventListener { final ListenerEvents capturedListenerEvents = new ListenerEvents(); + final List capturedAssertionErrors = new ArrayList<>(); + + protected AbstractObjectAssert assertThat(T actual) { + return new CapturingObjectAssert(actual, capturedAssertionErrors); + } + public CapturingTestableEventListener() {} public CapturingTestableEventListener(final EnabledEventCallback... enabledEventCallbacks) {