Update Dependencies #1378

JimBobSquarePants · 2020-10-12T22:15:04Z

Prerequisites

I have written a descriptive pull-request title
I have verified that there are no overlapping pull-requests open
I have verified that I am following matches the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
I have provided test coverage for my change (where applicable)

Description

Updating all possible dependencies to their latest versions.

There is a known issue #1380 with the reference BMP encoder on Ubuntu that I have applied a workaround for now by skipping on Ubuntu CI. This affects our coverage report but is tested on all other platforms.

Fixed.

I have a single outstanding upgrade of Microsoft.Net.Compilers.Toolset required but there's an issue that either needs fixing upstream for that or we can use the ActiveIssueAttribute to skip the affected tests on NET 4.7.2

~~Thoughts please on that.~~

I've disabled the codec tests using ActiveIssueAttribute and removed unnecessary implementations of the executor for the BokehBlur tests.

Looks like the executor issue now fails on Ubuntu also. 😞
https://github.com/SixLabors/ImageSharp/pull/1378/checks?check_run_id=1256202313#step:6:351

The file only exists if there are binding redirects.

This reverts commit 94ba508.

codecov · 2020-10-13T23:37:34Z

Codecov Report

Merging #1378 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1378   +/-   ##
=======================================
  Coverage   82.85%   82.85%           
=======================================
  Files         690      690           
  Lines       31074    31074           
  Branches     3512     3512           
=======================================
  Hits        25747    25747           
  Misses       4605     4605           
  Partials      722      722

Flag	Coverage Δ
#unittests	`82.85% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 70d636d...314e9f6. Read the comment docs.

JimBobSquarePants · 2020-10-13T23:47:43Z

tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs

@@ -129,10 +129,10 @@ public async Task DecodeAsnc_DegenerateMemoryRequest_ShouldTranslateTo_ImageForm
        [Theory]
        [InlineData(TestImages.Jpeg.Baseline.Jpeg420Small, 0)]
        [InlineData(TestImages.Jpeg.Issues.ExifGetString750Transform, 1)]
-        [InlineData(TestImages.Jpeg.Issues.ExifGetString750Transform, 10)]
+        [InlineData(TestImages.Jpeg.Issues.ExifGetString750Transform, 15)]


Test was flaky on NET Core 2.1, this seemed to make it less so.

JimBobSquarePants · 2020-10-14T21:56:16Z

tests/ImageSharp.Tests/TestUtilities/TestEnvironment.cs

@@ -170,7 +170,10 @@ private static void PrepareRemoteExecutor()
            }

            string testProjectConfigPath = TestAssemblyFile.FullName + ".config";
-            File.Copy(testProjectConfigPath, remoteExecutorConfigPath);
+            if (File.Exists(testProjectConfigPath))


This file only exists if there are binding redirects.

antonfirsov

A few concerns/questions.

antonfirsov · 2020-10-14T23:03:15Z

tests/Directory.Build.targets

-    <PackageReference Update="Microsoft.NET.Test.Sdk" Version="16.5.0" />
-    <PackageReference Update="Moq" Version="4.14.5" />
+    <PackageReference Update="Microsoft.DotNet.RemoteExecutor" Version="6.0.0-beta.20513.1" />
+    <PackageReference Update="Microsoft.DotNet.XUnitExtensions" Version="6.0.0-beta.20513.1" />


Are we adding this only because of ActiveIssueAttribute?

Ahh, I see it's not a trivial feature because of the optional TargetFrameworkMonikers parameter. Cool 👍

Yeah, it's pretty nifty. Some cool other attributes available also.

Not actually needed now but keeping it as it's far too useful!

antonfirsov · 2020-10-14T23:15:44Z

tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs

+            where TPixel : unmanaged, IPixel<TPixel>
+        {
+            // dotnet xunit doesn't respect filter.
+            if (TestEnvironment.IsFramework)


We are keeping dotnet xunit around only to do the x86 test run, which eventually happens only on Framework for some reason. Would be important to get the 32bit coverage across all platforms with dotnet test, where dotnet(.exe) has to come from a 32 bit SDK, see xunit/xunit#1123 (comment)

Is this something we can easily get with build-and-test.yml?

I don't know. Originally I wanted to do it via setup-dotnet but choosing the platform architecture there is not supported and no-one seems interested in the feature

actions/setup-dotnet#72

I'm not sure the x86 SDKs are installed by default.

https://github.com/actions/virtual-environments/blob/main/images/win/Windows2019-Readme.md#net-core-sdk

antonfirsov · 2020-10-14T23:17:01Z

tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs

            static void RunTest(string providerDump, string nonContiguousBuffersStr)
            {
                TestImageProvider<TPixel> provider = BasicSerializer.Deserialize<TestImageProvider<TPixel>>(providerDump);

                provider.LimitAllocatorBufferCapacity().InPixelsSqrt(100);

                using Image<TPixel> image = provider.GetImage(GifDecoder);
-                image.DebugSave(provider);
+                image.DebugSave(provider, nonContiguousBuffersStr);


antonfirsov · 2020-10-14T23:19:00Z

tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Progressive.cs

@@ -44,7 +62,7 @@ static void RunTest(string providerDump, string nonContiguousBuffersStr)
            RemoteExecutor.Invoke(
                RunTest,
                providerDump,
-                enforceDiscontiguousBuffers ? "Disco" : string.Empty)
+                "Disco")


We no longer need the parameter I guess.

antonfirsov · 2020-10-14T23:21:23Z

tests/ImageSharp.Tests/Processing/Processors/Convolution/BokehBlurTest.cs

-            RemoteExecutor
-                .Invoke(RunTest, BasicSerializer.Serialize(provider), BasicSerializer.Serialize(value))
-                .Dispose();
+            provider.RunValidatingProcessorTest(


This, and below tests were remote executed to reduce memory pressure, and the chance of OOM-s, especially on x86. BokehBlur is very memory intensive.

They seem to be ok now. As I recall it uses less memory than it did originally. Haven't seen any failures in the last few test runs due to this.

Definitely rock solid. Multiple test runs with zero issues.

Ok for now then, but if we'll see OOM-s after introducing new 32bit targets, this is a good place to get back.

Note that we were getting OOM-s not because of a specific test but rather because of reaching a critical mass of memory intentse-tests. I also pushed some Jpeg tests out-process together with the Bokeh ones back in the past. The price is execution time OFC. Testing image-processing is hard :)

Could have saved myself days of issues if i'd just thought of this first.

brianpopow

LGTM

JimBobSquarePants · 2020-10-15T20:20:09Z

@antonfirsov I'm gonna merge this as I'm happy with the result and have cleaned up anything questionable.

Update Dependencies

JimBobSquarePants added 2 commits October 12, 2020 23:06

Update test 3rd party libraries

6fed5f6

Update Test SDK.

feb4435

JimBobSquarePants added the area:build label Oct 12, 2020

JimBobSquarePants added 4 commits October 12, 2020 23:42

Tweak decode cancel timings

aeb1c1c

Bump src deps.

d51b725

Fix missing dll.config file

e94f879

The file only exists if there are binding redirects.

Update xunit runner

631b578

JimBobSquarePants added the unit tests label Oct 13, 2020

JimBobSquarePants added 4 commits October 13, 2020 18:04

Try System.Drawing BMP Decoder everywhere.

94ba508

Revert "Try System.Drawing BMP Decoder everywhere."

75cfa35

This reverts commit 94ba508.

Skip tests on Linux with known Magick issue

179563d

Add rule to coverlet settings.

bd59569

JimBobSquarePants changed the title ~~WIP Update Dependencies~~ Update Dependencies Oct 13, 2020

JimBobSquarePants requested a review from a team October 13, 2020 23:46

JimBobSquarePants commented Oct 13, 2020

View reviewed changes

JimBobSquarePants marked this pull request as ready for review October 13, 2020 23:48

JimBobSquarePants mentioned this pull request Oct 14, 2020

Upgraded Magick.NET. #1383

Merged

4 tasks

JimBobSquarePants added 5 commits October 14, 2020 16:54

Merge branch 'master' into js/update-dependencies

2a1587e

Remove ActiveIssue filters

1ea5e98

Merge branch 'master' into js/update-dependencies

ceb6dd5

Disable failing RemoteExecutor tests on NETFX.

c061218

Split out progressive jpeg decoding tests

abbf9e7

JimBobSquarePants commented Oct 14, 2020

View reviewed changes

JimBobSquarePants added 2 commits October 14, 2020 23:02

Skip limited buffer bitmap test on NETFX

e74c901

Add skip for dotnet xunit

65b1996

antonfirsov reviewed Oct 14, 2020

View reviewed changes

Don't use generics.

314e9f6

Could have saved myself days of issues if i'd just thought of this first.

JimBobSquarePants requested review from a team and antonfirsov October 15, 2020 11:36

brianpopow approved these changes Oct 15, 2020

View reviewed changes

JimBobSquarePants merged commit 4e61d87 into master Oct 15, 2020

JimBobSquarePants deleted the js/update-dependencies branch October 15, 2020 20:20

antonfirsov mentioned this pull request Oct 15, 2020

Extend 32bit test coverage #1386

Closed

JimBobSquarePants added a commit that referenced this pull request Mar 13, 2021

Merge pull request #1378 from SixLabors/js/update-dependencies

87f593a

Update Dependencies

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Dependencies #1378

Update Dependencies #1378

JimBobSquarePants commented Oct 12, 2020 •

edited

Loading

codecov bot commented Oct 13, 2020 •

edited

Loading

JimBobSquarePants Oct 13, 2020

JimBobSquarePants Oct 14, 2020

antonfirsov left a comment

antonfirsov Oct 14, 2020 •

edited

Loading

antonfirsov Oct 14, 2020 •

edited

Loading

JimBobSquarePants Oct 15, 2020

JimBobSquarePants Oct 15, 2020

antonfirsov Oct 14, 2020 •

edited

Loading

JimBobSquarePants Oct 15, 2020 •

edited

Loading

JimBobSquarePants Oct 15, 2020

antonfirsov Oct 14, 2020

antonfirsov Oct 14, 2020

antonfirsov Oct 14, 2020 •

edited

Loading

JimBobSquarePants Oct 15, 2020

JimBobSquarePants Oct 15, 2020

antonfirsov Oct 15, 2020

JimBobSquarePants Oct 15, 2020

brianpopow left a comment

JimBobSquarePants commented Oct 15, 2020

Update Dependencies #1378

Update Dependencies #1378

Conversation

JimBobSquarePants commented Oct 12, 2020 • edited Loading

Prerequisites

Description

codecov bot commented Oct 13, 2020 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antonfirsov left a comment

Choose a reason for hiding this comment

antonfirsov Oct 14, 2020 • edited Loading

Choose a reason for hiding this comment

antonfirsov Oct 14, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antonfirsov Oct 14, 2020 • edited Loading

Choose a reason for hiding this comment

JimBobSquarePants Oct 15, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antonfirsov Oct 14, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brianpopow left a comment

Choose a reason for hiding this comment

JimBobSquarePants commented Oct 15, 2020

JimBobSquarePants commented Oct 12, 2020 •

edited

Loading

codecov bot commented Oct 13, 2020 •

edited

Loading

antonfirsov Oct 14, 2020 •

edited

Loading

antonfirsov Oct 14, 2020 •

edited

Loading

antonfirsov Oct 14, 2020 •

edited

Loading

JimBobSquarePants Oct 15, 2020 •

edited

Loading

antonfirsov Oct 14, 2020 •

edited

Loading