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-29452] Allow unit tests to be executed independently #21289

Conversation

RyanSkraba
Copy link
Contributor

What is the purpose of the change

This is a minor clean up of the unit tests that check the functionality of @RetryOnFailure and @RetryOnException. Currently, executing a single test method will fail; all unit tests in these two classes need to be executed in order for any to pass.

Brief change log

  • The current strategy is to count the number of times a method was called in a static int variable, and checking that every test was run the expected number of times in an @AfterAll verification.
  • The new strategy is to count the number of times any test method was called in a single hashmap (using the @BeforeEach and TestInfo from JUnit5), and verify the counts only if they exist in the @AfterAll
  • Rename NUMBER_OF_RUNS to NUMBER_OF_RETRIES which is a bit clearer.
  • Additional hotfix: I noticed a missing space padding in one of the messages.

Verifying this change

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

One way to check this manually is to run any single test in isolation:

(
  cd flink-test-utils-parent/flink-test-utils-junit/ && 
    mvn test -Dtest=org.apache.flink.testutils.junit.RetryOnExceptionExtensionTest#testSuccessfulTest
)

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 Nov 10, 2022

CI report:

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

@RyanSkraba RyanSkraba force-pushed the rskraba/FLINK-29452-independent-junit-execution branch from dfc9311 to 7833f1e Compare January 19, 2023 17:38
@RyanSkraba
Copy link
Contributor Author

Hello! I've rebased this PR to master. I don't believe there are any remaining requested changes that I haven't addressed (by comment or other).

I will be mostly away from my computer for a couple of weeks, but I'll check in if anything changes!

@RyanSkraba RyanSkraba force-pushed the rskraba/FLINK-29452-independent-junit-execution branch from 7833f1e to 4ec0ed5 Compare March 30, 2023 14:39
@RyanSkraba
Copy link
Contributor Author

OK, change applied, thanks for the review

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 updating the PR. There are still some plain text method names in the test code which we could get rid off.

Additionally, can you reorganize/squash the commits and rebase the branch to prepare the final CI run?

@RyanSkraba RyanSkraba force-pushed the rskraba/FLINK-29452-independent-junit-execution branch from e982964 to f980618 Compare August 24, 2023 13:07
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 👍 Thanks for addressing my comments. Please squash the commits into reasonable chunks to prepare the PR merge.

[hotfix] Add missing space padding for test retry message

[FLINK-29452][test] Refactor for helper methods from review

Apply suggestions from code review

Co-authored-by: Matthias Pohl <matthias.pohl@aiven.io>

Change private HashMap<> to interface
@RyanSkraba RyanSkraba force-pushed the rskraba/FLINK-29452-independent-junit-execution branch from f980618 to bd29f34 Compare November 6, 2023 16:52
@RyanSkraba
Copy link
Contributor Author

Rebased and squashed!

@XComp XComp merged commit 2378bab into apache:master Nov 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants