logan-keede commented on code in PR #14306: URL: https://github.com/apache/datafusion/pull/14306#discussion_r1929840951
########## datafusion/sqllogictest/test_files/aggregate/complete_aggregate.slt: ########## Review Comment: > It seems the tests will be executed twice, how about we just left the one complete test file? > > Because we will only move testcases incrementlly after this pr, seems we can ensure no cases are lost by this way: > > * move cases from `complete_aggregate.slt` to `function1.slt` > * get `diff` between current moved `complete_aggregate.slt` > * compare `diff` and `function1.slt` > > And it seems great if we can make this an automatic process? If I understood you correctly, My fear is that we might lose track of what we have already moved, we might not be able to make sure that sum of all `funtions_*.slt` is equal to `old_aggregate.slt` or not, but for `base_aggregate.slt` we know if something is present in it, it is not present in any of the functions file. beside I can not think of test running twice as a bad thing, it is like an extra layer of security at the cost of ~5 sec of ci time(even on 1 thread). I can maintain this extra file on my local system but that is like binding this issue to me, it will be easier for anyone to contribute in spliting this file if we keep both. I definitely agree with making a README file, I was considering it myself. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org
