Refactoring org.apache.druid.segment.loading

[gzhao9]

Description

This PR addresses Issue #15164 of the DataSegment dependency being repeatedly mocked across three different test suites. It introduces a CreateMockDataSegment class to eliminate this redundancy, enhancing the maintainability and extensibility of the code.

And also Implemented a similar refactoring approach as described in issue #15164. Introduced a helper method CreateMockTaskActionToolbox to reuse the creation of TaskActionToolbox mocks. This reduces code redundancy and improves test readability.

Key Changes

Introduced the CreateMockDataSegment Class

A new class named CreateMockDataSegment has been created to centralize the mocking of DataSegment, reducing code duplication and improving maintainability.

Optimized Test Suite Code

This refactoring approach has been applied to three test suites: OmniDataSegmentMoverTest.java, OmniDataSegmentArchiverTest.java, and OmniDataSegmentKillerTest.java.

---

Key changed/added classes in this PR

• CreateMockTaskActionToolbox method in UpdateStatusActionTest.java

---

This PR has:

• [x] been self-reviewed.
• [x] added documentation for new or modified features or behaviors.
• [x] included a release note in the PR description.
• [x] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
• [] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
• [] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
• [] been tested in a test Druid cluster.

Please note, the above is a sample filled out using the information from your provided issue. You may need to adjust it according to the actual content of your PR.

[gzhao9]

@cryptoe Thank you very much for your reply in Issue. I have merged both PRs, #15187 and #15217 into this current PR.

[cryptoe]

@gzhao9 Please take a look into the conflicts.

[gzhao9]

@gzhao9 Please take a look into the conflicts.

Thanks for the reminder, I've resolved the conflict.

[gzhao9]

Thanks for trying to improve the tests, @gzhao9 !

DataSegment is a simple POJO Druid class which should not be mocked. Any test doing this should instead be updated to use concrete DataSegment objects. You can either use a DataSegment.Builder or the CreateDataSegment utility to easily create DataSegment objects in test.

Also, this is a change that doesn't affect Druid users in any way, so it doesn't need a release note.

Hi, sorry for the delayed response. I have removed the changes regarding the DataSegment section. Because this PR is mainly trying to reduce mock cloning in tests.

In the meantime, I submitted another PR that also tries to reduce mock cloning in tests. See the detail in #15980 If you are interested please take a look as well. @kfaraz @cryptoe

[kfaraz]

My feedback on this PR is the same as https://github.com/apache/druid/pull/15980#pullrequestreview-1924038812. I don't feel that these changes are necessary as they don't add any value except reducing a couple of lines and compromise the readability of the tests.

[github-actions[bot]]

This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the [email protected] list. Thank you for your contributions.

[github-actions[bot]]

This pull request/issue has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.