Refactor Test Code to Reduce Mock Duplication For Issue #15979

[gzhao9]

Description

Fixes #15979

This PR addresses the issue #15979 of duplicated mock creation code across multiple test classes within the Apache Druid project. By introducing the CreateMockColumnValueSelector.java utility class, I aim to centralize the creation of mocked ColumnValueSelector objects, thereby enhancing code reusability and simplifying maintenance.

In the course of testing, particularly within the ArrayDoubleGroupByColumnSelectorStrategyTest, ArrayLongGroupByColumnSelectorStrategyTest, and ArrayStringGroupByColumnSelectorStrategyTest classes, I identified a pattern of duplicate code concerning mock object creation. This redundancy not only bloats the test code but also introduces unnecessary complexity.

Solution

The newly introduced CreateMockColumnValueSelector class provides a streamlined approach to generating mocked ColumnValueSelector instances, equipped with predefined behaviors. This approach significantly reduces the boilerplate code in our test suite and improves the readability and maintainability of our tests.

Key changed/added classes in this PR

• CreateMockColumnValueSelector
• Modified:
  • ArrayDoubleGroupByColumnSelectorStrategyTest
  • ArrayLongGroupByColumnSelectorStrategyTest
  • ArrayStringGroupByColumnSelectorStrategyTest

By doing this, code repetition can be reduced. Make the test code clearer I'd love to hear your thoughts on this proposal! Do you agree with any of these changes, or do you have other insights or preferences?

[gzhao9]

Thanks for taking out the time. I donot see value in this refactor. This adds another layer which is used in a single testClass.

Thank you for your feedback. I've removed CreateMockColumnValueSelector and added its creat mock function directly into the test classes as suggested. Could you please share your preferences for preparing mock object data in tests? Or any suggestions you wish to change?

[gzhao9]

Thanks for taking out the time. I donot see value in this refactor. This adds another layer which is used in a single testClass.

Thank you for your feedback. I've removed CreateMockColumnValueSelector and added its creat mock function directly into the test classes as suggested. Could you please share your preferences for preparing mock object data in tests? Or any suggestions you wish to change?

I'd like to clarify. When I first submitted my PR, the class was used by 3 test classes, not a single test class. So I'm a little confused about This adds another layer which is used in a single testClass..

If you mean to avoid creating a new class. my modification is to create a method in each test class. In the test case, the method is called to minimize the duplicate creation of mock objects.

[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.