Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do tests really use images from 'ReferenceOutput' folder? #2158

Open
4 tasks done
br3aker opened this issue Jun 19, 2022 · 6 comments
Open
4 tasks done

Do tests really use images from 'ReferenceOutput' folder? #2158

br3aker opened this issue Jun 19, 2022 · 6 comments
Labels
Milestone

Comments

@br3aker
Copy link
Contributor

br3aker commented Jun 19, 2022

Prerequisites

  • I have written a descriptive issue title
  • I have verified that I am running the latest version of ImageSharp
  • I have verified if the problem exist in both DEBUG and RELEASE mode
  • 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
Copy link
Member

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

using (var imageSharpImage = Image.Load<TPixel>(actualOutputFile, new PngDecoder()))
using (var referenceImage = Image.Load<TPixel>(actualOutputFile, referenceDecoder))
{
ImageComparer.Exact.VerifySimilarity(referenceImage, imageSharpImage);
}

Other encoder tests seem to be correct.

@antonfirsov
Copy link
Member

antonfirsov commented Jun 20, 2022

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:

// Does DebugSave & load reference CompareToReferenceInput():
image.VerifyEncoder(provider, "bmp", bitsPerPixel, encoder, customComparer);

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.

@antonfirsov antonfirsov added this to the 3.0.0 milestone Jun 20, 2022
@br3aker
Copy link
Contributor Author

br3aker commented Jun 21, 2022

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
Copy link
Member

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

@brianpopow
Copy link
Collaborator

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
Copy link
Member

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.

@JimBobSquarePants JimBobSquarePants modified the milestones: 3.0.0, Future Feb 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants