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 Test Servers to Test Framework
Date Mon, 22 Feb 2016 11:36:36 GMT
abdullah alamoudi has posted comments on this change.

Change subject: Add Test Servers to Test Framework
......................................................................


Patch Set 1:

(16 comments)

https://asterix-gerrit.ics.uci.edu/#/c/651/1/asterix-common/pom.xml
File asterix-common/pom.xml:

Line 240:         </dependency>
> Please add an issue to reflect this in the LICENSE/NOTICE
Done


https://asterix-gerrit.ics.uci.edu/#/c/651/1/asterix-common/src/test/java/org/apache/asterix/test/aql/TestExecutor.java
File asterix-common/src/test/java/org/apache/asterix/test/aql/TestExecutor.java:

Line 73:     private HashMap<Integer, ITestServer> testServers = new HashMap<>();
> static final. Rename to testRunningServers
Done


Line 644:                                 String[] command = statement.trim().split(" ");
> Would be nice to skip lines starting with a '#' here, so that we can add co
Done


Line 647:                                             "invalid server command format. expected
format = (start <test server name> <port> [<arg1>][<arg2>][<arg3>]...|stop
(<port>|all))");
> line break
Done


Line 653:                                                 "invalid server start command. expected
format = (start <test server name> <port> [<arg1>][<arg2>][<arg3>]...");
> line break
Done


Line 662:                                     server.start();
> this start call in case of FileTestServer is going to block due to server.a
Good catch.
Done


https://asterix-gerrit.ics.uci.edu/#/c/651/1/asterix-common/src/test/java/org/apache/asterix/test/server/FileTestServer.java
File asterix-common/src/test/java/org/apache/asterix/test/server/FileTestServer.java:

Line 30:     private int port;
> final
Done


Line 52:                     byte[] chunk = new byte[1024];
> how about?
I can't handle this much coolness!
Done


Line 63:             } catch (IOException e) {
> Why do we ignore exceptions here? Please add a comment.
Done


https://asterix-gerrit.ics.uci.edu/#/c/651/1/asterix-common/src/test/java/org/apache/asterix/test/server/RSSFeedServlet.java
File asterix-common/src/test/java/org/apache/asterix/test/server/RSSFeedServlet.java:

Line 50:     private String _defaultFeedType;
> since when do we name member variables like this? :)
You know that I know that you know that I copied this from a sample example :D


Line 105:             // IT CANNOT HAPPEN WITH THIS SAMPLE
> Let's throw unchecked exceptions here - this is test code.
Done


Line 119:             // IT CANNOT HAPPEN WITH THIS SAMPLE
> Let's throw unchecked exceptions here - this is test code.
Done


Line 124:                 + "<p>For details check the <a href=\"https://rometools.jira.com/wiki/display/ROME/Change+Log#ChangeLog-Changesmadefromv0.3tov0.4\">Changes
Log for 0.3</a></p>");
> line break
Done


Line 134:             // IT CANNOT HAPPEN WITH THIS SAMPLE
> Let's throw unchecked exceptions here - this is test code.
Done


Line 139:                 + "<p>For details check the <a href=\"https://rometools.jira.com/wiki/display/ROME/Change+Log#ChangeLog-Changesmadefromv0.4tov0.5\">Changes
Log for 0.4</a></p>");
> line break
Done


https://asterix-gerrit.ics.uci.edu/#/c/651/1/asterix-common/src/test/java/org/apache/asterix/test/server/TestServerProvider.java
File asterix-common/src/test/java/org/apache/asterix/test/server/TestServerProvider.java:

Line 23:     public static ITestServer createTestServer(String string, Integer port) throws
Exception {
> string -> name
Done


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/651
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I3d0434925972770cdae168656e1672cf0f225980
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <bamousaa@gmail.com>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hubailmor@gmail.com>
Gerrit-Reviewer: Till Westmann <tillw@apache.org>
Gerrit-Reviewer: abdullah alamoudi <bamousaa@gmail.com>
Gerrit-HasComments: Yes

Mime
View raw message