[FLINK-32850][flink-runtime][JUnit5 Migration] Module: The io package of flink-runtime #23200

Jiabao-Sun · 2023-08-13T08:58:40Z

[FLINK-32850][flink-runtime][JUnit5 Migration] Module: The io package of flink-runtime

What is the purpose of the change

[flink-runtime][JUnit5 Migration] Module: The io package of flink-runtime

Brief change log

[flink-runtime][JUnit5 Migration] Module: The io package of flink-runtime

Verifying this change

This change is already covered by existing tests

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 · 2023-08-13T09:04:02Z

CI report:

84036ae Azure: SUCCESS

Bot commands

The @flinkbot bot supports the following commands:

@flinkbot run azure re-run the last Azure build

1996fanrui

Thanks @Jiabao-Sun for the quick contribution!

Would you mind updating all change about these improvements?

Update the isEqualTo(0) to isZero()
Update the isEqualTo(1) to isOne()
Check whether the fail with catch can be updated to assertThatThrownBy

...k-runtime/src/test/java/org/apache/flink/runtime/io/disk/BatchShuffleReadBufferPoolTest.java

flink-runtime/src/test/java/org/apache/flink/runtime/io/disk/FileChannelManagerImplTest.java

flink-runtime/src/test/java/org/apache/flink/runtime/io/disk/SpillingBufferTest.java

Jiabao-Sun · 2023-08-13T13:45:36Z

Thanks @Jiabao-Sun for the quick contribution!

Would you mind updating all change about these improvements?

Update the isEqualTo(0) to isZero()

Update the isEqualTo(1) to isOne()

Check whether the fail with catch can be updated to assertThatThrownBy

Thanks @1996fanrui for the review.
I'll make changes for that.

flink-runtime/src/test/java/org/apache/flink/runtime/io/disk/FileChannelManagerImplTest.java

.../src/test/java/org/apache/flink/runtime/io/disk/iomanager/AsynchronousFileIOChannelTest.java

flink-runtime/src/test/java/org/apache/flink/runtime/io/disk/iomanager/IOManagerAsyncTest.java

flink-runtime/src/test/java/org/apache/flink/runtime/io/disk/iomanager/IOManagerITCase.java

...e/src/test/java/org/apache/flink/runtime/io/network/api/writer/RecordWriterDelegateTest.java

...k-runtime/src/test/java/org/apache/flink/runtime/io/network/api/writer/RecordWriterTest.java

1996fanrui · 2023-08-16T13:24:54Z

I didn't finish the review due to this PR is totally huge!

Hi @X-czh , would you mind helping review this PR in your free time? thanks~

There are too many test migration PRs are waiting for my review, and I don't have enough time to do all review. It's better for contributors to review each other first. Of course, I will review them as well.

cc @Jiabao-Sun

X-czh · 2023-08-17T02:50:02Z

Hi @Jiabao-Sun, thanks for the contribution! Due to the size of the PR, I expect it to take quite a few time, I'll manage to finish reviewing it by the end of this week.

...k-runtime/src/test/java/org/apache/flink/runtime/io/disk/BatchShuffleReadBufferPoolTest.java

...ime/src/test/java/org/apache/flink/runtime/io/network/buffer/LocalBufferPoolDestroyTest.java

flink-runtime/src/test/java/org/apache/flink/runtime/io/network/buffer/LocalBufferPoolTest.java

...rc/test/java/org/apache/flink/runtime/io/network/partition/SortMergeResultPartitionTest.java

...src/test/java/org/apache/flink/runtime/io/network/partition/ProducerFailedExceptionTest.java

Jiabao-Sun · 2023-08-20T06:31:39Z

Thanks @X-czh for the detailed review.
Most of the comments are resolved.
Please help review it again when you have time.

X-czh · 2023-08-21T09:57:54Z

Thanks @Jiabao-Sun again for your hard work, LGTM now

1996fanrui

Hi @RocMarshal , would you mind helping reveiw this PR in your free time? thanks~

It's not urgent!

snuyanzin

Thanks for the contribution

I compared number of tests in maven build for this module before and after
Before
nonIT tests run: 7397, 73 skipped
IT tests run: 350

After (i rebased locally to the latest master )
nonITtests run: 7432, 73 skipped
IT tests run: 350

If we are talking only about migration then I would expect same number of tests before and after and same test set

...k-runtime/src/test/java/org/apache/flink/runtime/io/disk/BatchShuffleReadBufferPoolTest.java

Jiabao-Sun · 2023-08-28T08:57:41Z

Thanks @snuyanzin for the review.
Could you help review it again when you have time?

Jiabao-Sun · 2023-09-11T11:16:42Z

Test count verified of org.apache.flink.runtime.io package:
master
passed: 1847, ignored: 5, total: 1852
updated:
passed: 1847, ignored: 5, total: 1852

… of flink-runtime

Jiabao-Sun · 2023-10-23T08:31:14Z

Hi @JingGe, could you help review this PR when you have time?
This PR is more difficult to review, and I can also split it into multiple. WDYT?

Jiabao-Sun · 2023-10-25T02:45:05Z

This PR is too huge to review.
I will split it into multiple PRs.

RocMarshal · 2023-10-25T03:29:41Z

This PR is too huge to review. I will split it into multiple PRs.

SGTM +1.
Looking forward to it~

1996fanrui requested changes Aug 13, 2023

View reviewed changes

Jiabao-Sun requested a review from 1996fanrui August 14, 2023 07:02

1996fanrui requested changes Aug 16, 2023

View reviewed changes

Jiabao-Sun force-pushed the FLINK-32850 branch from fb84fea to 93cb8f2 Compare August 18, 2023 17:25

X-czh reviewed Aug 20, 2023

View reviewed changes

Jiabao-Sun force-pushed the FLINK-32850 branch from 93cb8f2 to adc6af1 Compare August 20, 2023 06:01

Jiabao-Sun requested review from X-czh and 1996fanrui August 20, 2023 06:32

1996fanrui reviewed Aug 26, 2023

View reviewed changes

snuyanzin requested changes Aug 27, 2023

View reviewed changes

snuyanzin reviewed Aug 27, 2023

View reviewed changes

...k-runtime/src/test/java/org/apache/flink/runtime/io/disk/BatchShuffleReadBufferPoolTest.java Outdated Show resolved Hide resolved

Jiabao-Sun force-pushed the FLINK-32850 branch from c8a8367 to af05770 Compare August 28, 2023 05:45

Jiabao-Sun requested a review from snuyanzin August 28, 2023 08:56

Jiabao-Sun force-pushed the FLINK-32850 branch from af05770 to 044612d Compare August 30, 2023 10:21

Jiabao-Sun requested a review from 1996fanrui August 30, 2023 10:21

Jiabao-Sun force-pushed the FLINK-32850 branch from 044612d to cedd4e1 Compare September 10, 2023 14:40

Jiabao-Sun force-pushed the FLINK-32850 branch from cedd4e1 to 3f6f9ce Compare September 12, 2023 04:55

Jiabao-Sun added 5 commits September 14, 2023 20:10

[FLINK-32850][flink-runtime][JUnit5 Migration] Module: The io package…

98065aa

… of flink-runtime

Address Rui's comments

477173b

Address Rui's comments

0317dd9

Address Rui's comments

aef534b

Address Zhanghao's comments

78d4a02

Jiabao-Sun added 2 commits September 14, 2023 20:11

Address Sergey's comments

557e0cf

Rebase and optimize

84036ae

Jiabao-Sun force-pushed the FLINK-32850 branch from 3f6f9ce to 84036ae Compare September 14, 2023 12:12

Jiabao-Sun closed this Oct 25, 2023

flinkbot added the component=Tests label Apr 4, 2024

[FLINK-32850][flink-runtime][JUnit5 Migration] Module: The io package of flink-runtime #23200

[FLINK-32850][flink-runtime][JUnit5 Migration] Module: The io package of flink-runtime #23200

Uh oh!

Conversation

Jiabao-Sun commented Aug 13, 2023

What is the purpose of the change

Brief change log

Verifying this change

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

Documentation

Uh oh!

flinkbot commented Aug 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI report:

Uh oh!

1996fanrui left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Jiabao-Sun commented Aug 13, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

1996fanrui commented Aug 16, 2023

Uh oh!

X-czh commented Aug 17, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Jiabao-Sun commented Aug 20, 2023

Uh oh!

X-czh commented Aug 21, 2023

Uh oh!

1996fanrui left a comment

Choose a reason for hiding this comment

Uh oh!

snuyanzin left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Jiabao-Sun commented Aug 28, 2023

Uh oh!

Jiabao-Sun commented Sep 11, 2023

Uh oh!

Jiabao-Sun commented Oct 23, 2023

Uh oh!

Jiabao-Sun commented Oct 25, 2023

Uh oh!

RocMarshal commented Oct 25, 2023

Uh oh!

Uh oh!

flinkbot commented Aug 13, 2023 •

edited

Loading

snuyanzin left a comment •

edited

Loading