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/01 12:49:13 UTC
[GitHub] [hive] zabetak opened a new pull request, #3335: HIVE-26278: Add unit tests for Hive#getPartitionsByNames using batching
zabetak opened a new pull request, #3335:
URL: https://github.com/apache/hive/pull/3335
### What changes were proposed in this pull request?
New tests cases for more code coverage.
### Why are the changes needed?
Ensure that ValidWriteIdList is set when batching is involved in `Hive#getPartitionByNames`.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
`mvn test -Dtest=TestHiveMetaStoreClientApiArgumentsChecker`
--
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] kasakrisz commented on a diff in pull request #3335: HIVE-26278: Add unit tests for Hive#getPartitionsByNames using batching
Posted by GitBox <gi...@apache.org>.
kasakrisz commented on code in PR #3335:
URL: https://github.com/apache/hive/pull/3335#discussion_r887711290
##########
ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHiveMetaStoreClientApiArgumentsChecker.java:
##########
@@ -146,6 +151,26 @@ public void testGetPartitionsByNames3() throws HiveException {
hive.getPartitionsByNames(t, new ArrayList<>(), true);
}
+ @Test
+ public void testGetPartitionsByNamesWithSingleBatch() throws HiveException {
+ hive.getPartitionsByNames(t, Arrays.asList("Greece", "Italy"), true);
+ }
+
+ @Test
+ public void testGetPartitionsByNamesWithMultipleEqualSizeBatches()
+ throws HiveException {
+ List<String> names = Arrays.asList("Greece", "Italy", "France", "Spain");
+ hive.getPartitionsByNames(t, names, true);
+ }
+
+ @Test
+ public void testGetPartitionsByNamesWithMultipleUnequalSizeBatches()
+ throws HiveException {
+ List<String> names =
+ Arrays.asList("Greece", "Italy", "France", "Spain", "Hungary");
+ hive.getPartitionsByNames(t, names, true);
Review Comment:
Could you please add some asserts into these test methods like is the result of `getPartitionsByNames` contains partitions we expect.
Example:
```
List<Partitions> result = objectStore.getAllStoredProcedures(new ListStoredProcedureRequest("hive"));
Assert.assertEquals(5, result.size());
assertThat(result.get(0).getName, is("Greece"));
...
```
It would be also good to know how many times was the method `IMetaStoreClient.getPartitionsByNames` called. I think to get this a mock framework should be used. Mockito is already used in some Hive tests.
--
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] kgyrtkirk commented on a diff in pull request #3335: HIVE-26278: Add unit tests for Hive#getPartitionsByNames using batching
Posted by GitBox <gi...@apache.org>.
kgyrtkirk commented on code in PR #3335:
URL: https://github.com/apache/hive/pull/3335#discussion_r887711311
##########
ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHiveMetaStoreClientApiArgumentsChecker.java:
##########
@@ -146,6 +151,26 @@ public void testGetPartitionsByNames3() throws HiveException {
hive.getPartitionsByNames(t, new ArrayList<>(), true);
}
+ @Test
+ public void testGetPartitionsByNamesWithSingleBatch() throws HiveException {
+ hive.getPartitionsByNames(t, Arrays.asList("Greece", "Italy"), true);
Review Comment:
I don't see any assertions - how these tests are working?
--
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 #3335: HIVE-26278: Add unit tests for Hive#getPartitionsByNames using batching
Posted by GitBox <gi...@apache.org>.
zabetak commented on code in PR #3335:
URL: https://github.com/apache/hive/pull/3335#discussion_r887714198
##########
ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHiveMetaStoreClientApiArgumentsChecker.java:
##########
@@ -146,6 +151,26 @@ public void testGetPartitionsByNames3() throws HiveException {
hive.getPartitionsByNames(t, new ArrayList<>(), true);
}
+ @Test
+ public void testGetPartitionsByNamesWithSingleBatch() throws HiveException {
+ hive.getPartitionsByNames(t, Arrays.asList("Greece", "Italy"), true);
Review Comment:
The assertions are in `TestHiveMetaStoreClient`.
--
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 #3335: HIVE-26278: Add unit tests for Hive#getPartitionsByNames using batching
Posted by GitBox <gi...@apache.org>.
zabetak closed pull request #3335: HIVE-26278: Add unit tests for Hive#getPartitionsByNames using batching
URL: https://github.com/apache/hive/pull/3335
--
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 #3335: HIVE-26278: Add unit tests for Hive#getPartitionsByNames using batching
Posted by GitBox <gi...@apache.org>.
zabetak commented on code in PR #3335:
URL: https://github.com/apache/hive/pull/3335#discussion_r887676826
##########
ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHiveMetaStoreClientApiArgumentsChecker.java:
##########
@@ -38,10 +38,13 @@
import org.apache.hadoop.hive.ql.udf.generic.GenericUDFOPEqualOrGreaterThan;
import org.apache.hadoop.hive.serde2.typeinfo.TypeInfoFactory;
import org.apache.thrift.TException;
+
+import org.junit.Assert;
Review Comment:
Unused import, will remove before merging.
--
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 #3335: HIVE-26278: Add unit tests for Hive#getPartitionsByNames using batching
Posted by GitBox <gi...@apache.org>.
zabetak commented on code in PR #3335:
URL: https://github.com/apache/hive/pull/3335#discussion_r887717257
##########
ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHiveMetaStoreClientApiArgumentsChecker.java:
##########
@@ -146,6 +151,26 @@ public void testGetPartitionsByNames3() throws HiveException {
hive.getPartitionsByNames(t, new ArrayList<>(), true);
}
+ @Test
+ public void testGetPartitionsByNamesWithSingleBatch() throws HiveException {
+ hive.getPartitionsByNames(t, Arrays.asList("Greece", "Italy"), true);
+ }
+
+ @Test
+ public void testGetPartitionsByNamesWithMultipleEqualSizeBatches()
+ throws HiveException {
+ List<String> names = Arrays.asList("Greece", "Italy", "France", "Spain");
+ hive.getPartitionsByNames(t, names, true);
+ }
+
+ @Test
+ public void testGetPartitionsByNamesWithMultipleUnequalSizeBatches()
+ throws HiveException {
+ List<String> names =
+ Arrays.asList("Greece", "Italy", "France", "Spain", "Hungary");
+ hive.getPartitionsByNames(t, names, true);
Review Comment:
The goal of the tests in this class are explained in the javadoc of `TestHiveMetaStoreClient`.
> Tests in this class ensure that both tableId and validWriteIdList are sent from HS2
Asserting more things seems to be out of scope for this PR.
--
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