[jira] [Work logged] (HIVE-26500) Improve TestHiveMetastore
[
https://issues.apache.org/jira/browse/HIVE-26500?focusedWorklogId=806627&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-806627
]
ASF GitHub Bot logged work on HIVE-26500:
-
Author: ASF GitHub Bot
Created on: 07/Sep/22 08:43
Start Date: 07/Sep/22 08:43
Worklog Time Spent: 10m
Work Description: abstractdog merged PR #3556:
URL: https://github.com/apache/hive/pull/3556
Issue Time Tracking
---
Worklog Id: (was: 806627)
Time Spent: 2h 10m (was: 2h)
> Improve TestHiveMetastore
> -
>
> Key: HIVE-26500
> URL: https://issues.apache.org/jira/browse/HIVE-26500
> Project: Hive
> Issue Type: Improvement
>Reporter: László Bodor
>Assignee: László Bodor
>Priority: Major
> Labels: pull-request-available
> Time Spent: 2h 10m
> Remaining Estimate: 0h
>
> {code}
> Thread.sleep(5 * 1000); // give HMS time to handle close request
> int numObjectsAfterClose = getJDOPersistanceManagerCacheSize();
> assertTrue(numObjectsBeforeClose == numObjectsAfterClose);
> {code}
> 1. easy fix: assertTrue(numObjectsBeforeClose == numObjectsAfterClose);
> this tells nothing in case of assertionerror:
> {code}
> java.lang.AssertionError
> at org.junit.Assert.fail(Assert.java:87)
> at org.junit.Assert.assertTrue(Assert.java:42)
> at org.junit.Assert.assertTrue(Assert.java:53)
> at
> org.apache.hadoop.hive.metastore.TestHiveMetaStore.testJDOPersistanceManagerCleanup(TestHiveMetaStore.java:3298)
> {code}
> should tell a more informative message with particular values
> 2. harder fix, maybe do it later
> {code}
> Thread.sleep(5 * 1000); // give HMS time to handle close request
> {code}
> sleep is always dangerous, cannot make sure it handled everything that we
> expected
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Work logged] (HIVE-26500) Improve TestHiveMetastore
[
https://issues.apache.org/jira/browse/HIVE-26500?focusedWorklogId=806626&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-806626
]
ASF GitHub Bot logged work on HIVE-26500:
-
Author: ASF GitHub Bot
Created on: 07/Sep/22 08:42
Start Date: 07/Sep/22 08:42
Worklog Time Spent: 10m
Work Description: abstractdog commented on PR #3556:
URL: https://github.com/apache/hive/pull/3556#issuecomment-1239090642
> LGTM. Sonar has some complains.
https://sonarcloud.io/project/issues?resolved=false&types=CODE_SMELL&pullRequest=3556&id=apache_hive&open=AYMS1ejLkC-HJGhR5irJ
>
> I am not sure how we are handling them..
sonar is absolutely right when saying "Using Thread.sleep in a test is just
generally a bad idea.", that's exactly what I fixed with this, but it hasn't
realized :)
Issue Time Tracking
---
Worklog Id: (was: 806626)
Time Spent: 2h (was: 1h 50m)
> Improve TestHiveMetastore
> -
>
> Key: HIVE-26500
> URL: https://issues.apache.org/jira/browse/HIVE-26500
> Project: Hive
> Issue Type: Improvement
>Reporter: László Bodor
>Assignee: László Bodor
>Priority: Major
> Labels: pull-request-available
> Time Spent: 2h
> Remaining Estimate: 0h
>
> {code}
> Thread.sleep(5 * 1000); // give HMS time to handle close request
> int numObjectsAfterClose = getJDOPersistanceManagerCacheSize();
> assertTrue(numObjectsBeforeClose == numObjectsAfterClose);
> {code}
> 1. easy fix: assertTrue(numObjectsBeforeClose == numObjectsAfterClose);
> this tells nothing in case of assertionerror:
> {code}
> java.lang.AssertionError
> at org.junit.Assert.fail(Assert.java:87)
> at org.junit.Assert.assertTrue(Assert.java:42)
> at org.junit.Assert.assertTrue(Assert.java:53)
> at
> org.apache.hadoop.hive.metastore.TestHiveMetaStore.testJDOPersistanceManagerCleanup(TestHiveMetaStore.java:3298)
> {code}
> should tell a more informative message with particular values
> 2. harder fix, maybe do it later
> {code}
> Thread.sleep(5 * 1000); // give HMS time to handle close request
> {code}
> sleep is always dangerous, cannot make sure it handled everything that we
> expected
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Work logged] (HIVE-26500) Improve TestHiveMetastore
[
https://issues.apache.org/jira/browse/HIVE-26500?focusedWorklogId=806543&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-806543
]
ASF GitHub Bot logged work on HIVE-26500:
-
Author: ASF GitHub Bot
Created on: 06/Sep/22 22:37
Start Date: 06/Sep/22 22:37
Worklog Time Spent: 10m
Work Description: ayushtkn commented on code in PR #3556:
URL: https://github.com/apache/hive/pull/3556#discussion_r964231400
##
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/MetastoreTestUtils.java:
##
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore.utils;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Common utils for metastore tests.
+ */
+public class MetastoreTestUtils {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(MetastoreTestUtils.class);
+
+ /**
+ * This method can run an assertion wrapped in to a runnable, and keep
retrying it for a certain amount of time.
+ * It can be useful when the assertion doesn't necessarily pass immediately,
and it would be hard
+ * to mock into production code in order to wait for some conditions
properly. Instead, it just
+ * waits, but not like a constant time before a single attempt (which is
easy, but errorprone).
+ * @param assertionContext
+ * @param runnable
+ * @param msBetweenAssertionAttempts
+ * @param msOverallTimeout
+ * @throws Exception
+ */
+ public static void waitForAssertion(String assertionContext, Runnable
assertionRunnable, int msBetweenAssertionAttempts,
Review Comment:
Looks good
Issue Time Tracking
---
Worklog Id: (was: 806543)
Time Spent: 1h 50m (was: 1h 40m)
> Improve TestHiveMetastore
> -
>
> Key: HIVE-26500
> URL: https://issues.apache.org/jira/browse/HIVE-26500
> Project: Hive
> Issue Type: Improvement
>Reporter: László Bodor
>Assignee: László Bodor
>Priority: Major
> Labels: pull-request-available
> Time Spent: 1h 50m
> Remaining Estimate: 0h
>
> {code}
> Thread.sleep(5 * 1000); // give HMS time to handle close request
> int numObjectsAfterClose = getJDOPersistanceManagerCacheSize();
> assertTrue(numObjectsBeforeClose == numObjectsAfterClose);
> {code}
> 1. easy fix: assertTrue(numObjectsBeforeClose == numObjectsAfterClose);
> this tells nothing in case of assertionerror:
> {code}
> java.lang.AssertionError
> at org.junit.Assert.fail(Assert.java:87)
> at org.junit.Assert.assertTrue(Assert.java:42)
> at org.junit.Assert.assertTrue(Assert.java:53)
> at
> org.apache.hadoop.hive.metastore.TestHiveMetaStore.testJDOPersistanceManagerCleanup(TestHiveMetaStore.java:3298)
> {code}
> should tell a more informative message with particular values
> 2. harder fix, maybe do it later
> {code}
> Thread.sleep(5 * 1000); // give HMS time to handle close request
> {code}
> sleep is always dangerous, cannot make sure it handled everything that we
> expected
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Work logged] (HIVE-26500) Improve TestHiveMetastore
[
https://issues.apache.org/jira/browse/HIVE-26500?focusedWorklogId=806490&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-806490
]
ASF GitHub Bot logged work on HIVE-26500:
-
Author: ASF GitHub Bot
Created on: 06/Sep/22 17:20
Start Date: 06/Sep/22 17:20
Worklog Time Spent: 10m
Work Description: sonarcloud[bot] commented on PR #3556:
URL: https://github.com/apache/hive/pull/3556#issuecomment-1238445963
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3556)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3556&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3556&resolved=false&types=BUG)
[0
Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3556&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3556&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3556&resolved=false&types=VULNERABILITY)
[0
Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3556&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3556&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3556&resolved=false&types=SECURITY_HOTSPOT)
[0 Security
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3556&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3556&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3556&resolved=false&types=CODE_SMELL)
[1 Code
Smell](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3556&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3556&metric=coverage&view=list)
No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3556&metric=duplicated_lines_density&view=list)
No Duplication information
Issue Time Tracking
---
Worklog Id: (was: 806490)
Time Spent: 1h 40m (was: 1.5h)
> Improve TestHiveMetastore
> -
>
> Key: HIVE-26500
> URL: https://issues.apache.org/jira/browse/HIVE-26500
> Project: Hive
> Issue Type: Improvement
>Reporter: László Bodor
>Assignee: László Bodor
>Priority: Major
> Labels: pull-request-available
> Time Spent: 1h 40m
> Remaining Estimate: 0h
>
> {code}
> Thread.sleep(5 * 1000); // give HMS time to handle close request
> int numObjectsAfterClose = getJDOPersistanceManagerCacheSize();
> assertTrue(numObjectsBeforeClose == numObjectsAfterClose);
> {code}
> 1. easy fix: assertTrue(numObjectsBeforeClose == numObjectsAfterClose);
> this tells nothing in case of assertionerror:
> {code}
> java.lang.AssertionError
> at org.junit.Assert.fail(Assert.java:87)
> at org.junit.Assert.assertTrue(Assert.java:42)
> at org.junit.Assert.assertTrue(Assert.java:53)
> at
> org.apache.hadoop.hive.metastore.TestHiveMetaStore.testJDOPersistanceManagerCleanup(TestHiveMetaStore.java
[jira] [Work logged] (HIVE-26500) Improve TestHiveMetastore
[
https://issues.apache.org/jira/browse/HIVE-26500?focusedWorklogId=806347&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-806347
]
ASF GitHub Bot logged work on HIVE-26500:
-
Author: ASF GitHub Bot
Created on: 06/Sep/22 12:45
Start Date: 06/Sep/22 12:45
Worklog Time Spent: 10m
Work Description: sonarcloud[bot] commented on PR #3556:
URL: https://github.com/apache/hive/pull/3556#issuecomment-1238102045
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3556)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3556&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3556&resolved=false&types=BUG)
[0
Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3556&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3556&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3556&resolved=false&types=VULNERABILITY)
[0
Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3556&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3556&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3556&resolved=false&types=SECURITY_HOTSPOT)
[0 Security
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3556&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3556&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3556&resolved=false&types=CODE_SMELL)
[5 Code
Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3556&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3556&metric=coverage&view=list)
No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3556&metric=duplicated_lines_density&view=list)
No Duplication information
Issue Time Tracking
---
Worklog Id: (was: 806347)
Time Spent: 1.5h (was: 1h 20m)
> Improve TestHiveMetastore
> -
>
> Key: HIVE-26500
> URL: https://issues.apache.org/jira/browse/HIVE-26500
> Project: Hive
> Issue Type: Improvement
>Reporter: László Bodor
>Assignee: László Bodor
>Priority: Major
> Labels: pull-request-available
> Time Spent: 1.5h
> Remaining Estimate: 0h
>
> {code}
> Thread.sleep(5 * 1000); // give HMS time to handle close request
> int numObjectsAfterClose = getJDOPersistanceManagerCacheSize();
> assertTrue(numObjectsBeforeClose == numObjectsAfterClose);
> {code}
> 1. easy fix: assertTrue(numObjectsBeforeClose == numObjectsAfterClose);
> this tells nothing in case of assertionerror:
> {code}
> java.lang.AssertionError
> at org.junit.Assert.fail(Assert.java:87)
> at org.junit.Assert.assertTrue(Assert.java:42)
> at org.junit.Assert.assertTrue(Assert.java:53)
> at
> org.apache.hadoop.hive.metastore.TestHiveMetaStore.testJDOPersistanceManagerCleanup(TestHiveMetaStore.java:
[jira] [Work logged] (HIVE-26500) Improve TestHiveMetastore
[
https://issues.apache.org/jira/browse/HIVE-26500?focusedWorklogId=806342&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-806342
]
ASF GitHub Bot logged work on HIVE-26500:
-
Author: ASF GitHub Bot
Created on: 06/Sep/22 12:12
Start Date: 06/Sep/22 12:12
Worklog Time Spent: 10m
Work Description: abstractdog commented on code in PR #3556:
URL: https://github.com/apache/hive/pull/3556#discussion_r963625098
##
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/MetastoreTestUtils.java:
##
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore.utils;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Common utils for metastore tests.
+ */
+public class MetastoreTestUtils {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(MetastoreTestUtils.class);
+
+ /**
+ * This method can run an assertion wrapped in to a runnable, and keep
retrying it for a certain amount of time.
+ * It can be useful when the assertion doesn't necessarily pass immediately,
and it would be hard
+ * to mock into production code in order to wait for some conditions
properly. Instead, it just
+ * waits, but not like a constant time before a single attempt (which is
easy, but errorprone).
+ * @param assertionContext
+ * @param runnable
+ * @param msBetweenAssertionAttempts
+ * @param msOverallTimeout
+ * @throws Exception
+ */
+ public static void waitForAssertion(String assertionContext, Runnable
assertionRunnable, int msBetweenAssertionAttempts,
Review Comment:
good point, however, there is no direct dependency on hadoop-common in
metastore (only provided, optional, etc.)
I like GenericTestUtils, I implemented pretty much the same, but I think
mine is maybe better in a sense that the caller doesn't have to create a
boolean function and handle TimeoutException and stuff, instead just write the
very same assertion in a runnable which would be originally there without this
timeout hack...if the code block throws assertion error, unit test fails
automatically as a unit test is expected, does this make sense to you?
other than that, anything you find better in hadoop's implementation in
particular, let me know and I can adapt
Issue Time Tracking
---
Worklog Id: (was: 806342)
Time Spent: 1h 20m (was: 1h 10m)
> Improve TestHiveMetastore
> -
>
> Key: HIVE-26500
> URL: https://issues.apache.org/jira/browse/HIVE-26500
> Project: Hive
> Issue Type: Improvement
>Reporter: László Bodor
>Assignee: László Bodor
>Priority: Major
> Labels: pull-request-available
> Time Spent: 1h 20m
> Remaining Estimate: 0h
>
> {code}
> Thread.sleep(5 * 1000); // give HMS time to handle close request
> int numObjectsAfterClose = getJDOPersistanceManagerCacheSize();
> assertTrue(numObjectsBeforeClose == numObjectsAfterClose);
> {code}
> 1. easy fix: assertTrue(numObjectsBeforeClose == numObjectsAfterClose);
> this tells nothing in case of assertionerror:
> {code}
> java.lang.AssertionError
> at org.junit.Assert.fail(Assert.java:87)
> at org.junit.Assert.assertTrue(Assert.java:42)
> at org.junit.Assert.assertTrue(Assert.java:53)
> at
> org.apache.hadoop.hive.metastore.TestHiveMetaStore.testJDOPersistanceManagerCleanup(TestHiveMetaStore.java:3298)
> {code}
> should tell a more informative message with particular values
> 2. harder fix, maybe do it later
> {code}
> Thread.sleep(5 * 1000); // give HMS time to handle close request
> {code}
> sleep is always dangerous, cannot make sure it handled everything that we
> expected
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Work logged] (HIVE-26500) Improve TestHiveMetastore
[
https://issues.apache.org/jira/browse/HIVE-26500?focusedWorklogId=806341&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-806341
]
ASF GitHub Bot logged work on HIVE-26500:
-
Author: ASF GitHub Bot
Created on: 06/Sep/22 12:11
Start Date: 06/Sep/22 12:11
Worklog Time Spent: 10m
Work Description: abstractdog commented on code in PR #3556:
URL: https://github.com/apache/hive/pull/3556#discussion_r963625098
##
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/MetastoreTestUtils.java:
##
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore.utils;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Common utils for metastore tests.
+ */
+public class MetastoreTestUtils {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(MetastoreTestUtils.class);
+
+ /**
+ * This method can run an assertion wrapped in to a runnable, and keep
retrying it for a certain amount of time.
+ * It can be useful when the assertion doesn't necessarily pass immediately,
and it would be hard
+ * to mock into production code in order to wait for some conditions
properly. Instead, it just
+ * waits, but not like a constant time before a single attempt (which is
easy, but errorprone).
+ * @param assertionContext
+ * @param runnable
+ * @param msBetweenAssertionAttempts
+ * @param msOverallTimeout
+ * @throws Exception
+ */
+ public static void waitForAssertion(String assertionContext, Runnable
assertionRunnable, int msBetweenAssertionAttempts,
Review Comment:
good point, however, there is no direct dependency on hadoop-common in
metastore (only provided, optional, etc.)
I like GenericTestUtils, I implemented pretty much the same, but I think
mine is maybe better in a sense that the caller doesn't have to create a
boolean function and handle TimeoutException and stuff, instead just write the
very same assertion in a runnable which would be originally there without this
timeout hack...if the code block throws assertion error, unit test fails
automatically as a unit test is expected
##
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/MetastoreTestUtils.java:
##
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore.utils;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Common utils for metastore tests.
+ */
+public class MetastoreTestUtils {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(MetastoreTestUtils.class);
+
+ /**
+ * This method can run an assertion wrapped in to a runnable, and keep
retrying it for a certain amount of time.
+ * It can be useful when the assertion doesn't necessarily pass immediately,
and it would be hard
+ * to mock into production code in order to wait for some conditions
properly. Instead, it just
+ * waits, but not like a constant time before a single attempt (which is
easy, but errorprone).
+ * @param assertionContext
+ * @param runnable
+ * @param msBetweenAssertionAttempts
+ * @param msOverallTimeout
+ * @throws Exception
+ */
+ public static void waitForAssertion(String assertionContext, Runnabl
[jira] [Work logged] (HIVE-26500) Improve TestHiveMetastore
[
https://issues.apache.org/jira/browse/HIVE-26500?focusedWorklogId=806339&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-806339
]
ASF GitHub Bot logged work on HIVE-26500:
-
Author: ASF GitHub Bot
Created on: 06/Sep/22 12:01
Start Date: 06/Sep/22 12:01
Worklog Time Spent: 10m
Work Description: abstractdog commented on code in PR #3556:
URL: https://github.com/apache/hive/pull/3556#discussion_r963615677
##
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java:
##
@@ -3306,22 +3314,38 @@ public void testJDOPersistanceManagerCleanup() throws
Exception {
return;
}
-int numObjectsBeforeClose = getJDOPersistanceManagerCacheSize();
+int numObjectsBeforeUse = getJDOPersistanceManagerCacheSize();
HiveMetaStoreClient closingClient = new HiveMetaStoreClient(conf);
closingClient.getAllDatabases();
closingClient.close();
-Thread.sleep(5 * 1000); // give HMS time to handle close request
-int numObjectsAfterClose = getJDOPersistanceManagerCacheSize();
-assertTrue(numObjectsBeforeClose == numObjectsAfterClose);
+MetastoreTestUtils.waitForAssertion("Checking pm cachesize after client
close", new Runnable() {
+ @Override
+ public void run() {
+int numObjectsAfterClose = getJDOPersistanceManagerCacheSize();
+assertTrue(String.format("numObjectsBeforeUse: %d !=
numObjectsAfterClose: %d", numObjectsBeforeUse,
+numObjectsAfterClose), numObjectsBeforeUse ==
numObjectsAfterClose);
+ }
+}, 500, 3);
+
+int numObjectsAfterClose = getJDOPersistanceManagerCacheSize();
HiveMetaStoreClient nonClosingClient = new HiveMetaStoreClient(conf);
nonClosingClient.getAllDatabases();
// Drop connection without calling close. HMS thread deleteContext
// will trigger cleanup
nonClosingClient.getTTransport().close();
-Thread.sleep(5 * 1000);
-int numObjectsAfterDroppedConnection =
getJDOPersistanceManagerCacheSize();
-assertTrue(numObjectsAfterClose == numObjectsAfterDroppedConnection);
+
+MetastoreTestUtils.waitForAssertion("Checking pm cachesize after transport
close", new Runnable() {
+ @Override
+ public void run() {
+int numObjectsAfterDroppedConnection =
getJDOPersistanceManagerCacheSize();
+assertTrue(String.format("numObjectsAfterClose: %d !=
numObjectsAfterDroppedConnection: %d",
+numObjectsAfterClose, numObjectsAfterDroppedConnection),
+numObjectsAfterClose == numObjectsAfterDroppedConnection);
+ }
+}, 500, 3);
Review Comment:
ack
Issue Time Tracking
---
Worklog Id: (was: 806339)
Time Spent: 1h (was: 50m)
> Improve TestHiveMetastore
> -
>
> Key: HIVE-26500
> URL: https://issues.apache.org/jira/browse/HIVE-26500
> Project: Hive
> Issue Type: Improvement
>Reporter: László Bodor
>Assignee: László Bodor
>Priority: Major
> Labels: pull-request-available
> Time Spent: 1h
> Remaining Estimate: 0h
>
> {code}
> Thread.sleep(5 * 1000); // give HMS time to handle close request
> int numObjectsAfterClose = getJDOPersistanceManagerCacheSize();
> assertTrue(numObjectsBeforeClose == numObjectsAfterClose);
> {code}
> 1. easy fix: assertTrue(numObjectsBeforeClose == numObjectsAfterClose);
> this tells nothing in case of assertionerror:
> {code}
> java.lang.AssertionError
> at org.junit.Assert.fail(Assert.java:87)
> at org.junit.Assert.assertTrue(Assert.java:42)
> at org.junit.Assert.assertTrue(Assert.java:53)
> at
> org.apache.hadoop.hive.metastore.TestHiveMetaStore.testJDOPersistanceManagerCleanup(TestHiveMetaStore.java:3298)
> {code}
> should tell a more informative message with particular values
> 2. harder fix, maybe do it later
> {code}
> Thread.sleep(5 * 1000); // give HMS time to handle close request
> {code}
> sleep is always dangerous, cannot make sure it handled everything that we
> expected
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Work logged] (HIVE-26500) Improve TestHiveMetastore
[
https://issues.apache.org/jira/browse/HIVE-26500?focusedWorklogId=806338&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-806338
]
ASF GitHub Bot logged work on HIVE-26500:
-
Author: ASF GitHub Bot
Created on: 06/Sep/22 12:01
Start Date: 06/Sep/22 12:01
Worklog Time Spent: 10m
Work Description: abstractdog commented on code in PR #3556:
URL: https://github.com/apache/hive/pull/3556#discussion_r963615346
##
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java:
##
@@ -3306,22 +3314,38 @@ public void testJDOPersistanceManagerCleanup() throws
Exception {
return;
}
-int numObjectsBeforeClose = getJDOPersistanceManagerCacheSize();
+int numObjectsBeforeUse = getJDOPersistanceManagerCacheSize();
HiveMetaStoreClient closingClient = new HiveMetaStoreClient(conf);
closingClient.getAllDatabases();
closingClient.close();
-Thread.sleep(5 * 1000); // give HMS time to handle close request
-int numObjectsAfterClose = getJDOPersistanceManagerCacheSize();
-assertTrue(numObjectsBeforeClose == numObjectsAfterClose);
+MetastoreTestUtils.waitForAssertion("Checking pm cachesize after client
close", new Runnable() {
+ @Override
+ public void run() {
+int numObjectsAfterClose = getJDOPersistanceManagerCacheSize();
+assertTrue(String.format("numObjectsBeforeUse: %d !=
numObjectsAfterClose: %d", numObjectsBeforeUse,
+numObjectsAfterClose), numObjectsBeforeUse ==
numObjectsAfterClose);
+ }
+}, 500, 3);
Review Comment:
ack
Issue Time Tracking
---
Worklog Id: (was: 806338)
Time Spent: 50m (was: 40m)
> Improve TestHiveMetastore
> -
>
> Key: HIVE-26500
> URL: https://issues.apache.org/jira/browse/HIVE-26500
> Project: Hive
> Issue Type: Improvement
>Reporter: László Bodor
>Assignee: László Bodor
>Priority: Major
> Labels: pull-request-available
> Time Spent: 50m
> Remaining Estimate: 0h
>
> {code}
> Thread.sleep(5 * 1000); // give HMS time to handle close request
> int numObjectsAfterClose = getJDOPersistanceManagerCacheSize();
> assertTrue(numObjectsBeforeClose == numObjectsAfterClose);
> {code}
> 1. easy fix: assertTrue(numObjectsBeforeClose == numObjectsAfterClose);
> this tells nothing in case of assertionerror:
> {code}
> java.lang.AssertionError
> at org.junit.Assert.fail(Assert.java:87)
> at org.junit.Assert.assertTrue(Assert.java:42)
> at org.junit.Assert.assertTrue(Assert.java:53)
> at
> org.apache.hadoop.hive.metastore.TestHiveMetaStore.testJDOPersistanceManagerCleanup(TestHiveMetaStore.java:3298)
> {code}
> should tell a more informative message with particular values
> 2. harder fix, maybe do it later
> {code}
> Thread.sleep(5 * 1000); // give HMS time to handle close request
> {code}
> sleep is always dangerous, cannot make sure it handled everything that we
> expected
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Work logged] (HIVE-26500) Improve TestHiveMetastore
[
https://issues.apache.org/jira/browse/HIVE-26500?focusedWorklogId=806336&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-806336
]
ASF GitHub Bot logged work on HIVE-26500:
-
Author: ASF GitHub Bot
Created on: 06/Sep/22 11:57
Start Date: 06/Sep/22 11:57
Worklog Time Spent: 10m
Work Description: abstractdog commented on code in PR #3556:
URL: https://github.com/apache/hive/pull/3556#discussion_r963612024
##
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java:
##
@@ -168,16 +169,16 @@ public void testNameMethods() {
try {
List testVals = client.partitionNameToVals(partName);
- assertTrue("Values from name are incorrect", vals.equals(testVals));
+ assertTrue("Values from name are incorrect: " + testVals,
vals.equals(testVals));
Map testSpec = client.partitionNameToSpec(partName);
- assertTrue("Spec from name is incorrect", spec.equals(testSpec));
+ assertTrue("Spec from name is incorrect: " + testSpec,
spec.equals(testSpec));
List emptyVals = client.partitionNameToVals("");
- assertTrue("Values should be empty", emptyVals.size() == 0);
+ assertTrue("Values should be empty, instead its size is: " +
emptyVals.size(), emptyVals.size() == 0);
Map emptySpec = client.partitionNameToSpec("");
- assertTrue("Spec should be empty", emptySpec.size() == 0);
+ assertTrue("Spec should be empty, instead its size is: " +
emptySpec.size(), emptySpec.size() == 0);
Review Comment:
ack
Issue Time Tracking
---
Worklog Id: (was: 806336)
Time Spent: 40m (was: 0.5h)
> Improve TestHiveMetastore
> -
>
> Key: HIVE-26500
> URL: https://issues.apache.org/jira/browse/HIVE-26500
> Project: Hive
> Issue Type: Improvement
>Reporter: László Bodor
>Assignee: László Bodor
>Priority: Major
> Labels: pull-request-available
> Time Spent: 40m
> Remaining Estimate: 0h
>
> {code}
> Thread.sleep(5 * 1000); // give HMS time to handle close request
> int numObjectsAfterClose = getJDOPersistanceManagerCacheSize();
> assertTrue(numObjectsBeforeClose == numObjectsAfterClose);
> {code}
> 1. easy fix: assertTrue(numObjectsBeforeClose == numObjectsAfterClose);
> this tells nothing in case of assertionerror:
> {code}
> java.lang.AssertionError
> at org.junit.Assert.fail(Assert.java:87)
> at org.junit.Assert.assertTrue(Assert.java:42)
> at org.junit.Assert.assertTrue(Assert.java:53)
> at
> org.apache.hadoop.hive.metastore.TestHiveMetaStore.testJDOPersistanceManagerCleanup(TestHiveMetaStore.java:3298)
> {code}
> should tell a more informative message with particular values
> 2. harder fix, maybe do it later
> {code}
> Thread.sleep(5 * 1000); // give HMS time to handle close request
> {code}
> sleep is always dangerous, cannot make sure it handled everything that we
> expected
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Work logged] (HIVE-26500) Improve TestHiveMetastore
[
https://issues.apache.org/jira/browse/HIVE-26500?focusedWorklogId=805513&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-805513
]
ASF GitHub Bot logged work on HIVE-26500:
-
Author: ASF GitHub Bot
Created on: 01/Sep/22 12:56
Start Date: 01/Sep/22 12:56
Worklog Time Spent: 10m
Work Description: ayushtkn commented on code in PR #3556:
URL: https://github.com/apache/hive/pull/3556#discussion_r960610224
##
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java:
##
@@ -168,16 +169,16 @@ public void testNameMethods() {
try {
List testVals = client.partitionNameToVals(partName);
- assertTrue("Values from name are incorrect", vals.equals(testVals));
+ assertTrue("Values from name are incorrect: " + testVals,
vals.equals(testVals));
Map testSpec = client.partitionNameToSpec(partName);
- assertTrue("Spec from name is incorrect", spec.equals(testSpec));
+ assertTrue("Spec from name is incorrect: " + testSpec,
spec.equals(testSpec));
List emptyVals = client.partitionNameToVals("");
- assertTrue("Values should be empty", emptyVals.size() == 0);
+ assertTrue("Values should be empty, instead its size is: " +
emptyVals.size(), emptyVals.size() == 0);
Map emptySpec = client.partitionNameToSpec("");
- assertTrue("Spec should be empty", emptySpec.size() == 0);
+ assertTrue("Spec should be empty, instead its size is: " +
emptySpec.size(), emptySpec.size() == 0);
Review Comment:
Can be changed to ``assertEquals``
##
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java:
##
@@ -3306,22 +3314,38 @@ public void testJDOPersistanceManagerCleanup() throws
Exception {
return;
}
-int numObjectsBeforeClose = getJDOPersistanceManagerCacheSize();
+int numObjectsBeforeUse = getJDOPersistanceManagerCacheSize();
HiveMetaStoreClient closingClient = new HiveMetaStoreClient(conf);
closingClient.getAllDatabases();
closingClient.close();
-Thread.sleep(5 * 1000); // give HMS time to handle close request
-int numObjectsAfterClose = getJDOPersistanceManagerCacheSize();
-assertTrue(numObjectsBeforeClose == numObjectsAfterClose);
+MetastoreTestUtils.waitForAssertion("Checking pm cachesize after client
close", new Runnable() {
+ @Override
+ public void run() {
+int numObjectsAfterClose = getJDOPersistanceManagerCacheSize();
+assertTrue(String.format("numObjectsBeforeUse: %d !=
numObjectsAfterClose: %d", numObjectsBeforeUse,
+numObjectsAfterClose), numObjectsBeforeUse ==
numObjectsAfterClose);
+ }
+}, 500, 3);
Review Comment:
Can use Lambda & assertTrue can be changed to ``assertEqual``?
Something like:
```
MetastoreTestUtils.waitForAssertion("Checking pm cachesize after client
close", () -> {
int numObjectsAfterClose = getJDOPersistanceManagerCacheSize();
assertEquals(String
.format("numObjectsBeforeUse: %d != numObjectsAfterClose: %d",
numObjectsBeforeUse, numObjectsAfterClose),
numObjectsBeforeUse, numObjectsAfterClose);
}, 500, 3);
```
##
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java:
##
@@ -3306,22 +3314,38 @@ public void testJDOPersistanceManagerCleanup() throws
Exception {
return;
}
-int numObjectsBeforeClose = getJDOPersistanceManagerCacheSize();
+int numObjectsBeforeUse = getJDOPersistanceManagerCacheSize();
HiveMetaStoreClient closingClient = new HiveMetaStoreClient(conf);
closingClient.getAllDatabases();
closingClient.close();
-Thread.sleep(5 * 1000); // give HMS time to handle close request
-int numObjectsAfterClose = getJDOPersistanceManagerCacheSize();
-assertTrue(numObjectsBeforeClose == numObjectsAfterClose);
+MetastoreTestUtils.waitForAssertion("Checking pm cachesize after client
close", new Runnable() {
+ @Override
+ public void run() {
+int numObjectsAfterClose = getJDOPersistanceManagerCacheSize();
+assertTrue(String.format("numObjectsBeforeUse: %d !=
numObjectsAfterClose: %d", numObjectsBeforeUse,
+numObjectsAfterClose), numObjectsBeforeUse ==
numObjectsAfterClose);
+ }
+}, 500, 3);
+
+int numObjectsAfterClose = getJDOPersistanceManagerCacheSize();
HiveMetaStoreClient nonClosingClient = new HiveMetaStoreClient(conf);
nonClosingClient.getAllDatabases();
// Drop connection without calling close. HMS thread deleteContext
// will trigger cleanup
nonClosingClient.getTTransport().close();
-Thread.sleep(5 * 1000);
-int numObjectsAfterDroppedConnection =
getJDOP
[jira] [Work logged] (HIVE-26500) Improve TestHiveMetastore
[
https://issues.apache.org/jira/browse/HIVE-26500?focusedWorklogId=804715&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-804715
]
ASF GitHub Bot logged work on HIVE-26500:
-
Author: ASF GitHub Bot
Created on: 30/Aug/22 08:39
Start Date: 30/Aug/22 08:39
Worklog Time Spent: 10m
Work Description: sonarcloud[bot] commented on PR #3556:
URL: https://github.com/apache/hive/pull/3556#issuecomment-1231353141
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3556)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3556&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3556&resolved=false&types=BUG)
[0
Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3556&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3556&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3556&resolved=false&types=VULNERABILITY)
[0
Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3556&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3556&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3556&resolved=false&types=SECURITY_HOTSPOT)
[0 Security
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3556&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3556&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3556&resolved=false&types=CODE_SMELL)
[10 Code
Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3556&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3556&metric=coverage&view=list)
No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3556&metric=duplicated_lines_density&view=list)
No Duplication information
Issue Time Tracking
---
Worklog Id: (was: 804715)
Time Spent: 20m (was: 10m)
> Improve TestHiveMetastore
> -
>
> Key: HIVE-26500
> URL: https://issues.apache.org/jira/browse/HIVE-26500
> Project: Hive
> Issue Type: Improvement
>Reporter: László Bodor
>Assignee: László Bodor
>Priority: Major
> Labels: pull-request-available
> Time Spent: 20m
> Remaining Estimate: 0h
>
> {code}
> Thread.sleep(5 * 1000); // give HMS time to handle close request
> int numObjectsAfterClose = getJDOPersistanceManagerCacheSize();
> assertTrue(numObjectsBeforeClose == numObjectsAfterClose);
> {code}
> 1. easy fix: assertTrue(numObjectsBeforeClose == numObjectsAfterClose);
> this tells nothing in case of assertionerror:
> {code}
> java.lang.AssertionError
> at org.junit.Assert.fail(Assert.java:87)
> at org.junit.Assert.assertTrue(Assert.java:42)
> at org.junit.Assert.assertTrue(Assert.java:53)
> at
> org.apache.hadoop.hive.metastore.TestHiveMetaStore.testJDOPersistanceManagerCleanup(TestHiveMetaStore.java:3298
[jira] [Work logged] (HIVE-26500) Improve TestHiveMetastore
[
https://issues.apache.org/jira/browse/HIVE-26500?focusedWorklogId=804658&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-804658
]
ASF GitHub Bot logged work on HIVE-26500:
-
Author: ASF GitHub Bot
Created on: 30/Aug/22 07:19
Start Date: 30/Aug/22 07:19
Worklog Time Spent: 10m
Work Description: abstractdog opened a new pull request, #3556:
URL: https://github.com/apache/hive/pull/3556
### What changes were proposed in this pull request?
### Why are the changes needed?
### Does this PR introduce _any_ user-facing change?
### How was this patch tested?
Issue Time Tracking
---
Worklog Id: (was: 804658)
Remaining Estimate: 0h
Time Spent: 10m
> Improve TestHiveMetastore
> -
>
> Key: HIVE-26500
> URL: https://issues.apache.org/jira/browse/HIVE-26500
> Project: Hive
> Issue Type: Improvement
>Reporter: László Bodor
>Assignee: László Bodor
>Priority: Major
> Time Spent: 10m
> Remaining Estimate: 0h
>
> {code}
> Thread.sleep(5 * 1000); // give HMS time to handle close request
> int numObjectsAfterClose = getJDOPersistanceManagerCacheSize();
> assertTrue(numObjectsBeforeClose == numObjectsAfterClose);
> {code}
> 1. easy fix: assertTrue(numObjectsBeforeClose == numObjectsAfterClose);
> this tells nothing in case of assertionerror:
> {code}
> java.lang.AssertionError
> at org.junit.Assert.fail(Assert.java:87)
> at org.junit.Assert.assertTrue(Assert.java:42)
> at org.junit.Assert.assertTrue(Assert.java:53)
> at
> org.apache.hadoop.hive.metastore.TestHiveMetaStore.testJDOPersistanceManagerCleanup(TestHiveMetaStore.java:3298)
> {code}
> should tell a more informative message with particular values
> 2. harder fix, maybe do it later
> {code}
> Thread.sleep(5 * 1000); // give HMS time to handle close request
> {code}
> sleep is always dangerous, cannot make sure it handled everything that we
> expected
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
