Skip to content

[HUDI-994] Split TestHBaseIndex to unit tests #1818

Merged
vinothchandar merged 2 commits intoapache:masterfrom
xushiyan:functional-tests-for-clients
Jul 14, 2020
Merged

[HUDI-994] Split TestHBaseIndex to unit tests #1818
vinothchandar merged 2 commits intoapache:masterfrom
xushiyan:functional-tests-for-clients

Conversation

@xushiyan
Copy link
Member

@xushiyan xushiyan commented Jul 11, 2020

  • Refactor and improve TestHBaseIndex for performance
  • Move HBaseIndex unit tests to different test classes

Committer checklist

  • Has a corresponding JIRA in PR title & commit

  • Commit message is descriptive of the change

  • CI is green

  • Necessary doc changes done or have another open PR

  • For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.

Copy link
Member Author

Choose a reason for hiding this comment

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

moved to a separate unit test class

Copy link
Member Author

Choose a reason for hiding this comment

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

duplicate test case in org.apache.hudi.index.hbase.TestHBaseQPSResourceAllocator

Copy link
Member Author

Choose a reason for hiding this comment

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

moved to a separate unit test class

@xushiyan xushiyan changed the title [HUDI-996] Functional tests for clients [HUDI-996] Move TestHBaseIndex to functional test suite Jul 11, 2020
@xushiyan
Copy link
Member Author

@vinothchandar this is ready for review. Thanks.

@yanghua yanghua self-assigned this Jul 12, 2020
@yanghua yanghua self-requested a review July 12, 2020 13:29
Copy link
Contributor

@yanghua yanghua left a comment

Choose a reason for hiding this comment

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

About the changes LGTM. Some minor suggestions:

  • The title of PR did not align with the Jira issue and can not describe all the changes in this PR;
  • This PR contains refactor about TestHBaseIndex, code cleanup, some optimization(reduce generated number of records), and so on. IMO, we should split these kinds of work into different PRs in the future.

@yanghua yanghua requested a review from vinothchandar July 13, 2020 00:44
Copy link
Member

@vinothchandar vinothchandar left a comment

Choose a reason for hiding this comment

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

Just 1 comment..
+1 to @yanghua 's suggestion. Not sure if splitting into different PRs is warranted but having a more pertinent title helps!

Copy link
Member

Choose a reason for hiding this comment

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

its not very clear what a sub path stands for? rename to make it clearer?

Copy link
Member Author

Choose a reason for hiding this comment

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

ok remove this api.. not used at the moment.

@xushiyan
Copy link
Member Author

@yanghua @vinothchandar Thanks for the review. I can split the changes into 2 PRs

  1. (this one) Improve TestHBaseIndex (refactoring, move test cases to unit tests)
  2. (create a new one) add functional test suite to hudi-client and tag TestHBaseIndex as "functional"

going to work on this in next few hours...

@xushiyan xushiyan force-pushed the functional-tests-for-clients branch from 6ff2cc1 to 8ab22ab Compare July 13, 2020 08:23
@xushiyan
Copy link
Member Author

xushiyan commented Jul 13, 2020

@yanghua @vinothchandar Thanks for the review. I can split the changes into 2 PRs

  1. (this one) Improve TestHBaseIndex (refactoring, move test cases to unit tests)
  2. (create a new one) add functional test suite to hudi-client and tag TestHBaseIndex as "functional"

going to work on this in next few hours...

Split done. The 2nd one goes to #1824 , which contains the commit in this PR. Please take another pass. Thanks.

@xushiyan xushiyan changed the title [HUDI-996] Move TestHBaseIndex to functional test suite [HUDI-994] Split TestHBaseIndex to unit tests Jul 13, 2020
- Refactor and improve TestHBaseIndex for performance
- Move HBaseIndex unit tests to different test classes
@xushiyan xushiyan force-pushed the functional-tests-for-clients branch from 8ab22ab to 6352eef Compare July 13, 2020 23:52
@vinothchandar vinothchandar merged commit f5dc8ca into apache:master Jul 14, 2020
@xushiyan xushiyan deleted the functional-tests-for-clients branch August 8, 2020 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants