You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "liuxiaocs7 (via GitHub)" <gi...@apache.org> on 2023/03/21 12:20:43 UTC
[GitHub] [iceberg] liuxiaocs7 opened a new pull request, #7159: MR: Remove deprecate AssertHelpers
liuxiaocs7 opened a new pull request, #7159:
URL: https://github.com/apache/iceberg/pull/7159
- SubTask of https://github.com/apache/iceberg/issues/7094
Remove `AssertHelpers` in `iceberg-mr` module
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] nastra commented on a diff in pull request #7159: MR: Remove deprecate AssertHelpers
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #7159:
URL: https://github.com/apache/iceberg/pull/7159#discussion_r1143305000
##########
mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandlerNoScan.java:
##########
@@ -210,28 +210,25 @@ public void testCreateDropTable() throws TException, IOException, InterruptedExc
shell.executeStatement("DROP TABLE customers");
// Check if the table was really dropped even from the Catalog
- AssertHelpers.assertThrows(
- "should throw exception",
- NoSuchTableException.class,
- "Table does not exist",
- () -> {
- testTables.loadTable(identifier);
- });
+ Assertions.assertThatThrownBy(
+ () -> {
Review Comment:
nit: the { } aren't necessary here and further below
##########
mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandlerNoScan.java:
##########
@@ -272,13 +269,12 @@ public void testCreateDropTableNonDefaultCatalog() throws TException, Interrupte
shell.executeStatement("DROP TABLE default.customers");
// Check if the table was really dropped even from the Catalog
- AssertHelpers.assertThrows(
- "should throw exception",
- NoSuchTableException.class,
- "Table does not exist",
- () -> {
- testTables.loadTable(identifier);
- });
+ Assertions.assertThatThrownBy(
+ () -> {
+ testTables.loadTable(identifier);
+ })
+ .isInstanceOf(NoSuchTableException.class)
+ .hasMessageStartingWith("Table does not exist");
Review Comment:
nit: should this also include the table name like the others further below?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] jackye1995 merged pull request #7159: MR: Remove deprecate AssertHelpers
Posted by "jackye1995 (via GitHub)" <gi...@apache.org>.
jackye1995 merged PR #7159:
URL: https://github.com/apache/iceberg/pull/7159
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] liuxiaocs7 commented on pull request #7159: MR: Remove deprecate AssertHelpers
Posted by "liuxiaocs7 (via GitHub)" <gi...@apache.org>.
liuxiaocs7 commented on PR #7159:
URL: https://github.com/apache/iceberg/pull/7159#issuecomment-1477755302
Hi, @nastra, @jackye1995, please help me take a look, thanks!
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] liuxiaocs7 commented on a diff in pull request #7159: MR: Remove deprecate AssertHelpers
Posted by "liuxiaocs7 (via GitHub)" <gi...@apache.org>.
liuxiaocs7 commented on code in PR #7159:
URL: https://github.com/apache/iceberg/pull/7159#discussion_r1143314803
##########
mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandlerNoScan.java:
##########
@@ -272,13 +269,12 @@ public void testCreateDropTableNonDefaultCatalog() throws TException, Interrupte
shell.executeStatement("DROP TABLE default.customers");
// Check if the table was really dropped even from the Catalog
- AssertHelpers.assertThrows(
- "should throw exception",
- NoSuchTableException.class,
- "Table does not exist",
- () -> {
- testTables.loadTable(identifier);
- });
+ Assertions.assertThatThrownBy(
+ () -> {
+ testTables.loadTable(identifier);
+ })
+ .isInstanceOf(NoSuchTableException.class)
+ .hasMessageStartingWith("Table does not exist");
Review Comment:
There are two cases here:
```shell
Expecting actual:
"Table does not exist: default.customers"
```
```shell
Expecting actual:
"Table does not exist at location: /tmp/junit7233921062738785052/default/customers"
```
so i use `hasMessageStartingWith` here.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] liuxiaocs7 commented on a diff in pull request #7159: MR: Remove deprecate AssertHelpers
Posted by "liuxiaocs7 (via GitHub)" <gi...@apache.org>.
liuxiaocs7 commented on code in PR #7159:
URL: https://github.com/apache/iceberg/pull/7159#discussion_r1143323310
##########
mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandlerNoScan.java:
##########
@@ -210,28 +210,25 @@ public void testCreateDropTable() throws TException, IOException, InterruptedExc
shell.executeStatement("DROP TABLE customers");
// Check if the table was really dropped even from the Catalog
- AssertHelpers.assertThrows(
- "should throw exception",
- NoSuchTableException.class,
- "Table does not exist",
- () -> {
- testTables.loadTable(identifier);
- });
+ Assertions.assertThatThrownBy(
+ () -> {
Review Comment:
Done!
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] jackye1995 commented on pull request #7159: MR: Remove deprecate AssertHelpers
Posted by "jackye1995 (via GitHub)" <gi...@apache.org>.
jackye1995 commented on PR #7159:
URL: https://github.com/apache/iceberg/pull/7159#issuecomment-1478186041
Thanks for the work @liuxiaocs7 and thanks for the review @nastra !
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] liuxiaocs7 commented on a diff in pull request #7159: MR: Remove deprecate AssertHelpers
Posted by "liuxiaocs7 (via GitHub)" <gi...@apache.org>.
liuxiaocs7 commented on code in PR #7159:
URL: https://github.com/apache/iceberg/pull/7159#discussion_r1143314803
##########
mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandlerNoScan.java:
##########
@@ -272,13 +269,12 @@ public void testCreateDropTableNonDefaultCatalog() throws TException, Interrupte
shell.executeStatement("DROP TABLE default.customers");
// Check if the table was really dropped even from the Catalog
- AssertHelpers.assertThrows(
- "should throw exception",
- NoSuchTableException.class,
- "Table does not exist",
- () -> {
- testTables.loadTable(identifier);
- });
+ Assertions.assertThatThrownBy(
+ () -> {
+ testTables.loadTable(identifier);
+ })
+ .isInstanceOf(NoSuchTableException.class)
+ .hasMessageStartingWith("Table does not exist");
Review Comment:
There are two cases:
```shell
Expecting actual:
"Table does not exist: default.customers"
```
```shell
Expecting actual:
"Table does not exist at location: /tmp/junit7233921062738785052/default/customers"
```
so i use `hasMessageStartingWith` here.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org