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.

Sign up for GitHub

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

Jump to bottom

Updates to SvgW3CTestRunner #1132

Merged
merged 2 commits into from
Jan 24, 2024
Merged

Updates to SvgW3CTestRunner #1132

merged 2 commits into from
Jan 24, 2024

Conversation

paulushub
Copy link
Contributor

@paulushub paulushub commented Jan 20, 2024 •
edited
Loading

Description

  • Fixes exception in pixel difference calculations due to image size difference
  • Added difference percentage calculations from Svg.UnitTests
  • Added a test-run dialog to quickly run tests in a selected tab
  • Added buttons to access the search and test-run dialogs

Exception:

WC3 TEST struct-image-15-f.svg
Result: TEST FAILED
SVG TO PNG COMPARISON ERROR for struct-image-15-f.svg
System.ArgumentException: Parameter is not valid.
   at System.Drawing.Bitmap.LockBits(Rectangle rect, ImageLockMode flags, PixelFormat format)
   at SvgW3CTestRunner.BitmapExtensions.DisposableImageData..ctor(Bitmap bitmap, Rectangle rect, ImageLockMode flags, PixelFormat format) in ...\SVG\Tests\SvgW3CTestRunner\BitmapExtensions.cs:line 26

WC3 TEST struct-image-14-f.svg
Result: TEST FAILED
SVG TO PNG COMPARISON ERROR for struct-image-14-f.svg
System.ArgumentException: Parameter is not valid.
   at System.Drawing.Bitmap.LockBits(Rectangle rect, ImageLockMode flags, PixelFormat format)
   at SvgW3CTestRunner.BitmapExtensions.DisposableImageData..ctor(Bitmap bitmap, Rectangle rect, ImageLockMode flags, PixelFormat format) in ...SVG\Tests\SvgW3CTestRunner\BitmapExtensions.cs:line 26

WC3 TEST struct-image-13-f.svg
Result: TEST FAILED
SVG TO PNG COMPARISON ERROR for struct-image-13-f.svg
System.ArgumentException: Parameter is not valid.
   at System.Drawing.Bitmap.LockBits(Rectangle rect, ImageLockMode flags, PixelFormat format)

Type of change

Only SvgW3CTestRunner is updated.

  • Bug fixes, no change in main source codes (non-breaking change which fixes an issue)
  • This change does not require a documentation update
  • Resolved review issues.

How Has This Been Tested?

  • SvgW3CTestRunner is tested for each supported platform
  • Svg.UnitTests tested on command line for each supported platform
  • Svg.Benchmarks tested on command line for each supported platform, warnings are as before will be addressed later.

@paulushub
Updates to SvgW3CTestRunner
31d0b4c 
- Fixes exception in pixel difference calculations due to image size difference
- Added difference percentage calculations from Svg.UnitTests
- Added a test-run dialog to quickly calculation tests in a selected tab
- Added buttons to access the search and test-run dialogs
@paulushub paulushub added the bug label Jan 20, 2024
@paulushub paulushub self-assigned this Jan 20, 2024
mrbean-bremen
mrbean-bremen reviewed Jan 20, 2024
View reviewed changes
Tests/SvgW3CTestRunner/BitmapExtensions.cs
private static readonly int ImageWidth = 64;
private static readonly int ImageHeight = 64;

public static float PercentageDifference(this Image img1, Image img2, byte threshold = 10)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand this is copied over from ImageComparisonTest. I wonder if it makes sense to move this into a common component to avoid the duplication (and potential bugs if the code is changed in ne place only).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes sense as the shared parts are increasing. What name will you suggest for the shared code?

  • Svg.Tests.Common
  • Svg.Tests.Shared
  • Svg.Tests.Core
  • etc

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Common sounds fine, but I have no strong feelings here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, will look into it - I will be leaving for church service, and they also requested some IT services from me so will not be available for at least 8 hours.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mrbean-bremen After looking into it, we should consider the shared library as separate PR.
Currently, the Svg.Benchmark project references the Svg.UnitTests project just to access an extension method, and I think the shared library should extend there too.
As a separate PR, we will give it a better review.

    <ItemGroup>
        <ProjectReference Include="..\..\Source\Svg.csproj" />
        <ProjectReference Include="..\Svg.UnitTests\Svg.UnitTests.csproj" />
    </ItemGroup>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, agreed!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, if there is no remaining issue on this PR, can I proceed with the merge?

Tests/SvgW3CTestRunner/RunTestsDialog.cs
@@ -0,0 +1,223 @@
using System;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not going to review the GUI code, instead I just ran it and checked the outcome.
I think this is very helpful - both the separate runner (which also shows that some of the tests marked as "passing" are not really working), and the display of the difference in the original TestRunner.

Copy link
Contributor Author

@paulushub paulushub Jan 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After testing with the #945 PR, I have decided to improve it.

  • it is now modeless, and can be minimized.
  • you can double-click or click a button to view the rendered images in the main TestRunner.
  • you copy the selected file name or path to a clipboard.

Pending

  • export the results so what we can compare the percentage, exception changes in each PR.

We will need to recreate the passing-failing lists after completing the PR merges and start work on the Issues.

@paulushub
Improvements to the tests-run dialog
a379e87 
- it is modeless
- exports the tests run
- added positive and negative groups to indicate changes from PR to PR
- added revisit group: a suggestions of test result to evaluate (whether passing or failing)
- double-click or click a button to view the result in main TestRunner
@paulushub paulushub merged commit 3b41c8c into svg-net:master Jan 24, 2024
7 checks passed
github-actions bot pushed a commit that referenced this pull request Jan 24, 2024
Updates to SvgW3CTestRunner (#1132) CONTRIBUTING.md Generators README…
8edf97b 
….md Samples Source Svg.Custom Tests doc docfx.json index.md license.txt Updates to SvgW3CTestRunner - Fixes exception in pixel difference calculations due to image size difference - Added difference percentage calculations from Svg.UnitTests - Added a test-run dialog to quickly calculation tests in a selected tab - Added buttons to access the search and test-run dialogs CONTRIBUTING.md Generators README.md Samples Source Svg.Custom Tests doc docfx.json index.md license.txt Improvements to the tests-run dialog - it is modeless - exports the tests run - added positive and negative groups to indicate changes from PR to PR - added revisit group: a suggestions of test result to evaluate (whether passing or failing) - double-click or click a button to view the result in main TestRunner
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrbean-bremen mrbean-bremen mrbean-bremen approved these changes

Assignees

@paulushub paulushub

Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@paulushub @mrbean-bremen