Do tests really use images from 'ReferenceOutput' folder?

[br3aker]

Prerequisites

• [X] I have written a descriptive issue title
• [X] I have verified that I am running the latest version of ImageSharp
• [X] I have verified if the problem exist in both DEBUG and RELEASE mode
• [X] I have searched open and closed issues to ensure it has not already been reported

ImageSharp version

main branch

Other ImageSharp packages and versions

None

Environment (Operating system, version and so on)

Windows 10

.NET Framework version

net6

Description

I was writing tests for jpeg encoder PR and looked at the tests which compare saved images to reference images from 'ReferenceOutput' folder. Thing is, I don't see anything related to that folder in test code. Just to be sure I've deleted 'ReferenceOutput/PngEncoderTests' folder and all of png encoder tests still pass.

Most likely I've missed something but I just can't see any connections to this folder in test code.

Steps to Reproduce

Delete 'ReferenceOutput/PngEncoderTests' folder and launch png tests.

Images

No response

[antonfirsov]

Nice catch! Looks like a bug sneaked into our PngEncoderTests code and we are comparing the image to itself instead of the reference file :)

https://github.com/SixLabors/ImageSharp/blob/75dc96a27f5120451a5a8c00c75d0eaeff68437c/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs#L624-L628

Other encoder tests seem to be correct.

[antonfirsov]

I was writing tests for jpeg encoder PR and looked at the tests which compare saved images to reference images

Note that the mentioned PNG tests are validating generated images. Tests which are working with file inputs are using a utility method VerifyEncoder:

https://github.com/SixLabors/ImageSharp/blob/2aa150c28bfcc0bf3238a4b39d8a3a667f3f9f53/tests/ImageSharp.Tests/Formats/Bmp/BmpEncoderTests.cs#L364-L365

It compares the original image to the one re-decoded from the ImageSharp encoder output by a reference decoder, therefore it doesn't need reference images. Questionable approach for Jpeg, but still used in the current tests. ~Maybe we should create a set of tests with smaller images and create optimized reference PNG-s instead? This would raise the question if we should use ImageSharp or some other software for generating the reference output.~ Scratch that doesn't make sense, I don't have a better idea for result verification than the re-decoding trick.

[br3aker]

Questionable approach for Jpeg, but still used in the current tests.

I don't really have an idea how to compare new output color types in jpeg encoder. Funny thing, current tests found a bug in my PR :D

[JimBobSquarePants]

@antonfirsov We actually do the same in the Bmp Encoder tests for quantized images.

[brianpopow]

What would be a solution to fix this issue?

• Use VerifyEncoder in TestPngEncoderCore and delete the PngEncoder reference images or
• Assume the reference images are correct and use CompareToReferenceOutput() instead?

[JimBobSquarePants]

I'll have to re-read our tests. As a side I really want to put some time to focus on cleaning up our test suite. It's difficult to navigate.