Skip to content

Hive: Refactor HiveIcebergStorageHandler tests to use catalogs as parameters#1840

Merged
shardulm94 merged 3 commits into
apache:masterfrom
pvary:parameterized
Dec 5, 2020
Merged

Hive: Refactor HiveIcebergStorageHandler tests to use catalogs as parameters#1840
shardulm94 merged 3 commits into
apache:masterfrom
pvary:parameterized

Conversation

@pvary

@pvary pvary commented Nov 27, 2020

Copy link
Copy Markdown
Contributor

As part of #1757 (Hive test refactor) we first should get rid of the HiveIcebergStorageHandlerCatalog test inheritance, replace it with parametrized tests

@github-actions github-actions Bot added the MR label Nov 27, 2020
@pvary pvary mentioned this pull request Nov 27, 2020
@pvary

pvary commented Nov 30, 2020

Copy link
Copy Markdown
Contributor Author

@marton-bod, @lcspinter: Would you mind reviewing the changes?

Thanks,
Peter

Comment thread mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandler.java Outdated
Comment thread mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandler.java Outdated
Comment thread mr/src/test/java/org/apache/iceberg/mr/hive/TestTables.java
Comment thread mr/src/test/java/org/apache/iceberg/mr/hive/TestTables.java Outdated
Comment thread mr/src/test/java/org/apache/iceberg/mr/hive/TestTables.java
Comment thread mr/src/test/java/org/apache/iceberg/mr/hive/TestTables.java
Comment thread mr/src/test/java/org/apache/iceberg/mr/hive/TestTables.java Outdated

@marton-bod marton-bod left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM now, thanks @pvary!

@pvary

pvary commented Dec 2, 2020

Copy link
Copy Markdown
Contributor Author

@rdblue, @shardulm94: Could you please review?
Thanks,
Peter

@shardulm94 shardulm94 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Thanks @pvary!

@shardulm94 shardulm94 merged commit cdf1c23 into apache:master Dec 5, 2020
@pvary

pvary commented Dec 6, 2020

Copy link
Copy Markdown
Contributor Author

Thanks @marton-bod for the review and @shardulm94 for the review and the push!

@pvary pvary deleted the parameterized branch December 6, 2020 13:24
pvary pushed a commit to pvary/iceberg that referenced this pull request Dec 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants