> Calcite is an intermediate framework. It is not a database. It is not a > training framework for machine learning. > Thus Calcite should not take much time to perform its duties.
I don't think the runtime of Calcite should necessarily be tied to the runtime of tests. I agree that Calcite should have minimal overhead. I don't think that means that the fully test suite is necessarily fast. If the ability of the test suite to catch important bugs scales with the test runtime, that's a good trade-off. > Would you mind if I add that kind of enabled-by-default test? Probably not, but I refer to my earlier statement that if the marginal ability of that test to catch bugs is significant, then it might be a good decision. Bottom line is that I agree with you that test runtime is important and we should try to minimize it. But the objective obviously isn't to minimize test runtime or we'd delete all the tests. It seems like what we're really trying to come to a consensus on is how to judge the value of a test. -- Michael Mior [email protected] Le lun. 10 sept. 2018 à 07:53, Vladimir Sitnikov < [email protected]> a écrit : > Michael>just because a test is slow of course doesn't mean it's not useful > > Does "slow test" impact your development experience in a bad way? > Does "slow test" impact contributor's experience in a bad way? > Do things like "MaterializationTest 160sec" and "LatticeTest 212sec" make > materialization/lattice much stronger? > > Spoiler: yes, yes, no. In the latter case, most of the time is spent not in > materialization/lattice engine, but most of the time is spent in data > loading. > That is the tests cover very little cases, and they spend too much time in > testing HSQLDB (or whatever DB is used to load the data). > In other words, current MaterializationTest/LatticeTest are much less > related to Calcite than they relate to H*DB. > > Michael>I'm not sure it makes sense to have one single duration that's > acceptable > for all tests > > Of course there are always exceptions. Like the one I have listed with > "Cassandra startup". > > Let me rephrase: "what test duration would make your eyebrow to raise?" > What test duration would raise both of your eyebrows? > > Michael>We could also put some slower tests behind a define since > just because a test is slow of course doesn't mean it's not useful. > > Calcite is an intermediate framework. It is not a database. It is not a > training framework for machine learning. > Thus Calcite should not take much time to perform its duties. > > For instance, if simple query fetches 100 rows by primary key literals > takes 80 seconds to "prepare" in Calcite, I would definitely call it a bug. > It does not mean that that case is "useful to run in a day-to-day" > codebase. > We can have it with @Ignore("the test is too slow, see CALCITE-XXXX"), > however it makes absolutely no sense in running the test on each test > execution. > > Michael>If we are going to pick a single duration, > > Note: I'm not suggesting to veto tests based on its duration. > > Suppose I'm reviewing a PR, and I see that the added test there takes 95 > seconds on my machine. > Is it reasonable to ask the author to reduce test duration? > > Michael>then I think it should be much higher than 5 seconds > > What's your idea on the reasonable duration for a single test? > > Just in case. https://issues.apache.org/jira/browse/CALCITE-2509 shows > that > a simple > coalesce(vInt(0), vInt(1), vInt(2), vInt(3), vInt(4), vInt(5), vInt(6), > vInt(7), vInt(8), vInt(9), vIntNotNull(0)) > takes 30+seconds in RexSimplifyTest. > > Would you mind if I add that kind of enabled-by-default test? > > Vladimir >
