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