hy144328 commented on PR #40277: URL: https://github.com/apache/superset/pull/40277#issuecomment-4772912837
> LGTM! One tiny thing before I merge: the `assert_*_with_text` helper is copy-pasted into presto/doris/trino/hive test files. Might be worth pulling into a shared test util so we're not maintaining four copies. Happy to take it as a follow-up rather than block on it, or you can tackle it here if you have the time. @rusackas Thanks for the feedback. I have learnt about [DAMP](https://abseil.io/resources/swe-book/html/ch12.html#shared_helpers_and_validation) tests the other day. But DRY is fine by me, too. [5859ecb](https://github.com/apache/superset/pull/40277/commits/5859ecb8fbba30a250df3fec76a75673a96f90c1) places the assertion helpers into a shared module. Hope this will do. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
