You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@calcite.apache.org by "Julian Hyde (Jira)" <ji...@apache.org> on 2021/04/28 03:53:00 UTC
[jira] [Created] (CALCITE-4593) DiffRepository tests should fail if
XML resources are not in alphabetical order
Julian Hyde created CALCITE-4593:
------------------------------------
Summary: DiffRepository tests should fail if XML resources are not in alphabetical order
Key: CALCITE-4593
URL: https://issues.apache.org/jira/browse/CALCITE-4593
Project: Calcite
Issue Type: Bug
Reporter: Julian Hyde
Tests that use XML resources managed via {{class DiffRepository}} should fail if the XML resources are not in alphabetical order.
First, some background. Quite a few tests store resources in an XML file, accessed via {{class DiffRepository}}. For example, {{RelOptRulesTest}} stores the plan before and after the rule(s) that it is testing are fired. The resources are organized by test case name, enclosed in {{<TestCase>}} elements.
Contributors have a tendency to add test resources at the end of the file. But this makes the end of the file a hot-spot for conflicts.
Therefore the best practice is to put the resources into alphabetical order. {{DiffRepository}} tries to help with this, by generating an {{_actual.xml}} file with the new resource in inserted in its right alphabetical position, but contributors somehow miss this, and write the file by hand. So, conflicts.
With this change, a test will fail if resources are out of order. The message will look something like this:
{noformat}
java.lang.IllegalArgumentException: expected 10 test cases to be out of order, but there were 11; here are the new ones:
"testAggregateRemove6"
at com.google.common.cache.LocalCache$Segment.get(LocalCache.java:2051)
at com.google.common.cache.LocalCache.get(LocalCache.java:3951)
at com.google.common.cache.LocalCache.getOrLoad(LocalCache.java:3974)
at com.google.common.cache.LocalCache$LocalLoadingCache.get(LocalCache.java:4958)
at com.google.common.cache.LocalCache$LocalLoadingCache.getUnchecked(LocalCache.java:4964)
at org.apache.calcite.test.DiffRepository.lookup(DiffRepository.java:808)
at org.apache.calcite.test.RelOptRulesTest.getDiffRepos(RelOptRulesTest.java:190){noformat}
Why does it say "expected 10 test case to be out of order, but there were 11"? Because resource files are not currently in total order: they were not sorted to start with, and we don't want to destroy history by sorting them. But to solve the conflict problem, we only need *new* test cases to be in order. So, this change adds a list of exceptions - test cases that are known to be out of order - and {{DiffRepository}} will not complain if those test cases are out of order. Over time, the sort order of resource files will get better, or at least will not get any worse.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)