Re: [DISCUSS] Move CalciteAssert and similar "test framework" classes to its own testlib module
As suggested above, I've prepared a PR that moves CalciteAssert, Matchers, and a couple of test schemata to the new :testkit module. calcite-*-tests.jar will no longer be published. PR: https://github.com/apache/calcite/pull/2558 The tests pass, and the change unlocks migration to Gradle 7.2 and Java 17 (the migration is included in PR#2558). I'm going to merge the PR soon. Vladimir
Re: [DISCUSS] Move CalciteAssert and similar "test framework" classes to its own testlib module
Jin>Some of my users write tests based on functionalities provided from Calcite Jin>test framework Frankly speaking, it was somewhat surprising for me, and it does bring extra demand for having a proper calcite-test-framework. The naming is hard, however, I would suggest artifactId=calcite-test-framework. Any thoughts? It turns out Flink depends on Calcite's test code like FlinkSqlParserImplTest extends calcite.SqlParserTest: https://issues.apache.org/jira/browse/CALCITE-2905?focusedCommentId=17002741=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17002741 For instance, a year or so ago I refactored SqlTestFactory (for enabling SqlAdvisor tests) in https://github.com/apache/calcite/commit/96b28f7ba11de22a68d984de6b6b88311cc2c57e , and I was not expecting that someone else might use that test code Vladimir
Re: [DISCUSS] Move CalciteAssert and similar "test framework" classes to its own testlib module
Julian>All of those changes were done by Vladimir. Not one of these changes Julian>adds a single feature, or fixes a single bug, in "Calcite the Julian>product", the library we deliver to our end-users. Julian, I see you are frustrated that one of your use-cases is broken. It is really sad it goes like that, and, please understand that it was not planned. It looks like to be the only defect so far which is remarkably good. It is fine to ignore all my emails (except this one, see below). I'm really surprised to hear "Not one of these changes adds a single feature or fixes a single bug". It is not Gradle's/JUnit's/Vladimir's fault that you do not care to read/ask/investigate regarding features added and bugs fixed. I see the following options to proceed: A) You do your due diligence (e.g. read JIRA comments, read mails, ask somebody, etc, etc), recognize there are both features added and bugs fixed, and you apologise B) You apologise, and stop using "Not one of these changes adds or fixes"-like justifications C) You prove that the changes indeed "neither add nor fix things". Note: "Not one of these changes adds or fixes" are your words, and it is you who should justify them. Please let me know your vision on the way to proceed here. I assume we do not count trivial changes like fixing typos here. Let us consider just the major stuff like "loading code to IDE", "configuring IDE", "creating new files/classes", "build times", "CI output", "code style feedback", "integrated testing between Calcite and Avatica", "release steps", "reproducible builds", "licensing", and so on. For instance: "Maven-based build was not working after import to IDEA", and "Gradle-based build is working right after the import". "Maven-based build did not configure import order" while the current one configures import order automatically. "Maven-based build did not configure copyright headers" while the new one does configure copyright headers, so newly created files have the proper headers. ^^ the above suggests "C" is not viable. Julian>After 3 or 4 days, I am still unable to run the Julian>test and have Intellij show the difference in a pop-up window You have asked for help exactly 0 times. Please remember that dev@ list is exactly for sharing knowledge among the developers. Julian>I ran into a problem running SqlPrettyWriterTest a few Julian>days ago[1] that did not exist a few months ago SqlPrettyWriterTest and DiffRepository were created by yourself. You are in a much much better position to understand the logic behind SqlPrettyWriterTest and DiffRepository than anybody else here. Julian>After 3 or 4 days, I am still unable to run the Julian>test and have Intellij show the difference in a pop-up window. Do you mean you've spent 3-4 days trying to figure out how SqlPrettyWriterTest and DiffRepository work? Does that suggest it is worth simplifying that code? AFAIK, there's a PR#1671 with the fix. Does it work for you? Julian> I can into another, and another It has no weight as it is not justified. Please let us know if you happen to run into a real issue. Vladimir
Re: [DISCUSS] Move CalciteAssert and similar "test framework" classes to its own testlib module
Hi, Vladimir
> I suggest we do not publish testlib artifacts to Nexus (who needs
> Calcite-related assertions and things like that?)
+1 to have the module be published.
Some of my users write tests based on functionalities provided from Calcite
test framework.
Best,
Jin
Michael Mior 于2019年12月18日周三 上午7:47写道：
> Sounds good to me. Although I'd still like to see the module be
> published. I'm currently using it my notebooks project
> (https://github.com/michaelmior/calcite-notebooks) because some of the
> test code removes boilerplate when writing examples.
> --
> Michael Mior
> mm...@apache.org
>
> Le mar. 17 déc. 2019 à 15:49, Vladimir Sitnikov
> a écrit :
> >
> > Hi,
> >
> > Currently
> > org.apache.calcite.test.CalciteAssert, org.apache.calcite.util.TestUtil,
> > and probably other classes are hard to find.
> > I suggest we create a testlib module (or something like that), that would
> > contain "test framework" code.
> >
> > That would make test framework easier to discover (one could open the
> full
> > set of packages in the testlib),
> > and it would make "autocomplete" better (especially for adapters).
> >
> > I suggest we do not publish testlib artifacts to Nexus (who needs
> > Calcite-related assertions and things like that?)
> >
> > Currently testImplementation(project(":core", "testClasses")) is used in
> > lots of places, and it causes "core"
> > test clases appear in autocomplete menu for adapters.
> > For example, typing "new Sql..." in GeodeAssertions suggests
> > SqlAdvisorJdbcTest which is valid, yet it is weird.
> >
> > What do you think?
> >
> > Just in case you did not know: file movement in Git keeps history (even
> in
> > case there are slight modifications).
> > However, the key idea here is to keep files in their current packages,
> but
> > move them into teslib module.
> >
> > Vladimir
>
Re: [DISCUSS] Move CalciteAssert and similar "test framework" classes to its own testlib module
Sounds good to me. Although I'd still like to see the module be
published. I'm currently using it my notebooks project
(https://github.com/michaelmior/calcite-notebooks) because some of the
test code removes boilerplate when writing examples.
--
Michael Mior
mm...@apache.org
Le mar. 17 déc. 2019 à 15:49, Vladimir Sitnikov
a écrit :
>
> Hi,
>
> Currently
> org.apache.calcite.test.CalciteAssert, org.apache.calcite.util.TestUtil,
> and probably other classes are hard to find.
> I suggest we create a testlib module (or something like that), that would
> contain "test framework" code.
>
> That would make test framework easier to discover (one could open the full
> set of packages in the testlib),
> and it would make "autocomplete" better (especially for adapters).
>
> I suggest we do not publish testlib artifacts to Nexus (who needs
> Calcite-related assertions and things like that?)
>
> Currently testImplementation(project(":core", "testClasses")) is used in
> lots of places, and it causes "core"
> test clases appear in autocomplete menu for adapters.
> For example, typing "new Sql..." in GeodeAssertions suggests
> SqlAdvisorJdbcTest which is valid, yet it is weird.
>
> What do you think?
>
> Just in case you did not know: file movement in Git keeps history (even in
> case there are slight modifications).
> However, the key idea here is to keep files in their current packages, but
> move them into teslib module.
>
> Vladimir
Re: [DISCUSS] Move CalciteAssert and similar "test framework" classes to its own testlib module
It probably makes sense.
But I am exhausted, utterly exhausted, with the "moving stuff around"
that Vladimir has done over the last few months.
For example. I ran into a problem running SqlPrettyWriterTest a few
days ago[1] that did not exist a few months ago (before the
gradle/junit upgrade). After I solved that problem, I can into
another, and another. After 3 or 4 days, I am still unable to run the
test and have Intellij show the difference in a pop-up window.
What caused these problems? Maybe the transition from maven to gradle.
Maybe I didn't adequately clean out generated files in my sandox when
I transitioned my Intellij project to gradle. Maybe I should be using
gradle-based tests in Intellij rather than junit-based tests. Maybe
the transition from junit4 to junit5. Who knows?
All of those changes were done by Vladimir. Not one of these changes
adds a single feature, or fixes a single bug, in "Calcite the
product", the library we deliver to our end-users. All of them are
supposedly to make developers' lives easier, but I have lost a week.
Julian
[1] https://issues.apache.org/jira/browse/CALCITE-3595
On Tue, Dec 17, 2019 at 12:49 PM Vladimir Sitnikov
wrote:
>
> Hi,
>
> Currently
> org.apache.calcite.test.CalciteAssert, org.apache.calcite.util.TestUtil,
> and probably other classes are hard to find.
> I suggest we create a testlib module (or something like that), that would
> contain "test framework" code.
>
> That would make test framework easier to discover (one could open the full
> set of packages in the testlib),
> and it would make "autocomplete" better (especially for adapters).
>
> I suggest we do not publish testlib artifacts to Nexus (who needs
> Calcite-related assertions and things like that?)
>
> Currently testImplementation(project(":core", "testClasses")) is used in
> lots of places, and it causes "core"
> test clases appear in autocomplete menu for adapters.
> For example, typing "new Sql..." in GeodeAssertions suggests
> SqlAdvisorJdbcTest which is valid, yet it is weird.
>
> What do you think?
>
> Just in case you did not know: file movement in Git keeps history (even in
> case there are slight modifications).
> However, the key idea here is to keep files in their current packages, but
> move them into teslib module.
>
> Vladimir
