wx930910 opened a new issue #11560: URL: https://github.com/apache/druid/issues/11560
### Description I noticed that a test subclass [WrappedExprMacro](https://github.com/apache/druid/blob/8296123d895db7d06bc4517db5e767afb7862b83/processing/src/test/java/org/apache/druid/query/expression/MacroTestBase.java#L72) implements production interface [ExprMacro](https://github.com/apache/druid/blob/8296123d895db7d06bc4517db5e767afb7862b83/core/src/main/java/org/apache/druid/math/expr/ExprMacroTable.java#L89) to assist testing and make sure the `apply(List)` method is invoked. This might not be the best priactice in unit testing and can be improved by leveraging mocking frameworks. ### Current Implementation - `WrappedExprMacro` implements `ExprMacro` and overrides two methods. Create a AtomicLong attribute to keep tracking of the invocation status of `apply(List)`. - Assert the value of the AtomicLong attribute to make sure method `apply(List)` is invoked. ### Purposed Implementation - Replace `WrappedExprMacro` with a mocking object created by Mockito. - Use method stub to control the behavior of the mocking object. - Extract the AtomicLong attribute and use the extracted attribute to check method invocation status. ### Motivation - Decouple test class `WrappedExprMacro` from production interface `ExprMacro`. - Make test logic more clear by using method stub instead of method overriding. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
