I haven’t heard from anyone. I plan to merge https://issues.apache.org/jira/browse/CALCITE-4986 <https://issues.apache.org/jira/browse/CALCITE-4986> "Make HepProgram thread-safe” in the next hour or two, then I will squash/rebase/merge https://issues.apache.org/jira/browse/CALCITE-4885 <https://issues.apache.org/jira/browse/CALCITE-4885> "Fluent test fixtures” tomorrow.
They’re both significant changes, review welcome. Julian > On Jan 12, 2022, at 10:11 AM, Julian Hyde <[email protected]> wrote: > > After a month or so of work I have a PR for review. See > https://github.com/apache/calcite/pull/2685/ > <https://github.com/apache/calcite/pull/2685/>. > > The PR is huge (80 commits, 127 files changed, 20k lines added/removed) so > may be hard to digest. I recommend that you read the commit log to understand > the various refactorings. Note that SqlOperatorBaseTest both moved (from main > to testkit) and changed name (to SqlOperatorTest) so diffing it might require > some ingenuity. > > At this point I am especially interested in high-level comments (Does the > test/fixture/tester/factory split work? Are there any other candidates for > this refactoring?). If you have a lot of low-level comments (e.g. spelling > mistakes) just send me a PR. > > It’s probably several days before I will squash, rebase and consider merging > to main. > > Julian > > >> On Nov 22, 2021, at 4:12 AM, Stamatis Zampetakis <[email protected] >> <mailto:[email protected]>> wrote: >> >> Hello, >> >> I generally agree with the problems and goals set by CALCITE-4885 so >> definitely worth pushing this forward. >> >> I also like the ideas for being compatible with JUnit5 extensions etc., but >> I guess it could be something to address later one and does not need to be >> part of CALCITE-4885. >> If and when somebody comes with a concrete proposal we can evaluate this. >> >> Same reasoning goes for AssertJ; it may be beneficial for the project but >> let's evaluate it separately. >> >> If I understand well, both AssertJ and extensions via JUnit5 may come in >> conflict with the new APIs exposed by CALCITE-4885 so it would be nice if >> we can advance all these in the same release. >> >> Last I wanted to mention that although people consuming Calcite releases >> may not care much about big changes in the testing code, those who maintain >> forks of the main repo will have a few more challenges to address when they >> cherry-pick changes. >> This is not an argument to push back the feature but just an additional >> element for completing the picture. >> >> Best, >> Stamatis >> >> On Wed, Nov 17, 2021 at 7:15 AM Vladimir Sitnikov < >> [email protected] <mailto:[email protected]>> wrote: >> >>> Jacques>This sounds like it will mean we will need to make calcite-core >>> test artifacts available >>> >>> Test artifacts publication is yet another anti-pattern just like "base test >>> class". >>> This change has been discussed: >>> https://lists.apache.org/thread/fz96p94h016p11g777otqntjxg2oxgh1 >>> <https://lists.apache.org/thread/fz96p94h016p11g777otqntjxg2oxgh1> >>> >>> If you want to depend on a class from tests, consider moving it to /testkit >>> module: >>> >>> https://github.com/apache/calcite/tree/0899e6c157632ba1c5369a942cfe2be15fb4ed9f/testkit >>> >>> Jacques>We should think about the rules around Kotlin >>> >>> What happens in calcite-core/tests stays in calcite-core/tests :) >>> >>> It is reasonable to assume that testkit module would have dependencies, >>> and testkit would provide API that is usable from Java and other JVM >>> languages. >>> >>> In that regard, Kotlin dependency in testkit is not much different from >>> Quidem or commons-lang3. >>> Consumers might use Quidem if it fits just like they could use Kotlin if it >>> fits. >>> >>> Vladimir >>> >
