You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2022/06/22 16:59:21 UTC

[GitHub] [hive] zabetak opened a new pull request, #3399: HIVE-26021: Change integration tests under DBInstallBase to Checkin tests

zabetak opened a new pull request, #3399:
URL: https://github.com/apache/hive/pull/3399

   ### What changes were proposed in this pull request and why?
   1. Simplify execution of tests by developers and CI
   2. Extend test coverage to include mysql and mssql install upgrade tests
   3. Rename tests to better reflect their purpose (not integration tests
   anymore) but more importantly to allow Jenkins splitTests step (
   https://plugins.jenkins.io/parallel-test-executor/) to pick them up
   automatically.
   
   ### Does this PR introduce _any_ user-facing change?
   Only developers are affected since tests are now run differently.
   
   ### How was this patch tested?
   ```
   mvn test -Dtest.groups=MetastoreCheckinTest -pl standalone-metastore/metastore-server -Dtest=ITestDerby,ITestOracle,ITestPostgres,ITestMssql,ITestMysql
   ```
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] pvary commented on a diff in pull request #3399: HIVE-26021: Change integration tests under DBInstallBase to Checkin tests

Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3399:
URL: https://github.com/apache/hive/pull/3399#discussion_r904509731


##########
Jenkinsfile:
##########
@@ -279,15 +279,6 @@ reinit_metastore $dbType
 time docker rm -f dev_$dbType || true
 '''
           }
-          stage('verify') {

Review Comment:
   Why only remove the `verify` stage? Why not remove the whole test?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] pvary commented on a diff in pull request #3399: HIVE-26021: Change integration tests under DBInstallBase to Checkin tests

Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3399:
URL: https://github.com/apache/hive/pull/3399#discussion_r904508963


##########
standalone-metastore/DEV-README:
##########
@@ -23,47 +23,34 @@ quick test can be done for the unit tests and more in depth testing as part
 of the checkin tests.
 
 --------------------------------------------------------------------------------
-There are integration tests for testing installation and upgrade of the
+There are checkin tests for testing installation and upgrade of the
 metastore on Derby, MySQL (actually MariaDB is used), Oracle, Postgres, and SQLServer.
 
-Each ITest runs two tests, one that installs the latest version of the
+For each DB type we runs two tests, one that installs the latest version of the
 database and one that installs the latest version minus one and then upgrades
 the database.
 
-To run the tests you will need to explicitly turn on integration testing by
-setting skipITests variable to false. The tests rely on Docker so the latter
-needs to be installed and configured properly (e.g., memory more than 3.5GB).
-
-Run all tests:
-
-mvn verify -DskipITests=false -Dtest=nosuch
+The tests (except Derby) rely on Docker so the latter needs to be installed and configured
+properly (e.g., memory more than 3.5GB).
 
 Run a single test:
 
-mvn verify -DskipITests=false -Dit.test=ITestMysql -Dtest=nosuch
+mvn test -Dtest.groups=MetastoreCheckinTest -Dtest=ITestDerby
 
 Supported databases for testing:
--Dit.test=ITestDerby
--Dit.test=ITestMysql
--Dit.test=ITestOracle
--Dit.test=ITestPostgres
--Dit.test=ITestMssql
+ITestDerby

Review Comment:
   I think these names are changed 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] zabetak commented on a diff in pull request #3399: HIVE-26021: Change integration tests under DBInstallBase to Checkin tests

Posted by GitBox <gi...@apache.org>.
zabetak commented on code in PR #3399:
URL: https://github.com/apache/hive/pull/3399#discussion_r904754760


##########
Jenkinsfile:
##########
@@ -93,7 +93,7 @@ OPTS+=" -Dorg.slf4j.simpleLogger.log.org.apache.maven.plugin.surefire.SurefirePl
 OPTS+=" -Dmaven.repo.local=$PWD/.git/m2"
 git config extra.mavenOpts "$OPTS"
 OPTS=" $M_OPTS -Dmaven.test.failure.ignore "
-if [ -s inclusions.txt ]; then OPTS+=" -Dsurefire.includesFile=$PWD/inclusions.txt"; sed -i '/\\/ITest/d' $PWD/inclusions.txt;fi
+if [ -s inclusions.txt ]; then OPTS+=" -Dsurefire.includesFile=$PWD/inclusions.txt";fi

Review Comment:
   Yes, they all run and all passed. More details here:
   ```
   http://ci.hive.apache.org/job/hive-precommit/job/PR-3399/1/testReport/api/xml?xpath=/testResult/suite[contains(name/text(),%22org.apache.hadoop.hive.metastore.dbinstall%22)]&wrapper=testcases
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] pvary commented on a diff in pull request #3399: HIVE-26021: Change integration tests under DBInstallBase to Checkin tests

Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3399:
URL: https://github.com/apache/hive/pull/3399#discussion_r904510981


##########
Jenkinsfile:
##########
@@ -93,7 +93,7 @@ OPTS+=" -Dorg.slf4j.simpleLogger.log.org.apache.maven.plugin.surefire.SurefirePl
 OPTS+=" -Dmaven.repo.local=$PWD/.git/m2"
 git config extra.mavenOpts "$OPTS"
 OPTS=" $M_OPTS -Dmaven.test.failure.ignore "
-if [ -s inclusions.txt ]; then OPTS+=" -Dsurefire.includesFile=$PWD/inclusions.txt"; sed -i '/\\/ITest/d' $PWD/inclusions.txt;fi
+if [ -s inclusions.txt ]; then OPTS+=" -Dsurefire.includesFile=$PWD/inclusions.txt";fi

Review Comment:
   Just to double check: Have you checked that the tests were run by the CI?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] zabetak commented on a diff in pull request #3399: HIVE-26021: Change integration tests under DBInstallBase to Checkin tests

Posted by GitBox <gi...@apache.org>.
zabetak commented on code in PR #3399:
URL: https://github.com/apache/hive/pull/3399#discussion_r904757984


##########
standalone-metastore/DEV-README:
##########
@@ -23,47 +23,34 @@ quick test can be done for the unit tests and more in depth testing as part
 of the checkin tests.
 
 --------------------------------------------------------------------------------
-There are integration tests for testing installation and upgrade of the
+There are checkin tests for testing installation and upgrade of the
 metastore on Derby, MySQL (actually MariaDB is used), Oracle, Postgres, and SQLServer.
 
-Each ITest runs two tests, one that installs the latest version of the
+For each DB type we runs two tests, one that installs the latest version of the
 database and one that installs the latest version minus one and then upgrades
 the database.
 
-To run the tests you will need to explicitly turn on integration testing by
-setting skipITests variable to false. The tests rely on Docker so the latter
-needs to be installed and configured properly (e.g., memory more than 3.5GB).
-
-Run all tests:
-
-mvn verify -DskipITests=false -Dtest=nosuch
+The tests (except Derby) rely on Docker so the latter needs to be installed and configured
+properly (e.g., memory more than 3.5GB).
 
 Run a single test:
 
-mvn verify -DskipITests=false -Dit.test=ITestMysql -Dtest=nosuch
+mvn test -Dtest.groups=MetastoreCheckinTest -Dtest=ITestDerby
 
 Supported databases for testing:
--Dit.test=ITestDerby
--Dit.test=ITestMysql
--Dit.test=ITestOracle
--Dit.test=ITestPostgres
--Dit.test=ITestMssql
+ITestDerby

Review Comment:
   Good catch. I originally wanted to keep the same names but due to various problems in CI I was forced to rename them to avoid introducing an ugly workaround. I will fix this during merge!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] zabetak commented on a diff in pull request #3399: HIVE-26021: Change integration tests under DBInstallBase to Checkin tests

Posted by GitBox <gi...@apache.org>.
zabetak commented on code in PR #3399:
URL: https://github.com/apache/hive/pull/3399#discussion_r904770029


##########
Jenkinsfile:
##########
@@ -279,15 +279,6 @@ reinit_metastore $dbType
 time docker rm -f dev_$dbType || true
 '''
           }
-          stage('verify') {

Review Comment:
   In principle the tests in `verify` stage are more complete so could potentially replace completely all the other stages.
   
   However, I find the `init-metastore` stage still useful cause it launches the various types of metastores using the hive-dev-box logic outside the unit test framework (classpaths may be different etc). This is closer to how a metastore is used in production so I think it is useful to have them as well.
   
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] zabetak closed pull request #3399: HIVE-26021: Change integration tests under DBInstallBase to Checkin tests

Posted by GitBox <gi...@apache.org>.
zabetak closed pull request #3399: HIVE-26021: Change integration tests under DBInstallBase to Checkin tests
URL: https://github.com/apache/hive/pull/3399


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org