Thank you for doing this, after reading the overview these changes seem like they will make a number of things easier related to testing. Super useful too when you're trying to start building something with Calcite but don't know it well enough to figure out how to put all the pieces together yourself.
On Fri, Jan 21, 2022 at 3:57 PM Julian Hyde <[email protected]> wrote: > If it helps you review, I have created a ’summary’ document with a > description of the changes. It will become the commit message when I > squash, rebase, and merge to master. > > > https://github.com/julianhyde/calcite/blob/4885-test-fixtures/summary-of-calcite-4885.md > < > https://github.com/julianhyde/calcite/blob/4885-test-fixtures/summary-of-calcite-4885.md> > > > Julian > > > > On Jan 19, 2022, at 9:08 PM, Jacques Nadeau <[email protected]> wrote: > > > > Unfortunately, the last minute attendance of the meetup today threw my > > schedule off and I won't be able to look at this for at least a few days. > > Don't feel obligated to hold up for me. > > > > On Wed, Jan 19, 2022, 9:04 AM Jacques Nadeau <[email protected]> wrote: > > > >> FYI, I'm trying to do a thorough review today (as much as possible with > >> patch this size). > >> > >> On Wed, Jan 19, 2022, 4:36 AM Michael Mior <[email protected]> wrote: > >> > >>> Thanks for doing this Julian! I haven't been able to do a detailed > review, > >>> but from my skim of the PR, this looks like a nice improvement. I think > >>> it's always been a bit difficult for new Calcite developers to write > good > >>> tests, especially for new modules. So anything that helps that > experience > >>> is very welcome. > >>> > >>> -- > >>> Michael Mior > >>> [email protected] > >>> > >>> > >>> Le mer. 17 nov. 2021 à 01:15, Vladimir Sitnikov < > >>> [email protected]> > >>> a écrit : > >>> > >>>> 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 > >>>> > >>>> 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 > >>>> > >>> > >> > >
