ImageSharp icon indicating copy to clipboard operation
ImageSharp copied to clipboard

WIP: Run unit tests on the CI for ARM with docker/QEMU

Open brianpopow opened this issue 3 years ago • 12 comments

Prerequisites

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

Description

This PR demonstrates how to run the unit tests on the CI for ARM with docker/QEMU.

Note that the target branch is arm and not main. All ARM related changes will be considered experimental for a while and should be gathered there.

~This is just an Experiment with running the unit tests for ARM, please ignore~ ~did not know how to trigger the CI without a PR~

brianpopow avatar Feb 12 '22 19:02 brianpopow

I managed to create docker image with Magick.Native build for ARM. With that almost all test can be execute with docker/QEMU:

Failed!  - Failed:   139, Passed: 34955, Skipped:    18, Total: 35112, Duration: 28 m 31 s

There a a few failing, but those are all TIFF related and because imagemagick can not decode them for some reason. I must have done something wrong with compiling imagemagick.

 typeof(ImageMagick.MagickMissingDelegateErrorException): no decode delegate for this image format `' @ error/blob.c/BlobToImage/458
imagesharptests_1  | ---- ImageMagick.MagickMissingDelegateErrorException : no decode delegate for this image format `' @ error/blob.c/BlobToImage/458
imagesharptests_1  |   Stack Trace:
imagesharptests_1  |      at ImageMagick.MagickImageCollection.NativeMagickImageCollection.ReadBlob(IMagickSettings`1 settings, Byte[] data, Int32 offset, Int32 length) in /_/src/Magick.NET/Native/MagickImageCollection.cs:line 588
imagesharptests_1  |    at ImageMagick.MagickImageCollection.AddImages(Byte[] data, Int32 offset, Int32 count, IMagickReadSettings`1 readSettings, Boolean ping) in /_/src/Magick.NET/MagickImageCollection.cs:line 1493

It is pretty slow, a whole test run takes roughly 50 minutes.

brianpopow avatar Feb 14 '22 17:02 brianpopow

I had a working run going a few commits ago. What was wrong with that?

JimBobSquarePants avatar Feb 14 '22 19:02 JimBobSquarePants

I had a working run going a few commits ago. What was wrong with that?

It was not working. If you look at the log output (Build and test Step) all tests which relied on ImageMagick failed. The return value of docker-compose was not evaluated, that's why it looked like it did succeeded.

brianpopow avatar Feb 14 '22 19:02 brianpopow

Ah right, I hadn’t spotted that! 😂

JimBobSquarePants avatar Feb 14 '22 21:02 JimBobSquarePants

It seems to work now with docker/QEMU, all tests pass. It also works emulating hardware intrinsics, which surprised me.

If we consider going down this road for testing ARM, we need to consider a few things:

  • The first big issue: It takes alot of time, roughly 50 minutes for all tests. What we may can consider: Mark this CI step as manual to only execute when need. Another option I could think of: Mark tests which actually have ARM specific code with Trait ARM and only execute those.
  • We need to maintain a docker image for this this use case, which has Magick.Native prebuild for ARM. Since we do not change the Magick.Native reference often, maybe this is not a big deal, but at least something we need to keep in mind.

brianpopow avatar Feb 16 '22 09:02 brianpopow

Thanks for the update @brianpopow that's really great of you.

We need to maintain a docker image for this this use case, which has Magick.Native prebuild for ARM

I won't lie, I don't really understand this additional requirement. Did you have to build the binaries yourself? If so is there something we can help @dlemstra with upstream to save the requirement?

It looks like we may be able to filter the ARM job to run only when we use a GitHub label,though I don't know if the property on only available with that trigger. This could be pretty useful and would avoid us having to filter the actual tests (which could get messy) https://docs.github.com/en/actions/using-workflows/triggering-a-workflow#using-conditionals

JimBobSquarePants avatar Feb 16 '22 11:02 JimBobSquarePants

If so is there something we can help @dlemstra with upstream to save the requirement?

As far as I understand it, @dlemstra does not support building Magick.Native for ARM/linux since github actions does not officially support building for ARM.

I won't lie, I don't really understand this additional requirement. Did you have to build the binaries yourself?

Yes I did build Magick.Native from scratch on ARM. Luckily I do own a ARM device, but even with that it takes a long time to compile all dependencies. Took about 2 hours. I think it should be possible to build without a real device with docker/QEMU, but that would take even longer.

So the steps would be:

  • Build Magick.Native on ARM
  • Then from the build artifacts, a docker image needs to be created (i have used the official ARM dotnet sdk docker image as a base image).
  • Push the docker image to a docker registry, like dockerhub. From there the CI can download the docker image.

edit: If we really decide to go this QEMU/docker route, I could fork Magick.Native and provide the Dockerfile there to create the image.

brianpopow avatar Feb 16 '22 12:02 brianpopow

With the release of magick.net 11.0.0, it should work on ARM/linux. This will make things easier. We no longer need to build Magick.Native from scratch for ARM and can use the official dotnet docker image.

@JimBobSquarePants is there a way to trigger the CI on this branch manually? I have updated Magick.Net to 11.0.0 and wanted to see if it works.

brianpopow avatar Mar 26 '22 16:03 brianpopow

@brianpopow Maybe you could temporary add this branch to the branches in build-and-test.yml?

dlemstra avatar Mar 26 '22 18:03 dlemstra

@brianpopow Maybe you could temporary add this branch to the branches in build-and-test.yml?

yes this should work, thanks for the tip @dlemstra, I will give it a try.

brianpopow avatar Mar 26 '22 18:03 brianpopow

It seems to have worked with Magick.Net 11.0.0, so we dont need a special docker image and can use the official dotnet sdk docker image.

brianpopow avatar Mar 27 '22 11:03 brianpopow

HOORAY! This is AWESOME progress!!

JimBobSquarePants avatar Mar 27 '22 12:03 JimBobSquarePants

closing this in favor of #2341

brianpopow avatar Feb 03 '23 13:02 brianpopow