Thanks for taking care of this Alessandro! I believe the resources are still sorted as per [1] but it appears that there are still a few manual edits to the file.
If we want to prevent such changes in the future it may be a good idea to add a post-class check In RelOptRulesTest (or somewhere in DiffRepo) checking that files are identical. Best, Stamatis [1] https://lists.apache.org/thread/j0fh9hhxdc9lc7n228kwj6xcs0z9hpb3 On Thu, Oct 20, 2022 at 8:36 PM Julian Hyde <[email protected]> wrote: > Do you think that RelOptRulesTest should fail if the resources are not > sorted alphabetically? (I thought it did that at some point… I may be > wrong. Anyway, I am a fan of ‘fail fast’. The person who introduces the > error should be the person to fix it.) > > > On Oct 20, 2022, at 9:20 AM, Alessandro Solimando < > [email protected]> wrote: > > > > Hello everyone, > > I have recently added tests to RelOptRulesTest.java, took the chance to > > update (maven instruction with gradle instructions, path has changed) and > > improved them a little bit. > > > > While testing the commands I have noticed that few PRs in the last months > > had tests added to RelOptRulesTest.java with a manual update of > > RelOptRulesTest.xml (possibly due to the stale instructions). > > > > Manual updates often suffer from formatting issues, stale/wrong SQL > > statements, different test order. > > > > Note that those discrepancies are not caught by running tests/CI, but > it's > > nonetheless problematic, especially because it forbids folks down the > line > > to re-generate the file automatically, because their PRs would carry the > > changes from past work. > > > > In https://github.com/apache/calcite/pull/2944 I will put back on track > > RelOptRulesTest.xml, please try to follow the procedure in the javadoc of > > RelOptRulesTest.java and let me know if we can improve them even further > in > > your opinion. > > > > Best regards, > > Alessandro > >
