> On March 2, 2016, 7:46 p.m., Hao Hao wrote: > > sentry-policy/sentry-policy-db/src/test/java/org/apache/sentry/policy/db/TestDBWildcardPrivilege.java, > > line 32 > > <https://reviews.apache.org/r/42661/diff/5/?file=1276950#file1276950line32> > > > > Why do we need to remove the Assert imports and make the class extends > > it?
It violates a PMD rule of having too many static imports: "If you overuse the static import feature, it can make your program unreadable and unmaintainable, polluting its namespace with all the static members you import. Readers of your code (including you, a few months after you wrote it) will not know which class a static member comes from (Sun 1.5 Language Guide). " It's simplier to make the class extend the junit Assert class, as many other test-classes in Sentry do. > On March 2, 2016, 7:46 p.m., Hao Hao wrote: > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtColumnScope.java, > > line 489 > > <https://reviews.apache.org/r/42661/diff/5/?file=1277003#file1277003line489> > > > > Looks like based on the previous code logic (eventhough it is weired > > :)), it should be USER1_1 here? Yes, however I believe the previous code logic was incorrect, as the "negative user" wasn't actually used at all in the for loop. - Colm ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42661/#review121696 ----------------------------------------------------------- On March 2, 2016, 3:04 p.m., Colm O hEigeartaigh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42661/ > ----------------------------------------------------------- > > (Updated March 2, 2016, 3:04 p.m.) > > > Review request for sentry. > > > Repository: sentry > > > Description > ------- > > Apply PMD to the Sentry test sources > > > Diffs > ----- > > build-tools/sentry-pmd-ruleset.xml 87a761c > pom.xml 151eefd > > sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java > 1fac0c7 > > sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/binding/solr/TestSolrAuthzBinding.java > c0445ab > > sentry-core/sentry-core-model-sqoop/src/test/java/org/apache/sentry/core/model/sqoop/TestSqoopAuthorizable.java > c346290 > > sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestKrbConnectionTimeout.java > 968d29c > > sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestUpdateableAuthzPaths.java > 98ab7ba > > sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestMetastoreCacheInitializer.java > 2c9e19d > > sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestUpdateForwarder.java > 54a83b0 > > sentry-policy/sentry-policy-db/src/test/java/org/apache/sentry/policy/db/TestDBWildcardPrivilege.java > 9fcf853 > > sentry-policy/sentry-policy-db/src/test/java/org/apache/sentry/policy/db/TestDatabaseRequiredInRole.java > f9b00b4 > > sentry-policy/sentry-policy-indexer/src/test/java/org/apache/sentry/policy/indexer/TestIndexerAuthorizationProviderGeneralCases.java > 00c1b6d > > sentry-policy/sentry-policy-indexer/src/test/java/org/apache/sentry/policy/indexer/TestIndexerRequiredInRole.java > 8494a8f > > sentry-policy/sentry-policy-indexer/src/test/java/org/apache/sentry/policy/indexer/TestIndexerWildcardPrivilege.java > b599a84 > > sentry-policy/sentry-policy-search/src/test/java/org/apache/sentry/policy/search/TestCollectionRequiredInRole.java > b626f1a > > sentry-policy/sentry-policy-search/src/test/java/org/apache/sentry/policy/search/TestSearchWildcardPrivilege.java > a4c8a2b > > sentry-policy/sentry-policy-sqoop/src/test/java/org/apache/sentry/policy/sqoop/TestSqoopAuthorizationProviderGeneralCases.java > e59164d > > sentry-policy/sentry-policy-sqoop/src/test/java/org/apache/sentry/policy/sqoop/TestSqoopWildcardPrivilege.java > f19a1f8 > > sentry-provider/sentry-provider-common/src/test/java/org/apache/sentry/provider/common/TestGetGroupMapping.java > dfb5d70 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestSentryGMPrivilege.java > 1411692 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestSentryRole.java > f8eecd9 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceIntegrationBase.java > e55f711 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java > 6821cf9 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericServiceIntegration.java > 7f8f916 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellSolr.java > 37cc966 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/appender/TestRollingFileWithoutDeleteAppender.java > e1ebce6 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java > 4e40038 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryServiceDiscovery.java > 2773a9e > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > 56c05c2 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStoreImportExport.java > 9350a50 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryVersion.java > 9c6597b > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServerForHaWithoutKerberos.java > e02e0ba > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServerWithoutKerberos.java > 5afc5b6 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/tools/TestSentryShellHive.java > 6cb1925 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java > 124293a > > sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/handler/admin/SecureCoreAdminHandlerTest.java > a145bc5 > > sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/handler/component/QueryDocAuthorizationComponentTest.java > c94f6fb > > sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/handler/component/QueryIndexAuthorizationComponentTest.java > b9766e0 > > sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/sentry/SentryIndexAuthorizationSingletonTest.java > 694c486 > > sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/sentry/SentrySingletonTestInstance.java > 664719f > > sentry-solr/solr-sentry-handlers/src/test/java/org/apache/solr/sentry/SentryTestBase.java > b5548e6 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java > 0fa21a2 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestConcurrentClients.java > d926797 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbColumnLevelMetaDataOps.java > e639071 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbComplexView.java > ef70050 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbConnections.java > d89b50e > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbEndToEnd.java > bb0ec7a > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java > 39b67f6 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSandboxOps.java > e21dfe3 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java > f166a11 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestPrivilegeWithGrantOption.java > 5c49f98 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestPrivilegeWithHAGrantOption.java > 9791790 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestHaEnd2End.java > 70828da > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java > 4d9e31c > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithHiveServer.java > 56ed955 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java > 79f74af > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java > f600fdf > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/DummySentryOnFailureHook.java > 4838f76 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestConfigTool.java > cd5a75f > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestCustomSerdePrivileges.java > 6dfdb3c > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestMetadataObjectRetrieval.java > 1415647 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java > 7b44e0a > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtColumnScope.java > ecf1117 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java > 9437fca > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestRuntimeMetadataRetrieval.java > 4925f2e > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUriPermissions.java > 911608a > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestViewPrivileges.java > 8e3d4c9 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFS.java > 872a084 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFSFactory.java > 3f03ef0 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServer.java > ee6155b > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java > e7e497d > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/InternalHiveServer.java > 0e53d3d > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/InternalMetastoreServer.java > 4f73954 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/AbstractMetastoreTestWithStaticConfiguration.java > 2c16cd6 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/SentryPolicyProviderForDb.java > 4e1e750 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreEndToEnd.java > 4d4b0fe > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java > b8cf894 > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/AbstractSolrSentryTestBase.java > 3a2104a > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/DocLevelGenerator.java > 30afd4c > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/ModifiableUserAuthenticationFilter.java > 533858b > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestDocLevelOperations.java > 46399df > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestQueryOperations.java > c257175 > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/TestRealTimeGet.java > 0d25562 > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/db/integration/AbstractSolrSentryTestWithDbProvider.java > b1a68aa > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/db/integration/TestSolrAdminOperations.java > 69b8357 > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/db/integration/TestSolrQueryOperations.java > c8f7e5f > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/db/integration/TestSolrUpdateOperations.java > 765fc34 > > sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/TestConnectorEndToEnd.java > 9e13b13 > > sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/TestOwnerPrivilege.java > 9bed526 > > sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/TestShowPrivilege.java > 609239f > > sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/TomcatSqoopRunner.java > 0d50574 > > Diff: https://reviews.apache.org/r/42661/diff/ > > > Testing > ------- > > > Thanks, > > Colm O hEigeartaigh > >
