mitsuhiko / insta

A snapshot testing library for rust
https://insta.rs
Apache License 2.0
2.16k stars 98 forks source link

Binary Snapshot Support #196

Closed louy2 closed 1 day ago

louy2 commented 2 years ago

I'd like to test a function rendering to an image, and I want to use snapshot testing. Currently I am using the Debug serializer but the resulting file is larger than my image and diff is useless. I'd really appreciate if I can generate image files and compare them visually, which means having the snapfiles open as image files.

I have thought about writing a serde serializer for image::DynamicImage, but AFAIK insta always adds the header so the resulting file cannot conform to an image format, which may prevent the file from being viewed as an image directly.

The metadata in the headers originally may be placed in EXIF tag UserComment for PNG and JPEG and in Comment Extension for GIF, although image doesn't seem to support metadata when encoding.

mitsuhiko commented 2 years ago

Even worse right now is the fact that insta is writing strings. So you won't be able to get anything binary through. I do agree though that there are cases where binary files are useful and screenshots is one of those reasons.

I think one way to approach this would be to emit .snap files with a new header that points to an out-of-band binary file. So something like this:

foo.snap

source: tests/test_image.rs
expression: "process_image()"
binary_snapshot: "foo.snap.png"

And then have the file in foo.snap.png. This has the added benefit that the original file extension can be kept (eg: png for a screenshot) and tools can better work with it.

aminya commented 1 year ago

By adding support for postcard, this issue can be fixed. Postcard serializes the data into a vector of numbers.

mitsuhiko commented 1 year ago

The issue is reviewing this. When working with binary snapshots (for instance pngs) one would want the snap files to be in a format that can be put into a viewer. So one idea was to make the snap files only contain the metadata and put the binary payload into an adjacent file.

aminya commented 1 year ago

Postcard supports deserialization from the stored bytes. So, the data can be restored and displayed using the Debug or Display trait.

mitsuhiko commented 1 year ago

Yes, but that does not address the problem that the file on the file system is not in a format that you can directly open with a viewer.

adriangb commented 1 year ago

+1 for this feature.

Even if the file isn't viewable in a viewer (in my case I'm snapshotting Postgres binary dumps) it is still useful to store it as a standalone file (in my case so that I can integration test by copying it via psql).

I think the snap could also hold some meta information about the binary file. Like size, maybe a hash? Size could be useful to see what's going on at a glance, hash I can't think of an immediate use case but can't hurt to have it.

peterstuart commented 1 year ago

@mitsuhiko I’m interested in working on this if you’d accept a PR implementing it. The approach you outline in https://github.com/mitsuhiko/insta/issues/196#issuecomment-989792972 seems reasonable to me.

mitsuhiko commented 1 year ago

@peterstuart yeah, would be super happy to accept patches for this.

nyurik commented 11 months ago

How about actually showing the real images in the terminal, side by side, using viu?

P.S. Insta could also print two full paths to the files, or possibly even use open on request?

zoechi commented 7 months ago

Showing diffs like https://github.com/nicolashahn/diffimg Flutter provides something like that with golden tests https://api.flutter.dev/flutter/flutter_test/GoldenFileComparator-class.html

lasernoises commented 5 months ago

I was trying to imlement something like this and I thought having side-by-side images would be nice. So I used the viuer crate as suggested by nyurik.

However i ran into an issue when the images are higher than the terminal window. It seems that terminals don't support moving the cursor back up above the current view and printing the image triggers scrolling down. This causes a weird effect where one of the images is vertically shifted down from the other one if one of them is larger than the available height.

I think the kitty image protocol would support positioning the image from a bottom corner instead of the top left. But the iterm protocol doesn't seem to support this and i don't know about sixel.

This issue could be solved by generating one image out of both and displaying that.

I also thought that it might be nice to display text next to the image if the snapshot changes from a text snapshot to an image snapshot. In that case it would also cause this vertical shift issue if either the text or the image are larger than the terminal height.

It would also be possible to shrink the image to the height of the terminal window, but i think that would often lead to The images being too small to be able to visually compare. Or we could put the terminal into an alternate screen mode and handle scrolling, but i think that would add a lot of complexity and would probably cause flickering from clearing and adding the images again.

The other issue is that a lot of terminal emulators (like gnome-terminal) don't support any images and half blocks are just too low-resolution for many use-cases.

So I think the most reasonable solution is to only have an action that opens the files in an external viewer. In that case it would probably also make sense to support arbitray file extensions.

I think in that case the macro should probably accept a closure with an &mut File parameter to avoid having to put the entire file contents into a Vec<u8>.

@mitsuhiko do you think this approach is reasonable?

You can find my (very unfinished) implementation with the side-by-side images at https://github.com/lasernoises/insta/tree/image-experiment.

mitsuhiko commented 5 months ago

I think showing stuff in the terminal is not a good strategy. I would rather surface the individual files and provide ways to open up an external diff viewer / visualization tool. In general I also think if a binary snapshot feature should be added (which I'm in favor of) it should not assume that images are the only binaries.

I think ideally this starts with a proposal of how assertions work and where and how the snapshots are stored.

max-sixty commented 1 day ago

Closed by #610