> On March 11, 2016, 12:58 a.m., Sravya Tirukkovalur wrote: > >
Thank you Sravya for your comments! > On March 11, 2016, 12:58 a.m., Sravya Tirukkovalur wrote: > > pom.xml, line 759 > > <https://reviews.apache.org/r/44677/diff/1/?file=1294776#file1294776line759> > > > > Wondering why we need this? Seems like inherited is by default true? > > https://maven.apache.org/pom.html#Plugins Yeah, will remove it. > On March 11, 2016, 12:58 a.m., Sravya Tirukkovalur wrote: > > pom.xml, line 778 > > <https://reviews.apache.org/r/44677/diff/1/?file=1294776#file1294776line778> > > > > Sorry not familiar here, why do we need this? Force child pom's surefire plugin configurations to combine with root pom's configurations. Some moduels add their unique surefire plugin configurations. > On March 11, 2016, 12:58 a.m., Sravya Tirukkovalur wrote: > > pom.xml, line 784 > > <https://reviews.apache.org/r/44677/diff/1/?file=1294776#file1294776line784> > > > > Do we anticipate there will be no tests run? There is a slight chance > > of a green run if we skip the tests for unintended reasons. Yes, since tests are divided into two executions. Not all modules contain tests for two executions. Don't want to have test failure because of this. > On March 11, 2016, 12:58 a.m., Sravya Tirukkovalur wrote: > > pom.xml, line 790 > > <https://reviews.apache.org/r/44677/diff/1/?file=1294776#file1294776line790> > > > > Is there a reason why we do not want to reuse forks? Reuse fork will occasionally cause problems when re-create sentry service or test context in the same JVM. For many modules seems "not to reuse" is a preferred way. https://github.com/apache/incubator-sentry/blob/master/sentry-tests/sentry-tests-hive/pom.xml#L278. I've tested. Reuse or use the same JVM doesn't improve running time much. But robust test results are more important in this case. > On March 11, 2016, 12:58 a.m., Sravya Tirukkovalur wrote: > > pom.xml, line 793 > > <https://reviews.apache.org/r/44677/diff/1/?file=1294776#file1294776line793> > > > > Ignore? Yeah, I will remove this property. Should use -fae instead. > On March 11, 2016, 12:58 a.m., Sravya Tirukkovalur wrote: > > sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java, > > line 62 > > <https://reviews.apache.org/r/44677/diff/1/?file=1294777#file1294777line62> > > > > Just curious on what was the menthodology you used to know which tests > > are not thread safe? By experiments, enable concurrency then continously run all tests multiple times then list out these tests which are keep failing. Analyzed the failures. Some are because certain service talks to a fixed port, once the port is occupied, following tests keep failing; another reason is when running slow tests concurrently, it takes much longer time to finish etc, so child JVM just gets recycled. So these tests are fated to run sequentially. > On March 11, 2016, 12:58 a.m., Sravya Tirukkovalur wrote: > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java, > > line 280 > > <https://reviews.apache.org/r/44677/diff/1/?file=1294797#file1294797line280> > > > > Seems like we should fail the test if setup fails? The problem is when throwing exception in the setup class method, Junit think it's an exception instead of test failure or error. In this case, execution stops abruptly, can't rerun the failed tests either. I plan to re-run failed tests twice in this case so to avoid flaky failures. > On March 11, 2016, 12:58 a.m., Sravya Tirukkovalur wrote: > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java, > > line 549 > > <https://reviews.apache.org/r/44677/diff/1/?file=1294797#file1294797line549> > > > > Ah, I see that we are failing if the setup was unsuccessful here. But > > curious why are we moving the failing from @BeforeClass to @Before? The main reaon is plan to re-run failed test cases instead of stop execution abruptly to avoid flaky failures. The exception is mainly caused by when creating sentry service takes more than maximum seconds. In this case, can just re-run the failed tests one more time to avoid such flaky failure. - Anne ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44677/#review123042 ----------------------------------------------------------- On March 10, 2016, 10:30 p.m., Anne Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44677/ > ----------------------------------------------------------- > > (Updated March 10, 2016, 10:30 p.m.) > > > Review request for sentry. > > > Bugs: SENTRY-1108 > https://issues.apache.org/jira/browse/SENTRY-1108 > > > Repository: sentry > > > Description > ------- > > First run unit tests concurrently, then sequentially for a selected > not-thread-safe tests. Improve the surefire plugin configurations to be more > robust, for example rerun failed tests more than once; Upgrade version to > 2.19; define unit test provider to be junit47 etc. > > Please help review the pom file changes, since they are criticial to > successfully and robustally run tests in the upstream. > > > Diffs > ----- > > pom.xml 5c31bf474dea40eee37d7162c7e75f32d042739d > > sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java > 726e3dcd97c4b851d040b182d20bc68f9a8c5ca3 > > sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestURI.java > cdd4e0ba1c5f37ee64b39bb4262ad85e57a63000 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java > 97c2e7153678a1680f54d2fe2baabc8a73ffc3aa > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestAuditLogForSentryGenericService.java > c3adacf52216b4ce331dbb92a06064ccd26aa043 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericServiceIntegration.java > 921685a07795bec5a952d287b12da8d1c1e65166 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellSolr.java > 6dbe7c0f562e7be65e807df41884b14467d68822 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestAuthorizingDDLAuditLogWithKerberos.java > 426b2f7abfd72b5688d69ec45caccfce47ff04c3 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceClientPool.java > e5285bd0a760bdcc7fe27e6af96d80ebc51d7dad > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceFailureCase.java > a453ff3237662d4f45902473392f0d07b36bacea > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceForHAWithKerberos.java > 813b30b52c6d0dde6ca2c51a148a3dff7cb5cb0e > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceForPoolWithKerberos.java > bd3c1ccba0a5b2e9863042f9a12f76b613c2041a > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java > 0d35b7d66b452b1a37fbe5d838b41f84e2aea9e5 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java > 09f3d8ed8919f00d27b4a6d1536992e41c820146 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithKerberos.java > ff73382664a243359aef0609f700c9ee4d04c0df > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryWebServerWithoutSecurity.java > 4a913e5189fa0aea7fb1770eb9f3e8e991289a50 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/tools/TestSentryShellHive.java > ded4b629579afacfe5ea0b6fabcb5e2be36eee2e > sentry-solr/solr-sentry-core/pom.xml > 44fbb864ad5828445c6358e7a0aa2eb34ebc65e5 > sentry-tests/sentry-tests-hive/pom.xml > 472cce7906a454af6f3a30edb8cc5760acaa0c25 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbConnections.java > 2af05360d8c5037a07fb3b838bd75a7c55bbdfa7 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegesAtDatabaseScope.java > 883bedd95982ad92e91d9b5d53297714343f9391 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java > 704bbeeb9bd1c134398de82b2c20437226c9c4d1 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestCustomSerdePrivileges.java > 27238154bb04b04a9fb3a640c09309c0e20252bf > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtFunctionScope.java > bb8d61d86730d7910c85a33c0fb3b066ba0d649a > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java > 054b1935975368db058791102f3892f027e31636 > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/db/integration/TestSolrAdminOperations.java > c07b3b8efebca5693cddf84cc1a7104d9a7b17ee > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/db/integration/TestSolrDocLevelOperations.java > 7f1fdfdbe2edbf7b340693ea1045470efc79b043 > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/db/integration/TestSolrQueryOperations.java > 3eb6c0f0250e0e6cd8362cd63b7988423d120d80 > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/db/integration/TestSolrUpdateOperations.java > 94123259ab9f9ac90cc3a6d1978f4df7af3f0425 > > Diff: https://reviews.apache.org/r/44677/diff/ > > > Testing > ------- > > > Thanks, > > Anne Yu > >
