You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Anne Yu <an...@cloudera.com> on 2016/01/15 23:14:34 UTC
Review Request 42364: Improve TestDatabaseProvider to validate test
object names instead of validating vague numbers.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42364/
-----------------------------------------------------------
Review request for sentry and Hao Hao.
Bugs: SENTRY-1009
https://issues.apache.org/jira/browse/SENTRY-1009
Repository: sentry
Description
-------
We started to see many TestDatabaseProvider test failures because it validates number of test objects instead of test object names. Because there are some system roles created in the environment, so the number of objects are usually larger than expected. Improved the test cases. Also catch the exception whenever cleanup fails since we don't need to do anything.
Diffs
-----
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java 98de57d3d749c3f887632ff1461ef05dd29376c1
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java dc8c1eb802adbf198e8623b02bd77bb8aff7fa41
Diff: https://reviews.apache.org/r/42364/diff/
Testing
-------
Yes!
Thanks,
Anne Yu
Re: Review Request 42364: Improve TestDatabaseProvider to validate
test object names instead of validating vague numbers.
Posted by Anne Yu <an...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42364/#review114801
-----------------------------------------------------------
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java
<https://reviews.apache.org/r/42364/#comment175653>
Will put it back. Was testing sth but forgot to revert.
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java (line 501)
<https://reviews.apache.org/r/42364/#comment175654>
Will remove the first close statement.
- Anne Yu
On Jan. 15, 2016, 10:44 p.m., Anne Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42364/
> -----------------------------------------------------------
>
> (Updated Jan. 15, 2016, 10:44 p.m.)
>
>
> Review request for sentry and Hao Hao.
>
>
> Bugs: SENTRY-1009
> https://issues.apache.org/jira/browse/SENTRY-1009
>
>
> Repository: sentry
>
>
> Description
> -------
>
> We started to see many TestDatabaseProvider test failures because it validates number of test objects instead of test object names. Because there are some system roles created in the environment, so the number of objects are usually larger than expected. Improved the test cases. Also catch the exception whenever cleanup fails since we don't need to do anything.
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java 98de57d3d749c3f887632ff1461ef05dd29376c1
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java dc8c1eb802adbf198e8623b02bd77bb8aff7fa41
>
> Diff: https://reviews.apache.org/r/42364/diff/
>
>
> Testing
> -------
>
> mvn test -Dtest=TestDatabaseProvider passed locally testing enviroments.
>
>
> Thanks,
>
> Anne Yu
>
>
Re: Review Request 42364: Improve TestDatabaseProvider to validate
test object names instead of validating vague numbers.
Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42364/#review114789
-----------------------------------------------------------
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java (line 2046)
<https://reviews.apache.org/r/42364/#comment175635>
Change the comments corrodingly.
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java
<https://reviews.apache.org/r/42364/#comment175636>
Why we remove the rest of the test?
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java (line 501)
<https://reviews.apache.org/r/42364/#comment175634>
If statement2.close() fail in line 497, could it success here?
- Hao Hao
On Jan. 15, 2016, 10:44 p.m., Anne Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42364/
> -----------------------------------------------------------
>
> (Updated Jan. 15, 2016, 10:44 p.m.)
>
>
> Review request for sentry and Hao Hao.
>
>
> Bugs: SENTRY-1009
> https://issues.apache.org/jira/browse/SENTRY-1009
>
>
> Repository: sentry
>
>
> Description
> -------
>
> We started to see many TestDatabaseProvider test failures because it validates number of test objects instead of test object names. Because there are some system roles created in the environment, so the number of objects are usually larger than expected. Improved the test cases. Also catch the exception whenever cleanup fails since we don't need to do anything.
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java 98de57d3d749c3f887632ff1461ef05dd29376c1
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java dc8c1eb802adbf198e8623b02bd77bb8aff7fa41
>
> Diff: https://reviews.apache.org/r/42364/diff/
>
>
> Testing
> -------
>
> mvn test -Dtest=TestDatabaseProvider passed locally testing enviroments.
>
>
> Thanks,
>
> Anne Yu
>
>
Re: Review Request 42364: Improve TestDatabaseProvider to validate
test object names instead of validating vague numbers.
Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42364/#review114815
-----------------------------------------------------------
Ship it!
Ship It!
- Hao Hao
On Jan. 16, 2016, 12:11 a.m., Anne Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42364/
> -----------------------------------------------------------
>
> (Updated Jan. 16, 2016, 12:11 a.m.)
>
>
> Review request for sentry and Hao Hao.
>
>
> Bugs: SENTRY-1009
> https://issues.apache.org/jira/browse/SENTRY-1009
>
>
> Repository: sentry
>
>
> Description
> -------
>
> We started to see many TestDatabaseProvider test failures because it validates number of test objects instead of test object names. Because there are some system roles created in the environment, so the number of objects are usually larger than expected. Improved the test cases. Also catch the exception whenever cleanup fails since we don't need to do anything.
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java 98de57d3d749c3f887632ff1461ef05dd29376c1
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java dc8c1eb802adbf198e8623b02bd77bb8aff7fa41
>
> Diff: https://reviews.apache.org/r/42364/diff/
>
>
> Testing
> -------
>
> mvn test -Dtest=TestDatabaseProvider passed locally testing enviroments.
>
>
> Thanks,
>
> Anne Yu
>
>
Re: Review Request 42364: Improve TestDatabaseProvider to validate
test object names instead of validating vague numbers.
Posted by Anne Yu <an...@cloudera.com>.
> On Jan. 16, 2016, 8:12 a.m., Lenni Kuff wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java, line 504
> > <https://reviews.apache.org/r/42364/diff/2/?file=1199038#file1199038line504>
> >
> > Do we want to swallow the exception here and on lne 525?
The reason to swallow is because our testing cluster/enviroments are not well created, they are havig issues with cm created monitoring tables. These cm created tables occasionally pop up when sentry tests are cleaning up databases/tables, when sentry tests actually delete them, exceptions occurred. It causes many e2e test failures. I need to spend time investigating why these cm tables get visible, but before it, in a normal situation, we never have cleanup exceptions unless the sentry/hive servers are encountering serious issues, which worthy a furthur debug.
org.apache.hive.service.cli.HiveSQLException: Error while compiling statement: FAILED: SemanticException [Error 10001]: Table not found cm_test_table
at org.apache.hive.jdbc.Utils.verifySuccess(Utils.java:235)
at org.apache.hive.jdbc.Utils.verifySuccessWithInfo(Utils.java:221)
at org.apache.hive.jdbc.HiveStatement.execute(HiveStatement.java:244)
at org.apache.sentry.tests.e2e.hive.AbstractTestWithStaticConfiguration.clearAll(AbstractTestWithStaticConfiguration.java:484)
at org.apache.sentry.tests.e2e.hive.AbstractTestWithStaticConfiguration.clearAfterPerTest(AbstractTestWithStaticConfiguration.java:463)
at sun.reflect.GeneratedMethodAccessor21.invoke(Unknown Source)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:606)
at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:45)
at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)
at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:42)
at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:36)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:263)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:68)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:47)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:231)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:60)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:229)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:50)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:222)
at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:28)
at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:30)
at org.junit.runners.ParentRunner.run(ParentRunner.java:300)
at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:264)
at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:153)
at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:124)
at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:200)
at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:153)
at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:103)
Caused by: org.apache.hive.service.cli.HiveSQLException: Error while compiling statement: FAILED: SemanticException [Error 10001]: Table not found cm_test_table
- Anne
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42364/#review114849
-----------------------------------------------------------
On Jan. 16, 2016, 12:11 a.m., Anne Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42364/
> -----------------------------------------------------------
>
> (Updated Jan. 16, 2016, 12:11 a.m.)
>
>
> Review request for sentry and Hao Hao.
>
>
> Bugs: SENTRY-1009
> https://issues.apache.org/jira/browse/SENTRY-1009
>
>
> Repository: sentry
>
>
> Description
> -------
>
> We started to see many TestDatabaseProvider test failures because it validates number of test objects instead of test object names. Because there are some system roles created in the environment, so the number of objects are usually larger than expected. Improved the test cases. Also catch the exception whenever cleanup fails since we don't need to do anything.
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java 98de57d3d749c3f887632ff1461ef05dd29376c1
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java dc8c1eb802adbf198e8623b02bd77bb8aff7fa41
>
> Diff: https://reviews.apache.org/r/42364/diff/
>
>
> Testing
> -------
>
> mvn test -Dtest=TestDatabaseProvider passed locally testing enviroments.
>
>
> Thanks,
>
> Anne Yu
>
>
Re: Review Request 42364: Improve TestDatabaseProvider to validate
test object names instead of validating vague numbers.
Posted by Lenni Kuff <ls...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42364/#review114849
-----------------------------------------------------------
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java (line 504)
<https://reviews.apache.org/r/42364/#comment175694>
Do we want to swallow the exception here and on lne 525?
- Lenni Kuff
On Jan. 16, 2016, 12:11 a.m., Anne Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42364/
> -----------------------------------------------------------
>
> (Updated Jan. 16, 2016, 12:11 a.m.)
>
>
> Review request for sentry and Hao Hao.
>
>
> Bugs: SENTRY-1009
> https://issues.apache.org/jira/browse/SENTRY-1009
>
>
> Repository: sentry
>
>
> Description
> -------
>
> We started to see many TestDatabaseProvider test failures because it validates number of test objects instead of test object names. Because there are some system roles created in the environment, so the number of objects are usually larger than expected. Improved the test cases. Also catch the exception whenever cleanup fails since we don't need to do anything.
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java 98de57d3d749c3f887632ff1461ef05dd29376c1
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java dc8c1eb802adbf198e8623b02bd77bb8aff7fa41
>
> Diff: https://reviews.apache.org/r/42364/diff/
>
>
> Testing
> -------
>
> mvn test -Dtest=TestDatabaseProvider passed locally testing enviroments.
>
>
> Thanks,
>
> Anne Yu
>
>
Re: Review Request 42364: Improve TestDatabaseProvider to validate
test object names instead of validating vague numbers.
Posted by Anne Yu <an...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42364/
-----------------------------------------------------------
(Updated Jan. 19, 2016, 10:21 p.m.)
Review request for sentry and Hao Hao.
Changes
-------
On a second thought, removed the code to catch exceptions in clean up codes.
Bugs: SENTRY-1009
https://issues.apache.org/jira/browse/SENTRY-1009
Repository: sentry
Description
-------
We started to see many TestDatabaseProvider test failures because it validates number of test objects instead of test object names. Because there are some system roles created in the environment, so the number of objects are usually larger than expected. Improved the test cases. Also catch the exception whenever cleanup fails since we don't need to do anything.
Diffs (updated)
-----
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java 98de57d3d749c3f887632ff1461ef05dd29376c1
Diff: https://reviews.apache.org/r/42364/diff/
Testing
-------
mvn test -Dtest=TestDatabaseProvider passed locally testing enviroments.
Thanks,
Anne Yu
Re: Review Request 42364: Improve TestDatabaseProvider to validate
test object names instead of validating vague numbers.
Posted by Anne Yu <an...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42364/
-----------------------------------------------------------
(Updated Jan. 16, 2016, 12:11 a.m.)
Review request for sentry and Hao Hao.
Changes
-------
Addressed previous code review comments: add test code back; then remove extra statement close code.
Bugs: SENTRY-1009
https://issues.apache.org/jira/browse/SENTRY-1009
Repository: sentry
Description
-------
We started to see many TestDatabaseProvider test failures because it validates number of test objects instead of test object names. Because there are some system roles created in the environment, so the number of objects are usually larger than expected. Improved the test cases. Also catch the exception whenever cleanup fails since we don't need to do anything.
Diffs (updated)
-----
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java 98de57d3d749c3f887632ff1461ef05dd29376c1
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java dc8c1eb802adbf198e8623b02bd77bb8aff7fa41
Diff: https://reviews.apache.org/r/42364/diff/
Testing
-------
mvn test -Dtest=TestDatabaseProvider passed locally testing enviroments.
Thanks,
Anne Yu
Re: Review Request 42364: Improve TestDatabaseProvider to validate
test object names instead of validating vague numbers.
Posted by Anne Yu <an...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42364/
-----------------------------------------------------------
(Updated Jan. 15, 2016, 10:44 p.m.)
Review request for sentry and Hao Hao.
Bugs: SENTRY-1009
https://issues.apache.org/jira/browse/SENTRY-1009
Repository: sentry
Description
-------
We started to see many TestDatabaseProvider test failures because it validates number of test objects instead of test object names. Because there are some system roles created in the environment, so the number of objects are usually larger than expected. Improved the test cases. Also catch the exception whenever cleanup fails since we don't need to do anything.
Diffs
-----
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java 98de57d3d749c3f887632ff1461ef05dd29376c1
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java dc8c1eb802adbf198e8623b02bd77bb8aff7fa41
Diff: https://reviews.apache.org/r/42364/diff/
Testing (updated)
-------
mvn test -Dtest=TestDatabaseProvider passed locally testing enviroments.
Thanks,
Anne Yu