[android_alarm_manager] Testing and documentation

[mklim]

Related Issues

Fixes flutter/flutter#44385.

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
• All existing and new tests are passing.
• I updated/added relevant documentation (doc comments with ///).
• The analyzer (flutter analyze) does not report any problems on my PR.
• I read and followed the Flutter Style Guide.
• The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
• I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
• I updated CHANGELOG.md to add a description of the change.
• I signed the CLA.
• I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

• Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
• No, this is not a breaking change.

[mklim]

I don't like exposing the internal state with @visibleForTesting, but I don't know how else to handle dealing with the native getCallbackHandle or the DateTime.now call. Same with mocking the MethodChannel.

[cyanglaz]

LGTM!

[bkonyi]

Will this appear in the docs for the plugin?

[mklim]

Yes. Technically this was a "missing doc" before since this API is public and visible to users of the plugin. I figured a warning saying not to touch this made the most sense as the only user-facing documentation on it.

[bkonyi]

I'm really not a fan of polluting the API surface with code only used for testing. If this wasn't going to appear in the docs that would be a different story, but it would be great if we could hide this somewhere.

[mklim]

Never mind, I'm going to delete this anyway as @cyanglaz suggested in #2283 (comment).

[mklim]

Sorry, Github hadn't loaded the earlier comment at the time I posted my last one.

Ideally I'd like to avoid all @visibleForTesting identifiers too but I'm not sure how to completely do it here. DateTime.now and PluginUtilities.getCallbackHandle are both static and referenced statically internally. AndroidAlarmManager itself is static so I can't use constructor injection. The unit tests can only touch the class through its public API surfaces. Right now I am cutting down on the visibility a little bit from what's currently in this patch, but I'm still adding this method to AndroidAlarmManager itself:

  /// This is exposed for the unit tests. It should not be accessed by users of
  /// the plugin.
  @visibleForTesting
  static void setTestOverides({Now now, GetCallbackHandle getCallbackHandle}) {
    _now = (now ?? _now);
    _getCallbackHandle = (getCallbackHandle ?? _getCallbackHandle);
  }

And then the class internally is using _now and _getCallbackHandle instead of the static functions.

If you have any ideas on how to avoid it totally I would be happy to hear it, I'd love to avoid this too. I'm also exposing the MethodChannel so I can test it too.

Edit: Strike the bit about the MethodChannel above, sorry. It looks like I don't actually need to access it directly if I just construct an identical instance in the tests and act on that.