Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FLINK-28449][tests][JUnit5 migration] flink-parquet #20230

Merged
merged 2 commits into from
Nov 4, 2022

Conversation

RyanSkraba
Copy link
Contributor

What is the purpose of the change

Update the flink-formats/flink-parquet module to AssertJ and JUnit 5 following the JUnit 5 Migration Guide

Brief change log

  • Removed dependences on JUnit 4, JUnit 5 Assertions and Hamcrest where possible.

Verifying this change

This change is a trivial rework / code cleanup without any test coverage.

I've verified that there are 94 tests run in this module before and after the refactoring.

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): no
  • The public API, i.e., is any changed class annotated with @Public(Evolving): no
  • The serializers: no
  • The runtime per-record code paths (performance sensitive):no
  • Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: no
  • The S3 file system connector: no

Documentation

  • Does this pull request introduce a new feature?no
  • If yes, how is the feature documented? not applicable

@flinkbot
Copy link
Collaborator

flinkbot commented Jul 8, 2022

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

@RyanSkraba
Copy link
Contributor Author

@flinkbot run azure

Copy link
Contributor

@XComp XComp left a comment

Choose a reason for hiding this comment

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

LGTM 👍 I added a few minor cosmetic comments. Please also rebase the branch to make it ready to be merged.

@RyanSkraba RyanSkraba force-pushed the rskraba/FLINK-28449-flink-parquet branch from baaa6af to 2022025 Compare October 25, 2022 13:27
@RyanSkraba RyanSkraba requested a review from XComp October 25, 2022 13:45
Copy link
Contributor

@XComp XComp left a comment

Choose a reason for hiding this comment

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

Thanks for addressing/answering my questions, @RyanSkraba . Two minor things are still required to change.

@XComp
Copy link
Contributor

XComp commented Nov 4, 2022

I created FLINK-29893 to cover the build failure.

@XComp
Copy link
Contributor

XComp commented Nov 4, 2022

I'm gonna merge this PR without rerunning the e2e tests because the change suggested by this PR is independent of the e2e tests.

@XComp XComp merged commit db3244a into apache:master Nov 4, 2022
@RyanSkraba RyanSkraba deleted the rskraba/FLINK-28449-flink-parquet branch November 4, 2022 17:58
dchristle pushed a commit to dchristle/flink that referenced this pull request Nov 18, 2022
akkinenivijay pushed a commit to krisnaru/flink that referenced this pull request Feb 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants