asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "abdullah alamoudi (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
Subject Change in asterixdb[master]: Add Unit Tests for Feed Runtime Input Handler
Date Thu, 19 May 2016 20:13:51 GMT
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 <bamousaa@gmail.com>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Till Westmann <tillw@apache.org>
Gerrit-Reviewer: abdullah alamoudi <bamousaa@gmail.com>
Gerrit-HasComments: Yes

Mime
View raw message