You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@sentry.apache.org by "Vadim Spector (JIRA)" <ji...@apache.org> on 2016/09/14 00:18:20 UTC
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java
implements HDFS ACL checking incorrectly
[ https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Vadim Spector updated SENTRY-1471:
----------------------------------
Description:
verifyOnAllSubDirsHelper() in TestHDFSIntegrationBase.java has a bug. When the number of 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.
was:
verifyOnAllSubDirsHelper() in TestHDFSIntegrationBase.java has a bug. When the number of 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.
> TestHDFSIntegrationBase.java implements HDFS ACL checking incorrectly
> ---------------------------------------------------------------------
>
> Key: SENTRY-1471
> URL: https://issues.apache.org/jira/browse/SENTRY-1471
> Project: Sentry
> Issue Type: Bug
> Components: Sentry
> Reporter: Vadim Spector
> Assignee: Anne Yu
>
> verifyOnAllSubDirsHelper() in TestHDFSIntegrationBase.java has a bug. When the number of 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.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)