Skip to content

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

Description

@br3aker

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

Activity

  1. antonfirsov commented on Jun 20, 2022

    @antonfirsov
    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.

  2. antonfirsov commented on Jun 20, 2022

    @antonfirsov
    Member

    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.

  3. added this to the 3.0.0 milestone on Jun 20, 2022
  4. br3aker commented on Jun 21, 2022

    @br3aker
    ContributorAuthor

    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

  5. JimBobSquarePants commented on Jun 22, 2022

    @JimBobSquarePants
    Member

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

  6. brianpopow commented on Aug 30, 2022

    @brianpopow
    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?
  7. JimBobSquarePants commented on Aug 30, 2022

    @JimBobSquarePants
    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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions