[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[ https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Vadim Spector updated SENTRY-1471: -- Attachment: (was: SENTRY-1471.001.patch) > 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)
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[ https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Vadim Spector updated SENTRY-1471: -- Attachment: (was: SENTRY-1471.001.patch) > 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)
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[ https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Vadim Spector updated SENTRY-1471: -- Attachment: (was: SENTRY-1471.001.patch) > 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)
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[ https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Vadim Spector updated SENTRY-1471: -- Attachment: SENTRY-1471.001.patch > 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)
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[ https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Vadim Spector updated SENTRY-1471: -- Attachment: SENTRY-1471.001.patch > 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)
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[ https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Vadim Spector updated SENTRY-1471: -- Attachment: (was: SENTRY-1471.001.patch) > 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)
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[ https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Vadim Spector updated SENTRY-1471: -- Attachment: (was: SENTRY-1471.001.patch) > 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 > > 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)
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[ https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Vadim Spector updated SENTRY-1471: -- Attachment: SENTRY-1471.001.patch SENTRY-1471.001.patch > 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)
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[ https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Vadim Spector updated SENTRY-1471: -- Attachment: (was: SENTRY-1471.001.patch) > 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)
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[ https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Vadim Spector updated SENTRY-1471: -- Attachment: (was: SENTRY-1471.001.patch) > 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 > > 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)
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[ https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Vadim Spector updated SENTRY-1471: -- Attachment: SENTRY-1471.001.patch > 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)
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[ https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Vadim Spector updated SENTRY-1471: -- Attachment: (was: SENTRY-1471.001.patch) > 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)
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[ https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Vadim Spector updated SENTRY-1471: -- Attachment: SENTRY-1471.001.patch > 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)
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[ https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Vadim Spector updated SENTRY-1471: -- Attachment: (was: SENTRY-1471.001.patch) > 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)
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification 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 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 https://issues.apache.org/jira/browse/SENTRY-1454. Suggested fixes match the original code prior to SENTRY-1454. was: 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 https://issues.apache.org/jira/browse/SENTRY-1454. > 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 > > 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
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification 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 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 https://issues.apache.org/jira/browse/SENTRY-1454. was: 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. > 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 > > 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 >
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification 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 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. 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. 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. > 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 > > 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. -- This
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification 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. 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 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. 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 > 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 > > 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. 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 -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification 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. 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. 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. 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 > 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 > > 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. 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. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification 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. 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 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. 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. > 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 > > 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. 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 -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (SENTRY-1471) TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly
[ https://issues.apache.org/jira/browse/SENTRY-1471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Vadim Spector updated SENTRY-1471: -- Summary: TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly (was: TestHDFSIntegrationBase.java implements HDFS ACL checking incorrectly) > 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 > > 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. 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. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
