Re: [PR] Migrate user_defined tests to insta [datafusion]

[via GitHub]

shruti2522 commented on PR #15255:
URL: https://github.com/apache/datafusion/pull/15255#issuecomment-2730590045

   > > I tried allow_duplicates!(), but it gets a bit tricky with async 
functions
   > 
   > Can you please explain more on this? I tried modifying _async_ 
`run_and_compare_query` and this worked for me:
   > 
   > <img alt="image" width="789" 
src="https://private-user-images.githubusercontent.com/19199204/423473541-0589862a-f891-453f-aa1d-e3af3a77aaa0.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3NDIyMzg4NjYsIm5iZiI6MTc0MjIzODU2NiwicGF0aCI6Ii8xOTE5OTIwNC80MjM0NzM1NDEtMDU4OTg2MmEtZjg5MS00NTNmLWFhMWQtZTNhZjNhNzdhYWEwLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAzMTclMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMzE3VDE5MDkyNlomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWU1MjMzNTBhYTRhMTgwMTg0ZWJjY2MzNGNjNmRjZDIzM2RiYTJjNDFiODVkYzNjNDgzMWRjMTZlMWU1Zjc2MTImWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.qkH-1A7Ps5aPDHRAlXn7vrZJ8YhkbX68oB1V4E3AxbQ";>
   > > So, I used snapshot files instead because they make it much easier to 
run multiple tests using the same query fn, also it makes it much easier to 
update expected outputs in the future and eliminates the use of 
#[rustfmt::skip] in some places.
   > 
   > Just to give a bit of context, we had a discussion about inline / file 
snapshots here: [#13672 
(comment)](https://github.com/apache/datafusion/pull/13672#discussion_r1894531818)
 and [#13672 
(comment)](https://github.com/apache/datafusion/pull/13672#discussion_r1894532385).
   > 
   > To me personally, it feels like we can use snapshot files, but the 
snapshot itself should give enough information to understand the context. For 
example, in CLI, you can see [program input and 
tests](https://github.com/apache/datafusion/blob/87eec43856a5d8cefef24d1ff85d375d2b58d8c2/datafusion-cli/tests/snapshots/cli_format%40csv.snap#L6-L9),
 and the snapshot file itself doesn't contain a lot of logic. Here I'm not sure 
if that's the case for ALL the test fixtures? Maybe some of them can be inline?
   
   Yes, it worked. I was using` insta::allow_duplicates!` like a function call:
   ```rust
    insta::allow_duplicates!(|| {
           insta::with_settings!({
               description => description,
           }, {
               insta::assert_snapshot!(actual, @r###"
               +-------------+---------+
               | customer_id | revenue |
               +-------------+---------+
               | paul        | 300     |
               | jorge       | 200     |
               | andy        | 150     |
               +-------------+---------+
               "###);
           });
       })();
   ```
   On changing it to this it worked  
   ```rust
    insta::allow_duplicates! {
           insta::with_settings!({
               description => description,
           }, {
               insta::assert_snapshot!(actual, @r###"
               +-------------+---------+
               | customer_id | revenue |
               +-------------+---------+
               | paul        | 300     |
               | jorge       | 200     |
               | andy        | 150     |
               +-------------+---------+
           "###);
           });
       }
   ```
   Thanks a lot @blaginin and @alamb ❤️


-- 
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