[
https://issues.apache.org/jira/browse/CALCITE-2462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16582218#comment-16582218
]
Vladimir Sitnikov edited comment on CALCITE-2462 at 8/16/18 9:06 AM:
---------------------------------------------------------------------
{quote}RexImplicationCheckerTest{quote}
I have not.
Extracting base class enables to keep the current source of {{RexProgramTest}}
intact.
{quote} necessary methods and fields in a separate class from the test{quote}
Any problem with that yet?
Do you suggest to replace all usages of {{and(or(..))}} in existing
{{RexProgramTest}} with {{f.add(f.or(...))}}?
As you see, new tests are added to RexProgramTest, so it makes sense to keep it
readable.
{quote}better than @Before, in my opinion{quote}
This is to be treated by JUnit 5. For instance, it can initialize test fields
just once, not once per test method.
{quote}Also, tests can sub-class Fixture to add more things that they
need.{quote}
Both existing "Fixture2 extends Fixture" could be merged right into original
Fixture class ==> it does not sound like a strong reason to have "builder
class" isolated.
On the other hand, `isNull(not(vBoolNotNull()))` is easier to read than
`f.isNull(f.not(f.vBoolNotNull()))`
was (Author: vladimirsitnikov):
{quote}RexImplicationCheckerTest{quote}
I have not.
Extracting base class enables to keep the current source of {{RexProgramTest}}
intact.
{quote} necessary methods and fields in a separate class from the test{quote}
Any problem with that yet?
Do you suggest to replace all usages of {{and(or(..))}} in existing
{{RexProgramTest}} with {{f.add(f.or(...))}}?
As you see, new tests are added to RexProgramTest, so it makes sense to keep it
readable.
{quote}better than @Before, in my opinion{quote}
This is to be treated by JUnit 5. For instance, it can initialize test fields
just once, not once per test method.
{quote}Also, tests can sub-class Fixture to add more things that they
need.{quote}
Both existing "Fixture2 extends Fixture" could be merged right into original
Fixture class.
> RexProgramTest: move "rex building" methods to base class
> ---------------------------------------------------------
>
> Key: CALCITE-2462
> URL: https://issues.apache.org/jira/browse/CALCITE-2462
> Project: Calcite
> Issue Type: Sub-task
> Components: core
> Affects Versions: 1.17.0
> Reporter: Vladimir Sitnikov
> Assignee: Julian Hyde
> Priority: Major
> Fix For: 1.18.0
>
>
> RexProgramTest is quite big (2000 lines now), and "easy to use" rex building
> is useful for many tests.
> So I suggest to move methods like {{gt}}, {{lt}}, etc to the base class, so
> multiple other tests can reuse it.
> Alternative option would be to use Kotlin for builders, yet I think Java
> tests will be there for quite a while, so it would be nice to simplify them
> as well.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
