ambari-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Vitalyi Brodetskyi" <vbrodets...@hortonworks.com>
Subject Re: Review Request 18271: Add unit test that verifies escaped urls(ProxyService)
Date Wed, 19 Feb 2014 16:15:25 GMT


> On Feb. 19, 2014, 3:42 p.m., Nate Cole wrote:
> > The new test method does not make any assertions.  You should try using a Capture
to verify the string URI's are the same.  Also (and this is my fault for not realizing it
earlier), a year ago or so, the team agreed to use EasyMock instead of PowerMock.  If you
can, please move this test class over to use EasyMock.

About assertions. Using expect() method, we can test it too. For example:
URI uri = UriBuilder.fromUri("http://dev01.hortonworks.com:8080/proxy?url=http%3a%2f%2fserver%3a8188%2fws%2fv1%2f"
+
+     "apptimeline%2fHIVE_QUERY_ID%3ffields=events%2cprimaryfilters%26limit=10%26primaryFilter=user%3ahiveuser1").build();
expect(getUriInfo().getRequestUri()).andReturn(uri);
expect(streamProviderMock.processURL("http://server:8188/ws/v1/apptimeline/HIVE_QUERY_ID?fields=events,primary"
+
+     "filters&limit=10&primaryFilter=user:hiveuser1", "GET", null, headerParamsToForward)).andReturn(urlConnectionMock);
1) Create uri object with undecoded url.
2) Add expect() to return created earlier uri object.
3) Add expect() to return urlConnectionMock. If expect params will not be the same as in test,
then we will got NPE, because not urlConnectionMock will be returned, bu null.

About PowerMock. I agree with but that it's more correct to use EasyMock. But i can't mock
final classes with EasyMock(URI,ResponseBuilderImpl) and can't mock instance creation like
for URLStreamProvider(May be we can add some method to set it).


- Vitalyi


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18271/#review34864
-----------------------------------------------------------


On Feb. 19, 2014, 3:25 p.m., Vitalyi Brodetskyi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18271/
> -----------------------------------------------------------
> 
> (Updated Feb. 19, 2014, 3:25 p.m.)
> 
> 
> Review request for Ambari, Andrew Onischuk and Nate Cole.
> 
> 
> Bugs: AMBARI-4738
>     https://issues.apache.org/jira/browse/AMBARI-4738
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> For completeness, we should have a unit test that verifies escaped urls are covered.
IE:
> 
> /proxy?url=http%3a%2f%2fserver%3a8188%2fws%2fv1%2fapptimeline%2fHIVE_QUERY_ID%3ffields=events%2cprimaryfilters%26limit=10%26primaryFilter=user%3ahiveuser1
> 
> In fact, requiring that syntax gets out of the substring/replace business and just use
the url parameter and make a URI out of it.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/test/java/org/apache/ambari/server/proxy/ProxyServiceTest.java b90af09

> 
> Diff: https://reviews.apache.org/r/18271/diff/
> 
> 
> Testing
> -------
> 
> -------------------------------------------------------
>  T E S T S
> -------------------------------------------------------
> Listening for transport dt_socket at address: 5007
> Running org.apache.ambari.server.proxy.ProxyServiceTest
> Tests run: 8, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 68.465 sec
> 
> Results :
> 
> Tests run: 8, Failures: 0, Errors: 0, Skipped: 0
> 
> 
> Thanks,
> 
> Vitalyi Brodetskyi
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message