[ 
https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Anne Yu reassigned SENTRY-1471:
-------------------------------

    Assignee: Vadim Spector  (was: Anne Yu)

+1. Thanks [~vspec...@gmail.com]

> TestHDFSIntegrationBase.java implements HDFS ACL checking and query 
> verification incorrectly
> --------------------------------------------------------------------------------------------
>
>                 Key: SENTRY-1471
>                 URL: https://issues.apache.org/jira/browse/SENTRY-1471
>             Project: Sentry
>          Issue Type: Bug
>          Components: Sentry
>            Reporter: Vadim Spector
>            Assignee: Vadim Spector
>         Attachments: SENTRY-1471.001.patch
>
>
> verifyOnAllSubDirsHelper() in TestHDFSIntegrationBase.java has a bug. When 
> the number of failed retries exceeds max value (the "else" portion of "if 
> (retry > 0)" statement, it erroneously assigns hasSucceeded = true. It means 
> that verifyOnAllSubDirsHelper() never returns false.
> Once the problem was fixed in a local development branch, several tests in 
> TestHDFSIntegrationAdvanced.java started failing. TestHDFSIntegrationEnd2End 
> tests are still ok.
> Also, it is unfortunate that TestHDFSIntegrationBase() returns boolean 
> instead of throwing the original AssertionError which contains information 
> about expected vs. found ACL permissions - this information is invaluable for 
> debugging. The fix is easy - the to-be-fixed "else" portion is inside the 
> "catch (Throwable th)" clause, so it should just re-throw caught exception. 
> This makes "hasSucceeded" variable unnecessary - verifyOnAllSubDirsHelper() 
> should have "void" return type.
> The same problem exists in verifyQuery(), also  in 
> TestHDFSIntegrationBase.java. The "else" clause of "if (retry > 0)" also 
> assigns "isSucceeded = true;" leading to the same problem - it never fails. 
> Suggested solution would be the same - just re-throw the exception. It will 
> not only fix the problem, but also make debugging easier. Current code 
> "Assert.assertTrue(isSucceeded);" throws away valuable information about the 
> expected vs detected number of database rows.
> Note: the above problems were introduced by SENTRY-1454. Suggested fixes 
> match the original code prior to SENTRY-1454.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to