You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ranger.apache.org by deepak sharma <de...@gmail.com> on 2017/04/04 19:57:12 UTC
Review Request 58184: [RANGER-1497]Improvement of unit test coverage
for ranger
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58184/
-----------------------------------------------------------
Review request for ranger, Ankita Sinha and Mehul Parikh.
Bugs: RANGER-1497
https://issues.apache.org/jira/browse/RANGER-1497
Repository: ranger
Description
-------
[RANGER-1497]Improvement of unit test coverage for ranger by adding the unit tests for org.apache.ranger.rest.AssetREST
Diffs
-----
security-admin/src/test/java/org/apache/ranger/rest/TestAssetREST.java PRE-CREATION
Diff: https://reviews.apache.org/r/58184/diff/1/
Testing
-------
tested the Junit tests added locally.
Thanks,
deepak sharma
Re: Review Request 58184: [RANGER-1497]Improvement of unit test
coverage for ranger
Posted by Ramesh Mani <rm...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58184/#review171156
-----------------------------------------------------------
security-admin/src/test/java/org/apache/ranger/rest/TestAssetREST.java
Lines 839 (patched)
<https://reviews.apache.org/r/58184/#comment244040>
Please check that if there is exception in serviceREST.grantAccess is it to be considered as failure, instead of doing e.printStackTrace()
serviceREST.grantPermission() uses serviceREST.grantAccess() for all the grant access and if there is a failure there that means grant failed.
Please check all the other occurances like this for handling the exception cases.
- Ramesh Mani
On April 5, 2017, 9:08 a.m., deepak sharma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58184/
> -----------------------------------------------------------
>
> (Updated April 5, 2017, 9:08 a.m.)
>
>
> Review request for ranger, Ankita Sinha and Mehul Parikh.
>
>
> Bugs: RANGER-1497
> https://issues.apache.org/jira/browse/RANGER-1497
>
>
> Repository: ranger
>
>
> Description
> -------
>
> [RANGER-1497]Improvement of unit test coverage for ranger by adding the unit tests for org.apache.ranger.rest.AssetREST
>
>
> Diffs
> -----
>
> security-admin/src/test/java/org/apache/ranger/rest/TestAssetREST.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/58184/diff/2/
>
>
> Testing
> -------
>
> tested the Junit tests added locally.
>
>
> Thanks,
>
> deepak sharma
>
>
Re: Review Request 58184: [RANGER-1497]Improvement of unit test
coverage for ranger
Posted by Colm O hEigeartaigh <co...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58184/#review171990
-----------------------------------------------------------
Ship it!
Ship It!
- Colm O hEigeartaigh
On April 6, 2017, 9:09 a.m., deepak sharma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58184/
> -----------------------------------------------------------
>
> (Updated April 6, 2017, 9:09 a.m.)
>
>
> Review request for ranger, Ankita Sinha and Mehul Parikh.
>
>
> Bugs: RANGER-1497
> https://issues.apache.org/jira/browse/RANGER-1497
>
>
> Repository: ranger
>
>
> Description
> -------
>
> [RANGER-1497]Improvement of unit test coverage for ranger by adding the unit tests for org.apache.ranger.rest.AssetREST
>
>
> Diffs
> -----
>
> security-admin/src/test/java/org/apache/ranger/rest/TestAssetREST.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/58184/diff/3/
>
>
> Testing
> -------
>
> tested the Junit tests added locally.
>
>
> Thanks,
>
> deepak sharma
>
>
Re: Review Request 58184: [RANGER-1497]Improvement of unit test
coverage for ranger
Posted by deepak sharma <de...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58184/
-----------------------------------------------------------
(Updated April 6, 2017, 9:09 a.m.)
Review request for ranger, Ankita Sinha and Mehul Parikh.
Bugs: RANGER-1497
https://issues.apache.org/jira/browse/RANGER-1497
Repository: ranger
Description
-------
[RANGER-1497]Improvement of unit test coverage for ranger by adding the unit tests for org.apache.ranger.rest.AssetREST
Diffs (updated)
-----
security-admin/src/test/java/org/apache/ranger/rest/TestAssetREST.java PRE-CREATION
Diff: https://reviews.apache.org/r/58184/diff/3/
Changes: https://reviews.apache.org/r/58184/diff/2-3/
Testing
-------
tested the Junit tests added locally.
Thanks,
deepak sharma
Re: Review Request 58184: [RANGER-1497]Improvement of unit test
coverage for ranger
Posted by deepak sharma <de...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58184/
-----------------------------------------------------------
(Updated April 5, 2017, 9:08 a.m.)
Review request for ranger, Ankita Sinha and Mehul Parikh.
Bugs: RANGER-1497
https://issues.apache.org/jira/browse/RANGER-1497
Repository: ranger
Description
-------
[RANGER-1497]Improvement of unit test coverage for ranger by adding the unit tests for org.apache.ranger.rest.AssetREST
Diffs (updated)
-----
security-admin/src/test/java/org/apache/ranger/rest/TestAssetREST.java PRE-CREATION
Diff: https://reviews.apache.org/r/58184/diff/2/
Changes: https://reviews.apache.org/r/58184/diff/1-2/
Testing
-------
tested the Junit tests added locally.
Thanks,
deepak sharma
Re: Review Request 58184: [RANGER-1497]Improvement of unit test
coverage for ranger
Posted by deepak sharma <de...@gmail.com>.
> On April 5, 2017, 4:01 a.m., Ayub Pathan wrote:
> > security-admin/src/test/java/org/apache/ranger/rest/TestAssetREST.java
> > Lines 110 (patched)
> > <https://reviews.apache.org/r/58184/diff/1/?file=1684530#file1684530line110>
> >
> > All the below Mock injection is not used in the code.
> >
> > vXAsset
> > searchFilter
> > xAssetService
> > xResourceService
> > xCredentialStoreService
> > request
> > servicePolicies
> > xTrxLogService
> > xPolicyExportAudits
> > xAccessAuditService
> > xxServiceDefDao
> > restErrorUtil
Thanks for the review Ayub, i removed some of the unused mock injection, but following are used actually, and if you remove them then test will fail:
xAssetService: https://github.com/hortonworks/ranger/blob/HDP-2.3.1.1/security-admin/src/main/java/org/apache/ranger/rest/AssetREST.java#L255
same way below mentioned are also used:
xResourceService:
xCredentialStoreService:
XTrxLogService:
xPolicyExportAudits
xAccessAuditService
but rest rest of them i have removed.
- deepak
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58184/#review171084
-----------------------------------------------------------
On April 5, 2017, 9:08 a.m., deepak sharma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58184/
> -----------------------------------------------------------
>
> (Updated April 5, 2017, 9:08 a.m.)
>
>
> Review request for ranger, Ankita Sinha and Mehul Parikh.
>
>
> Bugs: RANGER-1497
> https://issues.apache.org/jira/browse/RANGER-1497
>
>
> Repository: ranger
>
>
> Description
> -------
>
> [RANGER-1497]Improvement of unit test coverage for ranger by adding the unit tests for org.apache.ranger.rest.AssetREST
>
>
> Diffs
> -----
>
> security-admin/src/test/java/org/apache/ranger/rest/TestAssetREST.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/58184/diff/2/
>
>
> Testing
> -------
>
> tested the Junit tests added locally.
>
>
> Thanks,
>
> deepak sharma
>
>
Re: Review Request 58184: [RANGER-1497]Improvement of unit test
coverage for ranger
Posted by Ayub Pathan <ar...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58184/#review171084
-----------------------------------------------------------
security-admin/src/test/java/org/apache/ranger/rest/TestAssetREST.java
Lines 110 (patched)
<https://reviews.apache.org/r/58184/#comment243942>
All the below Mock injection is not used in the code.
vXAsset
searchFilter
xAssetService
xResourceService
xCredentialStoreService
request
servicePolicies
xTrxLogService
xPolicyExportAudits
xAccessAuditService
xxServiceDefDao
restErrorUtil
security-admin/src/test/java/org/apache/ranger/rest/TestAssetREST.java
Lines 133 (patched)
<https://reviews.apache.org/r/58184/#comment243946>
remove whitespaces
security-admin/src/test/java/org/apache/ranger/rest/TestAssetREST.java
Lines 355 (patched)
<https://reviews.apache.org/r/58184/#comment243944>
Use the already defined Mock
@Mock
HttpServletRequest request;
Similarly check for other Mock objects as well.
- Ayub Pathan
On April 4, 2017, 7:57 p.m., deepak sharma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58184/
> -----------------------------------------------------------
>
> (Updated April 4, 2017, 7:57 p.m.)
>
>
> Review request for ranger, Ankita Sinha and Mehul Parikh.
>
>
> Bugs: RANGER-1497
> https://issues.apache.org/jira/browse/RANGER-1497
>
>
> Repository: ranger
>
>
> Description
> -------
>
> [RANGER-1497]Improvement of unit test coverage for ranger by adding the unit tests for org.apache.ranger.rest.AssetREST
>
>
> Diffs
> -----
>
> security-admin/src/test/java/org/apache/ranger/rest/TestAssetREST.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/58184/diff/1/
>
>
> Testing
> -------
>
> tested the Junit tests added locally.
>
>
> Thanks,
>
> deepak sharma
>
>