Skip to content

[HUDI-995] Use Transformations, Assertions and SchemaTestUtil#1884

Merged
yanghua merged 1 commit intoapache:masterfrom
xushiyan:organize-testutils-classes
Aug 1, 2020
Merged

[HUDI-995] Use Transformations, Assertions and SchemaTestUtil#1884
yanghua merged 1 commit intoapache:masterfrom
xushiyan:organize-testutils-classes

Conversation

@xushiyan
Copy link
Member

@xushiyan xushiyan commented Jul 28, 2020

There are testutils functions scattered around in different modules (client, spark, common) for common transformations like HoodieRecords to HoodieKeys. This is to organize them in Transformations.java for ease of discovery and use.

Similar purpose for adding Assertions.java.

Also make use of SchemaTestUtil to load test schema files from resource wherever applicable.

Changes

  • Consolidate transform functions for tests in Transformations.java
  • Consolidate assertion functions for tests in Assertions.java
  • Make use of SchemaTestUtil for loading schema from resource

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.

@xushiyan
Copy link
Member Author

@yanghua @vinothchandar This is another set of incremental changes for testutils re-organization.

@yanghua yanghua self-assigned this Jul 29, 2020
@yanghua yanghua requested review from vinothchandar and yanghua July 29, 2020 00:30
@xushiyan xushiyan force-pushed the organize-testutils-classes branch from ed48c2d to a883343 Compare July 29, 2020 03:36
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.

Hi @xushiyan thanks for your contribution. I left some comments you can consider.

@xushiyan
Copy link
Member Author

@yanghua address the comments in the last 2 commits. thanks.

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.

@xushiyan Thanks for addressing my comments. Left one minor comment and one question.

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.

Thanks @xushiyan LGTM now. Will merge it after CI pass.

@xushiyan xushiyan force-pushed the organize-testutils-classes branch 2 times, most recently from 783a3a1 to 523df11 Compare August 1, 2020 01:55
- Consolidate transform functions for tests in Transformations.java
- Consolidate assertion functions for tests in Assertions.java
- Make use of SchemaTestUtil for loading schema from resource
@xushiyan xushiyan force-pushed the organize-testutils-classes branch from 523df11 to 3d6e60d Compare August 1, 2020 02:11
@xushiyan
Copy link
Member Author

xushiyan commented Aug 1, 2020

@yanghua rebased and resolved conflicts. thanks

@yanghua yanghua merged commit 10e4268 into apache:master Aug 1, 2020
@xushiyan xushiyan deleted the organize-testutils-classes branch August 1, 2020 18:04
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.

2 participants