ngsg commented on code in PR #5771:
URL: https://github.com/apache/hive/pull/5771#discussion_r2181572515
##########
ql/src/test/org/apache/hadoop/hive/ql/metadata/TestSessionHiveMetastoreClientGetPartitionsTempTable.java:
##########
@@ -124,7 +124,7 @@ public void testGetPartitionsByNamesEmptyParts() throws
Exception {
getClient().getPartitionsByNames(DB_NAME, TABLE_NAME,
Lists.newArrayList("", ""));
}
- @Test(expected = MetaException.class)
+ @Test
Review Comment:
Thank you for pointing it out, that part wasn't actually easy for me (just a
small joke).
I have restored `TestSessionHiveMetastoreClient{Get,
List}PartitionsTempTable` to match the master branch. I observed that the
changes I made yesterday caused some test failures, and I noticed that now we
can pass those tests without modifying them.
---
The previous change was acceptable because this test (==
`super.testGetPartitionsByNamesNullTblName` ==
`TestGetPartitions#testGetPartitionsByNamesNullTblName`) is designed to pass
when the client just throws an exception for a null table name.
More specifically, `TestGetPartitions#testGetPartitionsByNamesNullTblName`
tests `HiveMetaStoreClient` and expects `NullPointerException` or
`TTransportException`.
In contrast, `TestSHMCGetPartitionsTT#testGetPartitionsByNamesNullTblName`
tests `SessionHiveMetaStoreClient`, which throws `MetaException`.
Therefore, `TestSHMCGetPartitionsTT#testGetPartitionsByNamesNullTblName`
includes an additional `@Test(expected = MetaException.class)` annotation,
since `TestGetPartitions` would not swallow `MetaException`.
At an early stage of the work, the modified `SessionHiveMetaStoreClient`
started throwing `NullPointerException`, and I decided to remove the expected
exceptions from the test to minimize code changes. However, after moving the
filterHook from the Hook layer to the Thrift layer as suggested by a reviewer,
it seems that the argument checking order can be the same as in the master
branch. So, I think we no longer need to modify the expected exceptions.
cf. `TestGetPartitions#testGetPartitionsByNamesNullTblName`
https://github.com/apache/hive/blob/279996b0858896a5b953745ec793c89d82378ea6/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestGetPartitions.java#L423-L431
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
