iceberg-rust icon indicating copy to clipboard operation
iceberg-rust copied to clipboard

Snapshot Testing for Integration Tests

Open feniljain opened this issue 1 year ago • 10 comments

Hey everyone! 👋🏻

Creating this issue to continue discussion from these messages on possibility of using snapshot testing for integration tests:

  • https://github.com/apache/iceberg-rust/pull/742#issuecomment-2516279844
  • https://github.com/apache/iceberg-rust/pull/742#issuecomment-2523503652

feniljain avatar Dec 14 '24 14:12 feniljain

@ZENOTME you mentioned:

A creative idea is to support Avro format files, allowing us to create snapshots of the entire Iceberg metadata, which can then be used for quick comparisons in end-to-end tests.

Do you mean using avro metadata files directly as snapshots? I am not sure how that addresses your concern of:

One thing we need to address maybe filter out some random field.

Am I understanding something wrong here? Can you please elaborate a bit more :)

feniljain avatar Dec 14 '24 14:12 feniljain

@ZENOTME you mentioned:

A creative idea is to support Avro format files, allowing us to create snapshots of the entire Iceberg metadata, which can then be used for quick comparisons in end-to-end tests.

Do you mean using avro metadata files directly as snapshots? I am not sure how that addresses your concern of:

One thing we need to address maybe filter out some random field.

Am I understanding something wrong here? Can you please elaborate a bit more :)

Sorry, I didn't have a clear expression here. Actually, they are two things independently here.

ZENOTME avatar Dec 16 '24 03:12 ZENOTME

I'm also a fan of snapshot testing. In this PR https://github.com/apache/iceberg-rust/pull/822, I used expect-test to do it.

xxchan avatar Dec 19 '24 04:12 xxchan

Thanks @feniljain for raising this. I'm fine with snapshot testing, but currently didn't come up with a concrete example to do it.

liurenjie1024 avatar Jan 02 '25 03:01 liurenjie1024

I find this issue during reviewing https://github.com/apache/iceberg-rust/pull/1265

Maybe we can use https://github.com/mitsuhiko/insta for better DX?

Xuanwo avatar Apr 27 '25 08:04 Xuanwo

We already have expect-test snapshot tests, as mentioned above, why not just use it? (To clarify, I'm fine with switching if people prefer.)

BTW, share a very nice blog about snapshot testing here: https://matklad.github.io/2025/04/15/underusing-snapshot-testing.html

xxchan avatar Apr 27 '25 08:04 xxchan

Snapshot testing should also be perfectly suitable for test like this (args parsing) https://github.com/apache/iceberg-rust/pull/1220/files#diff-cb9d997d5104f8406186a80782bdc7f9bde4426ec307b73ce710d94bfae5ac5aR2270-R2295

xxchan avatar Apr 27 '25 08:04 xxchan

We already have expect-test snapshot tests, as mentioned above, why not just use it? (To clarify, I'm fine with switching if people prefer.)

mitsuhiko/insta is used more widely and provides better DX like a vscode extension (https://marketplace.visualstudio.com/items?itemName=mitsuhiko.insta).

BTW, share a very nice blog about snapshot testing here: https://matklad.github.io/2025/04/15/underusing-snapshot-testing.html

Thanks a lot for the share!

Snapshot testing should also be perfectly suitable for test like this (args parsing) https://github.com/apache/iceberg-rust/pull/1220/files#diff-cb9d997d5104f8406186a80782bdc7f9bde4426ec307b73ce710d94bfae5ac5aR2270-R2295

Agreed. It should be good for us to migrate tests.

Xuanwo avatar Apr 27 '25 09:04 Xuanwo

provides better DX like a vscode extension (marketplace.visualstudio.com/items?itemName=mitsuhiko.insta).

Regarding this, expect-test is supported natively by Rust Analyzer vscode extension 🤣.

Image

Actually I think expect-test has smaller cognitive load for new comers: no extra tooling is needed, just cargo test. And I prefer "inline snapshot" better (although also support by insta, but not the default suggestion in doc)

Just FYI, I'm fine with whatever choice.

xxchan avatar Apr 27 '25 09:04 xxchan

Thank you @xxchan for mentioning this. I just learned about that.

If you're okay with either option, I would prefer to use insta here since it has wider adoption and is more actively maintained.

Xuanwo avatar Apr 27 '25 09:04 Xuanwo

This issue has been automatically marked as stale because it has been open for 180 days with no activity. It will be closed in next 14 days if no further activity occurs. To permanently prevent this issue from being considered stale, add the label 'not-stale', but commenting on the issue is preferred when possible.

github-actions[bot] avatar Oct 26 '25 00:10 github-actions[bot]

This issue has been closed because it has not received any activity in the last 14 days since being marked as 'stale'

github-actions[bot] avatar Nov 09 '25 00:11 github-actions[bot]