abdullah alamoudi has posted comments on this change. Change subject: Add Unit Tests for Feed Runtime Input Handler ......................................................................
Patch Set 5: (11 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? Done Line 307: ByteBuffer next = null; > Use conditional expression here? Done Line 318: } catch (InterruptedException e) { > CRITICAL SonarQube violation: Done Line 427: private boolean done = false; > Call this differently (e.g. deathRequested)? (Your choice :) ). Done 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. Done https://asterix-gerrit.ics.uci.edu/#/c/866/5/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/FrameSpiller.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/FrameSpiller.java: Line 96: try { > MAJOR SonarQube violation: Done 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/ Done Line 233: release(freeBuffer); > rethrow? Done Line 283: return subscribers; > Make this package accessible to be accessed in the tests. Done 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 396: Assert.assertTrue(hde != null); > assertNotNull? Done 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 Done -- 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
