Skip to content

Conversation

@antonfirsov
Copy link
Member

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

Just noticed there is an available virtual environment for macOS. I think we should add it to the list of CI builds.

Copy link
Member

@JimBobSquarePants JimBobSquarePants left a comment

Choose a reason for hiding this comment

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

Nice!

@antonfirsov
Copy link
Member Author

How can we add this to the list of Required checks?

@JimBobSquarePants
Copy link
Member

JimBobSquarePants commented Aug 3, 2020

In the branch protection settings. You should have access but if not, I’ll do it

Looks like we’re missing libgdiplus on the Mac OS. No idea how to add it

@antonfirsov
Copy link
Member Author

antonfirsov commented Aug 3, 2020

There are many tests failing because of over-threshold differences in the output, two resize tests reporting 18% and 25% difference which may indicate an actual bug.

macOS-Failure.log

@antonfirsov antonfirsov marked this pull request as draft August 3, 2020 17:31
@brianpopow
Copy link
Collaborator

In the branch protection settings. You should have access but if not, I’ll do it

Looks like we’re missing libgdiplus on the Mac OS. No idea how to add it

could this do the trick?

- name: Install macos dependencies
  if: matrix.os == 'macos-latest'
  run: brew install mono-libgdiplus

@antonfirsov
Copy link
Member Author

@brianpopow the condition did not evaluate to true somehow.

@brianpopow
Copy link
Collaborator

brianpopow commented Aug 3, 2020

@brianpopow the condition did not evaluate to true somehow.

@antonfirsov i cannot spot why that is, looks correct.

edit: maybe try if: startsWith(matrix.os, 'mac')

@antonfirsov
Copy link
Member Author

Looks like it has been moved to matrix.options.os .

@antonfirsov
Copy link
Member Author

antonfirsov commented Aug 3, 2020

29 test failures:

  • 3 test code compatibility issues
  • 2 of them likely indicating a macOS-only bug in padded resize
  • The rest: accuracy issues, mostly in the range of 0.1% - 0.9%. (Reminds me of our 32 bit Framework accuracy issues.)

@JimBobSquarePants
Copy link
Member

JimBobSquarePants commented Aug 3, 2020

The padded resize bugs are interesting. Not sure how that happens...

Transforms I’ll never be 100% happy with, could be the perfectionist in me but the edges never looked quite right to me. Not as crisp as imagemagick.

I’ll see if I can jump on my wife’s Mac and check the output to see if any are worth investigating

@antonfirsov
Copy link
Member Author

Doesn't make sense to keep around a stale PR, closing this in favor of #1387.

@JimBobSquarePants JimBobSquarePants deleted the af/macos-ci branch November 21, 2020 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants