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

SpectralConverter.CommitConversion fails in Debug #1998

Closed
antonfirsov opened this issue Feb 15, 2022 · 4 comments · Fixed by #2000
Closed

SpectralConverter.CommitConversion fails in Debug #1998

antonfirsov opened this issue Feb 15, 2022 · 4 comments · Fixed by #2000
Labels
Milestone

Comments

@antonfirsov
Copy link
Member

antonfirsov commented Feb 15, 2022

ImageSharp version

2.0

Other ImageSharp packages and versions

N/A

Environment (Operating system, version and so on)

any

.NET Framework version

any

Description

The following assertion fails in almost all of our baseline jpeg tests:

public void CommitConversion()
{
DebugGuard.IsFalse(this.Converted, nameof(this.Converted), $"{nameof(this.CommitConversion)} must be called only once");

@br3aker any idea why is CommitConversion called multiple times? Could this lead to actual issues?

Steps to Reproduce

Run JpegDecoderTests.DecodeBaselineJpeg in Debug.

Images

No response

@br3aker
Copy link
Contributor

br3aker commented Feb 15, 2022

That's strange, I've tested it locally and it worked. Will wokr on it asap.

Could this lead to actual issues?

Most likely no but nothing should call it multiple times.

@antonfirsov
Copy link
Member Author

I've tested it locally and it worked

Hm hope it's not some black magic on my machine only.

@br3aker
Copy link
Contributor

br3aker commented Feb 15, 2022

Can confirm, already found a bug (it's a very silly one).

Why did CI passed it? Doesn't it have a DEBUG job?

@antonfirsov
Copy link
Member Author

Why did CI passed it? Doesn't it have a DEBUG job?

Unfortunately, no. IMO, would be nice to have it at least against one target framework. Alternatively, we can introduce Checked build configuration, so avoid very long Debug test runs.

@JimBobSquarePants JimBobSquarePants added this to the 2.1.0 milestone Mar 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants