[local_auth_darwin] MacOS Support

[alexrabin-sentracam]

Adds macOS support for local_auth_darwin

Cancelled Example:

Success Example

Error Example

List which issues are fixed by this PR. You must list at least one issue.

https://github.com/flutter/flutter/issues/140685

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

Pre-launch Checklist

• [X] I read the Contributor Guide and followed the process outlined there for submitting PRs.
• [X] I read the Tree Hygiene wiki page, which explains my responsibilities.
• [X] I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
• [X] I signed the CLA.
• [X] The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
• [X] I linked to at least one issue that this PR fixes in the description above.
• [X] I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
• [X] I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
• [X] I updated/added relevant documentation (doc comments with ///).
• [X] I added new tests to check the change I am making, or this PR is test-exempt.
• [X] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[google-cla[bot]]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[stuartmorgan-g]

Thanks for the contribution!

This appears to be very similar to https://github.com/flutter/packages/pull/5766; does this need to be a new PR rather than leaving comments on that one about any suggested changes?

[alexrabin-sentracam]

@stuartmorgan That pr doesn't seem to have any momentum and is outdated so I figured I would take this on.

[alexrabin-sentracam]

@stuartmorgan I have addressed all your comments

[alexrabin-sentracam]

Based on the CI failure message I suspect you didn't add any source files to the RunnerTest target's build.

I had to remove them since the CI said there were duplicate tests from the iOS project

[stuartmorgan-g]

Based on the CI failure message I suspect you didn't add any source files to the RunnerTest target's build.

I had to remove them since the CI said there were duplicate tests from the iOS project

This won't pass CI without native unit tests. You'll need to provide an exact error message you were getting from having them, since that description doesn't make sense to me. We have this pattern in a number of other plugins.

[alexrabin-sentracam]

This won't pass CI without native unit tests. You'll need to provide an exact error message you were getting from having them, since that description doesn't make sense to me. We have this pattern in a number of other plugins.

Okay I'll put back the tests then

[alexrabin-sentracam]

@stuartmorgan I found this reference for all the url schemes that Apple has within the settings app: https://www.mbsplugins.de/archive/2020-04-05/MacOS_System_Preference_Links. If theses aren't up to par with you then what do you suggest we do for the settings button?

[stuartmorgan-g]

@stuartmorgan I found this reference for all the url schemes that Apple has within the settings app: https://www.mbsplugins.de/archive/2020-04-05/MacOS_System_Preference_Links.

That's not official documentation from Apple, so it doesn't address my earlier comments.

If theses aren't up to par with you then what do you suggest we do for the settings button?

If you are not able to find a supported way to open settings programmatically, then I wouldn't attempt to provide a button that claims that it will do that, and would instead provide a high-level explanation of what the user needs to do.

[stuartmorgan-g]

It still looks from the error message like the RunnerTests target is empty. Are you sure the file is added to that target? Are you able to run those tests in Xcode locally?

[alexrabin-sentracam]

@stuartmorgan I found an article talking about how to find all the available app link URL's that Apple Supports and the plist keys that are needed to support them: https://www.linkedin.com/pulse/creating-web-links-launch-mac-apps-preferences-script-nick-tong

sudo grep -A1 -r "NSPrefPaneAllowsXAppleSystemPreferencesURLScheme" /System/Library/PreferencePanes

<key>NSPrefPaneAllowsXAppleSystemPreferencesURLScheme</key>
<true/>

I updated the Readme accordingly.

I also added the Tests for macOS.

[stuartmorgan-g]

@stuartmorgan I found an article talking about how to find all the available app link URL's that Apple Supports and the plist keys that are needed to support them: https://www.linkedin.com/pulse/creating-web-links-launch-mac-apps-preferences-script-nick-tong

Please read my comments above; a random person posting an article on LinkedIn is not the same as official documentation from Apple.

I updated the Readme accordingly.

Adding an undocumented key that is used for preference panes to an application is unlikely to do anything, so is not something we would instruct people to do.

I also added the Tests for macOS.

Based on the CI error, your change to the project appears to have broken the ability to build the example app.

[alexrabin-sentracam]

@stuartmorgan I found an article talking about how to find all the available app link URL's that Apple Supports and the plist keys that are needed to support them: https://www.linkedin.com/pulse/creating-web-links-launch-mac-apps-preferences-script-nick-tong

Please read my comments above; a random person posting an article on LinkedIn is not the same as official documentation from Apple.

I updated the Readme accordingly.

Adding an undocumented key that is used for preference panes to an application is unlikely to do anything, so is not something we would instruct people to do.

Okay I am going to remove the settings button in the pop up and just update the messaging to tell the user to go to the settings app manually.

[alexrabin-sentracam]

@stuartmorgan @LouiseHsu is there anything else needed before this pr can be merged?

[stuartmorgan-g]

Please see https://github.com/flutter/flutter/wiki/Tree-hygiene#tldr for expected review times. It's been less than a full business day since you posted the last version.

[alexrabin-sentracam]

@stuartmorgan sorry this is my first pr into flutter packages so I didn't know. Thanks for all your help.

[jmagman]

@LouiseHsu could you take a look at this one?

[stuartmorgan-g]

Is this ready for re-review? There are still unaddressed comments and failing tests, so I assumed it was a work-in-progress.

[alexrabin-sentracam]

I could use some assistance on the comments @stuartmorgan left me. I am not seeing a clean way to use the Platform.isMacos and still have the dart test cases pass.

I am also not well versed in testing the alert construction as you requested.

[stuartmorgan-g]

I am not seeing a clean way to use the Platform.isMacos and still have the dart test cases pass.

Per my earlier comment, you can wrap those calls in something that can have an alternate implementation injected for unit tests, using standard dependency injection patterns.

[alexrabin-sentracam]

I am not seeing a clean way to use the Platform.isMacos and still have the dart test cases pass.

Per my earlier comment, you can wrap those calls in something that can have an alternate implementation injected for unit tests, using standard dependency injection patterns.

All fixed. I addressed all your comments @stuartmorgan . This pr is ready for re-review.

[stuartmorgan-g]

@alexrabin-sentracam I'll re-review when I have time (I currently have some very high priority work that will delay it); it's not necessary to constantly merge the branch in the meantime.

[alexrabin-sentracam]

@stuartmorgan Okay sounds good. Thank you.

[jmagman]

@alexrabin-sentracam looks like there's some merge conflicts, let us know when this is ready to re-review!

[jmagman]

@LouiseHsu would you mind taking a look at this?

[stuartmorgan-g]

Is this actually ready for re-review? There are still open comments from the last review, and it's failing several CI checks, so I was assuming it was not yet ready.

[alexrabin-sentracam]

@stuartmorgan I am seeing this error in the LUCI after the tests were converted to swift.

xcodebuild: error: Failed to build workspace Runner with scheme Runner.: Cannot test target “RunnerTests” on “My Mac”: My Mac’s macOS 13.6.1 doesn’t match RunnerTests’s macOS 14.0 deployment target. https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8746482769422894929/+/u/Run_package_tests/native_test/stdout?format=raw

Do you have any recommendations for what I should do?

[stuartmorgan-g]

@stuartmorgan I am seeing this error in the LUCI after the tests were converted to swift.

Why do you believe the error is related to the tests being converted to Swift? You changed the MACOSX_DEPLOYMENT_TARGET in this commit.

[LouiseHsu]

@alexrabin-sentracam do you need some help with the OCMock bit?

[alexrabin-sentracam]

@alexrabin-sentracam do you need some help with the OCMock bit?

@LouiseHsu Yes please, I'd appreciate that