tkalkirill commented on code in PR #4602: URL: https://github.com/apache/ignite-3/pull/4602#discussion_r1808574501
########## modules/metastorage/src/test/java/org/apache/ignite/internal/metastorage/server/RocksDbKeyValueStorageTest.java: ########## @@ -49,7 +49,12 @@ public class RocksDbKeyValueStorageTest extends BasicOperationsKeyValueStorageTest { @Override public KeyValueStorage createStorage() { - return new RocksDbKeyValueStorage(NODE_NAME, workDir.resolve("storage"), new NoOpFailureManager()); + return new RocksDbKeyValueStorage( + NODE_NAME, + workDir.resolve("storage"), + new NoOpFailureManager(), + new ReadOperationForCompactionTracker() Review Comment: When implementing, I thought about what to do correctly: 1. Lay down some straw. 2. Make it as simple as possible. I was thinking about making a `@TestOnly` constructor that would have both `new NoOpFailureManager()` and `new ReadOperationForCompactionTracker()`. Or some kind of test helper class that will do something like this: ``` pubic class TestRocksDbKeyValueStorageUtils { public static createForTest(String nodeName, Path workDir) { return new RocksDbKeyValueStorage(nodeName, wordir.resolve("storage"), new NoOpFailureManager(), new ReadOperationForCompactionTracker()); } } ``` I chose option (1), do you think it's worth doing differently? -- 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: notifications-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org