[ 
https://issues.apache.org/jira/browse/CALCITE-5923?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17759140#comment-17759140
 ] 

Julian Hyde commented on CALCITE-5923:
--------------------------------------

Good point, [~zabetak]. SqlOperatorTest makes it possible to define a lot of 
tests and run them (at least) twice: once for parsing and validation, and once 
by generating code in Enumerable convention and executing the functions to 
check the results.

SqlOperatorTest uses the fixture, and other tests may use that same fixture, 
but only if you add tests to SqlOperatorTest do you get that effect of running 
the same test in two different environments.

Maybe that 'running the test twice' effect could be achieved, in modern junit, 
using parameterized tests, and maybe we should consider that. But the discusion 
should recognize the immense effort that has gone into the suite already, and 
the value it provides, and the difficulty of refactoring it if the only benefit 
is that we are 'modern'.

> 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)

Reply via email to