[
https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15494222#comment-15494222
]
Vadim Spector edited comment on SENTRY-1471 at 9/15/16 6:56 PM:
----------------------------------------------------------------
Submitting the patch that fixes validation logic.
a) testAuthzObjOnPartitionMultipleTables() is definitely broken - line 603,
permissions not propagated to /tmp/external. Test marked as @Ignore'd
b) testNonHDFSLocations() - in two places calls to verifyOnAllSubDirs() replace
"FsAction.ALL" with "null" - - both mentioning SENTRY-546 as justification for
code change. They start failing, and reverting to "FsAction.ALL" fixes the test.
c) after the above changes, all tests pass
I suggest proceeding with this JIRA and opening another one, just to fix
testAuthzObjOnPartitionMultipleTables() test.
was (Author: vspec...@gmail.com):
Submitting the patch that fixes validation logic.
a) testAuthzObjOnPartitionMultipleTables() is definitely broken - line 603,
permissions not propagated to /tmp/external. Test marked as @Ignore'd
b) testNonHDFSLocations() - in two places calls to verifyOnAllSubDirs() replace
"FsAction.ALL" with "null" - - both mentioning SENTRY-546 as justification for
code change. Well, they actually start failing, and switching back to
"FsAction.ALL" fixes the test.
c) after the above changes, all tests pass
I suggest proceeding with this JIRA and opening another one, just to fix
testAuthzObjOnPartitionMultipleTables() test.
> 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: Anne Yu
> 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)
