Skip to content

Conversation

@RunDevelopment
Copy link
Contributor

This PR adds tests for encoders along with a few improvements and fixes.

The tests are reference tests similar to the existing ones. Basically, the encoder is given a dyn image and is asked to encode it. We then save the encoded image to disk and commit it. Additionally, the encoded image is then opened to verify that it is valid and saved as PNG to show the output (in case pixel data changed). PNGs are git-ignored.

Similar to #17, files are only written to disk if the BLESS env var is set.

Other changes:

  • Fixed WbmpEncoder ignoring threshold.
  • Fixed links for OTB. Turns out, the WAE spec has nothing to say about OTB.
  • Removed unnecessary lifetime from OtbEncoder and WbmpEncoder. This is now consistent with all encoders in image.
  • *Encoder::new now returns Self instead of an ImageResult<Self> that is always Ok for all encoders. This is now consistent with all encoders in image.

@fintelia
Copy link
Contributor

Encoding isn't necessarily deterministic, so I'd rather limit to just roundtrip testing where we check that feeding an encoded image back into our decoder produces the original contents.

@RunDevelopment
Copy link
Contributor Author

Roundtrip testing only works for lossless formats. The two formats I tested only support binary black and white. Roundtrip testing can still be used with the right images, but as soon as any information is lost during encoding, roundtrip testing can't be used anymore. E.g. it wouldn't have been able to catch the threshold bug in WbmpEncoder.

But I agree that determinism is a concern. So how about we do both? If the format is lossless (for certain images), use roundtrip testing. If it's deterministic, test using references (snapshots). Of course, those two don't exclude each other. What do you think?

(Also, it can be useful to write out a few images. Just because the decoder we implement can read them, doesn't mean others can.)

@fintelia
Copy link
Contributor

My goal is to ensure that tests only fail due to bugs, not because of code changes that happen to produce different output. Many (most?) image formats have a huge number of possible ways to encode any given image, so asserting that an encoder for one of those formats produces bit exact exact output will lead to spuriously failing tests.

I'm fine with whatever sort of test if it isn't going to produce spurious failures

@RunDevelopment
Copy link
Contributor Author

Alright. Then I'll make something for roundtrip testing later.

@RunDevelopment RunDevelopment deleted the test-encoders branch November 26, 2025 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants