Till Westmann has posted comments on this change. Change subject: Add Unit Tests for Feed Runtime Input Handler ......................................................................
Patch Set 5: (10 comments) https://asterix-gerrit.ics.uci.edu/#/c/866/5/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/FeedRuntimeInputHandler.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/FeedRuntimeInputHandler.java: Line 38: * TODO: Add unit test cases for this class Make this comment more specific? Line 307: ByteBuffer next = null; Use conditional expression here? Line 427: private boolean done = false; Call this differently (e.g. deathRequested)? (Your choice :) ). https://asterix-gerrit.ics.uci.edu/#/c/866/5/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/FrameAction.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/FrameAction.java: Line 46: public synchronized ByteBuffer getAllocated() throws InterruptedException { Different name? This sounds like this is idempotent. https://asterix-gerrit.ics.uci.edu/#/c/866/5/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/management/ConcurrentFramePool.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/management/ConcurrentFramePool.java: Line 223: + frameAction.getSize() / defaultFrameSize + " frame"); s/frame/frames/ Line 233: release(freeBuffer); rethrow? Line 283: return subscribers; Make this package accessible to be accessed in the tests. https://asterix-gerrit.ics.uci.edu/#/c/866/5/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/feed/test/ConcurrentFramePoolUnitTest.java File asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/feed/test/ConcurrentFramePoolUnitTest.java: Line 49: private volatile static HyracksDataException cause = null; remove static? Line 396: Assert.assertTrue(hde != null); assertNotNull? https://asterix-gerrit.ics.uci.edu/#/c/866/5/hyracks-fullstack/hyracks/hyracks-api/src/test/java/org/apache/hyracks/api/test/TestFrameWriter.java File hyracks-fullstack/hyracks/hyracks-api/src/test/java/org/apache/hyracks/api/test/TestFrameWriter.java: Line 55: } factor out waiting code -- To view, visit https://asterix-gerrit.ics.uci.edu/866 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7088f489a7d53dee8cf6cdbf5baa7cd8d3884f55 Gerrit-PatchSet: 5 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: abdullah alamoudi <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: Yes
