Thanks for the thorough review. New file attached to the issue.
http://codereview.appspot.com/7441/diff/1/3 File java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/RpcServlet.java (right): http://codereview.appspot.com/7441/diff/1/3#newcode155 Line 155: } On 2008/10/07 23:44:13, johnfargo wrote:
space between methods and between members and ctor
Done. http://codereview.appspot.com/7441/diff/1/2 File java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/RpcServletTest.java (right): http://codereview.appspot.com/7441/diff/1/2#newcode74 Line 74: assertDoGetWithoutHandler(null, "function"); On 2008/10/07 23:44:13, johnfargo wrote:
It looks like these don't actually perform the test you want -- you'll
need to
run EasyMock.verify(response) on the ServletResponse object you're
creating for
each test. You could probably unroll the test helper methods too, since they're
not
repeatedly used too much. Thus testDoGetNormal would look like: request = createMockRequest(req, callback); response = createExpectedResponse(string, code); expect(handler.stuffHappens); servlet.doGet(req, response); verify(response);
Done. Should be all fixed now. http://codereview.appspot.com/7441/diff/1/2#newcode90 Line 90: int httpStatusCode) throws Exception { On 2008/10/07 23:44:13, johnfargo wrote:
args on the first line, wrapped 4-indented after
Done. http://codereview.appspot.com/7441/diff/1/2#newcode97 Line 97: EasyMock.replay(request, response, handler, writer); On 2008/10/07 23:44:13, johnfargo wrote:
stylistically we import static on these eg. import static ...EasyMock.expect;
Done. http://codereview.appspot.com/7441/diff/1/2#newcode106 Line 106: int httpStatusCode) throws Exception { On 2008/10/07 23:44:13, johnfargo wrote:
args on the first line, wrapped 4-indented after
Done. http://codereview.appspot.com/7441/diff/1/2#newcode137 Line 137: String contentType, int httpStatusCode) throws IOException { On 2008/10/07 23:44:13, johnfargo wrote:
This can be called createHttpResponse (and perhaps used for POST tests
later) Done. http://codereview.appspot.com/7441
