villebro commented on PR #30325: URL: https://github.com/apache/superset/pull/30325#issuecomment-2361450114
> @villebro I can mimic those tests for Postgres. Though I wonder about the rigor of comparing the code in my spec to pasting the same code in a test. What if instead/also I had it test the transformation of a specific timestamp? Like that rounding `... 00:15:01` to 1 minute intervals gives `... 00:15:00` etc. Unless that's already happening here and I'm missing it. So the reason why these tests are valuable, despite looking like pure duplication, is because a change might happen that causes these time grain definitions to no longer be used. For example, I once did a big refactor of the db engine specs (#7676), and that refactor caused the time grains to be removed by mistake from the Hive engine spec (this was later fixed in #10084). If we would have had tests like this in place, we would have caught that error during the refactor. I know it may look funny to duplicate that functional logic to the unit tests, but having them there makes it less likely to introduce regressions, and it also makes it safer to refactor the implementation details. Regarding actual functional verification that the expression actually rounds down the timestamp based on the time grain, that is something that we can't unfortunately test here, as that logic resides in Postgres. But correct me if I'm misunderstanding what you're saying. -- 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: notifications-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
