qstommyshu commented on PR #15444: URL: https://github.com/apache/datafusion/pull/15444#issuecomment-2758556657
Hi @alamb and @blaginin , I this PR is mostly done. There are still some areas (non-trivial test cases migration) where I'm not sure if I should or how should I migrate them to `insta`. For example: The [`assert_expected_plan`](https://github.com/apache/datafusion/blob/main/datafusion/substrait/tests/cases/roundtrip_logical_plan.rs) in `roundtrip_logical_plan.rs` accepts a hard-coded `expected_plan_str`, and uses `assert_eq!` internally. I wanted to replace the `assert_eq!` with `assert_snapshot!`, but Rust function does not accept `insta` inline snapshot syntax like `@r`, meaning that I can't pass a raw string to `assert_expected_plan` to perform a inline snapshot assertion. And there are many more functions like this, I can't simply change them as they accepts dynamically generated `plan_str` and `plan.schema()` Just want to know if I need to migrate those non-trivial test cases as well, thanks. -- 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
