[
https://issues.apache.org/jira/browse/CALCITE-4593?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Julian Hyde resolved CALCITE-4593.
----------------------------------
Resolution: Fixed
Fixed in
[dfb934ab|https://github.com/apache/calcite/commit/dfb934aba5ca830066a0d683c888070165283de6].
> 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
> Assignee: Julian Hyde
> Priority: Major
> Labels: pull-request-available
> Fix For: 1.27.0
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> 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)
