blaginin commented on issue #15791: URL: https://github.com/apache/datafusion/issues/15791#issuecomment-2934726143
> should I call assert_snapshot!() once for each line individually, or combine all the target lines into a single snapshot call? I like the second option - check the whole logical plan with metrics in one big assert. There will be variable parts (e.g. `elapsed_compute=47.332µs`), but you can handle those using the [redaction](https://insta.rs/docs/redactions/#selectors). > I modified it to return the matching lines from the plan We switched to insta to be able to automatically update all snapshots when making a big formatting-related change. The issue with the "matching" approach is that automatic updates won't work - one would need to manually update the matching line to fix the tests. -- > Only move the plan text checking from the macro and replace it with assert_snapshot!() You can also keep `assert_snapshot` in place, just call `assert_snapshot` internally - [example](https://github.com/apache/datafusion/pull/15984/files#diff-c211e7f11e4de0cc04173e5e16ab9e6a97a72f1b0057690761f35eb154e27aa3R146) -- Also, feel free to split this ticket into several PRs - I think that way you'll get feedback faster :) -- 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
