[
https://issues.apache.org/jira/browse/CALCITE-5923?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17755314#comment-17755314
]
Runkang He commented on CALCITE-5923:
-------------------------------------
[~julianhyde] Thanks for giving good advice.
For the first question, besides parameterized tests using `@MethodSource`
annotation, there is other place violates the rule, see
[SqlOperatorTest.testArgMin|https://github.com/apache/calcite/blob/51da34bdf91e602f32f965b3490bace026d51ef4/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java#L11912].
I want to fix them in this Jira at the same time.
For the second question, I checked all related test cases using `@MethodSource`
annotation in Calcite, excludes `SqlOperatorTest`, they have two manifestations:
1. the test class is a final class or no subclasses, such as
`JoinPreserveOrderTest`, `RexBuilderTest`.
2. the method used as MethodSource is self-closing, not refering to other
methods of the class, such as `SourceTest.relativePaths`
For above reason, I think these places could not benefit from
Lifecycle.PER_CLASS currently. But it's more safe to add Lifecycle.PER_CLASS to
all these places, to avoid this issue when modifying or adding new test
subclasses or methods in the future.
> Some test cases in `SqlOperatorTest` violates the test fixture's design
> principle
> ---------------------------------------------------------------------------------
>
> Key: CALCITE-5923
> URL: https://issues.apache.org/jira/browse/CALCITE-5923
> Project: Calcite
> Issue Type: Bug
> Components: core
> Affects Versions: 1.35.0
> Reporter: Runkang He
> Assignee: Runkang He
> Priority: Minor
> Labels: pull-request-available
>
> There are some test cases in `SqlOperatorTest` directly use the
> `SqlOperatorFixtureImpl.DEFAULT` to get the `SqlOperatorFixture`, including
> `SqlOperatorTest.testCast` and many other test cases related with `CAST`
> operator. This causes that the result check is missing when execute
> `CalciteSqlOperatorTest`, which should has result check.
> This violates the design principle introduced by CALCITE-4885, which we
> should alway use `SqlOperatorTest.fixture()` to get the `SqlOperatorFixture`.
> This principle allows us to override`fixture()` method in subclasses to run
> tests in a different environment.
> So I think we should fix these related test cases to keep consistent with the
> principle.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
