New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make MessagingCenter testable #723

Merged
merged 3 commits into from Jan 31, 2017

Conversation

Projects
None yet
5 participants
@hartez
Member

hartez commented Jan 27, 2017

Description of Change

Proposing this as an alternative to PR 541. This change is designed to allow for easier testing of components which utilize MessagingCenter by introducing an interface (IMessagingCenter) which can be faked/mocked/substituted in automated tests (see the TestMessagingCenterSubstitute() method in MessagingCenterTests for an example).

This is essentially the same interface proposed in 541, but built on top of the changes from PR 617. Also, this version allows the continued use of the original MessagingCenter API without deprecation; the static methods on MessagingCenter simply call through to the new singleton instance, allowing the code to be shared rather than duplicated.

Bugs Fixed

API Changes

Added:

public interface IMessagingCenter
{
	void Send<TSender, TArgs>(TSender sender, string message, TArgs args) where TSender : class;
	void Send<TSender>(TSender sender, string message) where TSender : class;
	void Subscribe<TSender, TArgs>(object subscriber, string message, Action<TSender, TArgs> callback, TSender source = null) where TSender : class;
	void Subscribe<TSender>(object subscriber, string message, Action<TSender> callback, TSender source = null) where TSender : class;
	void Unsubscribe<TSender, TArgs>(object subscriber, string message) where TSender : class;
	void Unsubscribe<TSender>(object subscriber, string message) where TSender : class;
}

Behavioral Changes

None

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • Changes adhere to coding standard
  • Consolidate commits as makes sense

@hartez hartez referenced this pull request Jan 27, 2017

Closed

Deprecate MessagingCenter in favor of Messaging #541

3 of 4 tasks complete
@StephaneDelcroix

See my note, discuss, then 👍

Show outdated Hide outdated Xamarin.Forms.Core/MessagingCenter.cs
{
static readonly Lazy<MessagingCenter> s_instance = new Lazy<MessagingCenter>(() => new MessagingCenter());
public static IMessagingCenter Instance => s_instance.Value;

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Jan 27, 2017

Member

what's the point of the Lazy ? Allocations ? I don't think we gain anything measurable by instantiating a Lazy<MC> (and the Func) instead of a MessagingCenter in .cctor (static constructor), and at the end of the day, we have allocated one object more...

@StephaneDelcroix

StephaneDelcroix Jan 27, 2017

Member

what's the point of the Lazy ? Allocations ? I don't think we gain anything measurable by instantiating a Lazy<MC> (and the Func) instead of a MessagingCenter in .cctor (static constructor), and at the end of the day, we have allocated one object more...

This comment has been minimized.

@hartez

hartez Jan 27, 2017

Member

Now that I think about it more, there really is no point. Initializing it in the static constructor is just as thread-safe and there's no startup performance gain because Core uses it for several features, so it always gets created when you start the application no matter what.

I'll update it to remove the Lazy initialization.

@hartez

hartez Jan 27, 2017

Member

Now that I think about it more, there really is no point. Initializing it in the static constructor is just as thread-safe and there's no startup performance gain because Core uses it for several features, so it always gets created when you start the application no matter what.

I'll update it to remove the Lazy initialization.

hartez added some commits Jan 27, 2017

@rmarinho rmarinho merged commit b4fe4e0 into master Jan 31, 2017

6 checks passed

Android-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: UI Tests :: OSX Test Cloud Package - Run Android 6.0.1 : Tests passed: 352, i…
Details
OSX-Debug-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: OSX Debug : Running
Details
Windows-Debug-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: Windows Debug : Tests passed: 3718, ignored: 10
Details
iOS10-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified iOS10 : Tests passed: 34…
Details
iOS8-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified IOS8 : Tests passed: 348…
Details
iOS9-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified iOS9 : Tests passed: 350…
Details

@hartez hartez deleted the fix-bugzilla46601 branch May 16, 2017

@samhouts samhouts added D-15.4 and removed cla-not-required labels Oct 10, 2017

@samhouts samhouts modified the milestones: 2.3.0, 2.3.5 Jun 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment