[FLINK-27146] [Filesystem] Migrate to Junit5

[kottmann]

What is the purpose of the change

This migrates the Filesystem test code to use Junit5 and Assertj.

Brief change log

• Migrated the test code to use Junit5 according to the migration documentation
• Also migrated to Assertj (this is in a separate commit and could be dropped or resubmitted in a separate PR)

Verifying this change

This change will not have any impact on non-test code since only test code was changed. The change can potentially break tests, but the majority of changes are simple and don't touch the logic.

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]

CI report:

• 276f51f0da3c96f8abc56fc386c55430044b02b2 Azure: SUCCESS

Bot commands

The @flinkbot bot supports the following commands:

• @flinkbot run azure re-run the last Azure build

[kottmann]

@ferenc-csaky Thank you for your insightful feedback and for directing me towards the assertj decision. I updated my branch by rebasing it onto master and incorporating the changes related to assertj. Additionally, I will make another commit to address the remaining feedback that isn't connected to assertj.

[kottmann]

Sorry for the delay on my end, I added a new commit to address all your comments above.

[kottmann]

This is fixed now as well, see new commit.

[kottmann]

@ferenc-csaky Thank you for reviewing my PR and for your helpful feedback! I really appreciate your support in improving it.

[ferenc-csaky]

@kottmann I was not a committer back then, but if you will have some time and desire to resolve the current conflicts, I still believe this would be a nice improvement and will add it afterwards.

[Jiabao-Sun]

Thanks @kottmann for this contribution and @ferenc-csaky for the review, Sorry for missing the message earlier. Could you rebase master and resolve the conflicts?

[kottmann]

Thanks, I rebased it onto master and it should be possible to merge now. Some classes received a bit of JUnit 5 migration from another commit before, and I resolved those conflicts or removed my changes.

[kottmann]

@Jiabao-Sun Thanks for your review, I addressed your comments in the last commit, for the more general things you mentioned I searched in flink-filesystem for other occurrence of the same pattern and changed it too.

I am not sure about the number of tests running, it appears that the number of tests in CI for my branch is lower than for master, and also the test duration is shorter, so it would be good to understand whats going on before merging this.

[kottmann]

I tracked a missing test case down to HadoopS3FileSystemBehaviorITCase this test doesn't run on my branch, but does run on the master branch. But this wasn't modified in my PR, do you know why this doesn't run? Are they disabled by default in CI for PRs?

[kottmann]

Thanks for the feedback!

@Jiabao-Sun Not sure why the CI failed, this seems to be about timeouts in tests, the changes in the last commit where mostly about reducing method visibility which seems to be less likely be linked. Since I got more feedback, I added more commits and CI will run again and maybe the timeouts don't occur again. Let's see, otherwise I will dig into it.

@ferenc-csaky I reverted the assumeFalse changes back to assumeThat.

[kottmann]

@ferenc-csaky Shall I squash the commits, or will you do that when it gets merged?

[ferenc-csaky]

@ferenc-csaky Shall I squash the commits, or will you do that when it gets merged?

Squash and merge takes care of that, so it is not necessary I would say.