All types failing test win10 vs2015 update 3

[ghost]

This one was weird. This test was failing.

Here is the stacktrace.

System.ArgumentException : Delegate given was not a method subscription delegate. Please use something similar to: 'publisher => publisher += null'. If you did, than it's probably a bug. Please use the other overload and specify name of the event as string.
at Castle.Facilities.EventWiring.EventWiringRegistrationExtensions.GetEventName[TPublisher](Action1 eventSubscribtion) in C:\code\Windsor\src\Castle.Facilities.EventWiring\EventWiringRegistrationExtensions.cs:line 84 at Castle.Facilities.EventWiring.EventWiringRegistrationExtensions.PublishEvent[TPublisher](ComponentRegistration registration, Action1 eventSubscribtion, Action`1 toSubscribers) in C:\code\Windsor\src\Castle.Facilities.EventWiring\EventWiringRegistrationExtensions.cs:line 41
at CastleTests.Facilities.EventWiring.FluentRegistrationTestCase.<>c.<Can_publish_events_via_AllTypes>b__2_0(ComponentRegistration r) in C:\code\Windsor\src\Castle.Windsor.Tests\Facilities\EventWiring\FluentRegistrationTestCase.cs:line 43
at Castle.MicroKernel.Registration.BasedOnDescriptor.TryRegister(Type type, IKernel kernel) in C:\code\Windsor\src\Castle.Windsor\MicroKernel\Registration\BasedOnDescriptor.cs:line 502
at Castle.MicroKernel.Registration.FromDescriptor.Castle.MicroKernel.Registration.IRegistration.Register(IKernelInternal kernel) in C:\code\Windsor\src\Castle.Windsor\MicroKernel\Registration\FromDescriptor.cs:line 184
at Castle.MicroKernel.Registration.BasedOnDescriptor.Castle.MicroKernel.Registration.IRegistration.Register(IKernelInternal kernel) in C:\code\Windsor\src\Castle.Windsor\MicroKernel\Registration\BasedOnDescriptor.cs:line 558
at Castle.MicroKernel.DefaultKernel.Register(IRegistration[] registrations) in C:\code\Windsor\src\Castle.Windsor\MicroKernel\DefaultKernel.cs:line 502
at Castle.Windsor.WindsorContainer.Register(IRegistration[] registrations) in C:\code\Windsor\src\Castle.Windsor\Windsor\WindsorContainer.cs:line 487
at CastleTests.Facilities.EventWiring.FluentRegistrationTestCase.Can_publish_events_via_AllTypes() in C:\code\Windsor\src\Castle.Windsor.Tests\Facilities\EventWiring\FluentRegistrationTestCase.cs:line 40

[kkozmic]

Ah yes. My guess is the bytecode that gets generated changed this new version.

[ghost]

That was my suspicion. The NaiveMethodNameExtractor was not allocating the calledMethod field because on line 71 the IsSupportedCode method was returning false for currentOpCode = '0x0003'. Extending LdArg support sorted it.

[kkozmic]

It was called native for a reason 😊

On Thu, 9 Feb 2017, 12:00 Gav ***@***.***> wrote: That was my suspicion. The NaiveMethodNameExtractor was not allocating the calledMethod field because on line 71 the IsSupportedCode method was returning false for currentOpCode = '0x0003'. Extending LdArg support sorted it. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#168 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAHMt6TKxLO6qpJGoolmofWWy-hAuV3dks5ramVIgaJpZM4L7jM3> .

-- sent from my phone Krzysztof

[jonorossi]

@Fir3pho3nixx looks good, I suspect this must have changed in .NET 4.6 because I don't think our build server has it installed.

Could you fix your indentation (Castle uses tabs), and we'll merge this.

[jonorossi]

@Fir3pho3nixx if your aren't aware your git commits aren't linking against your GitHub account because you are using your chambersandpartners.co.uk email address.

[ghost]

@jonorossi - Fixed email address, thank for letting me know. Also fixed up tabs.

[kkozmic-seek]

Totally shouldn't stop merging of this PR but I'd be interested to see how the IL for it changed, and how/why it now needs the first argument.

[ghost]

Got some IL instructions out of the test run using CIL.

IL: ldarg.1
IL: ldnull
IL: callvirt
IL: nop
IL: ret

More detailed output:

** ARGS **
ARG_NAME: p - ARG_TYPE: SimplePublisher
** LOCALS **
NIL
** IL BODY **
OP_CODE: ldarg.1 - OP_CD_NAME: ldarg.1 - OP_CD_TYPE: Macro - OP_CD_SIZE: 1 - OP_CD_VAL:'3'
OP_CODE: ldnull - OP_CD_NAME: ldnull - OP_CD_TYPE: Primitive - OP_CD_SIZE: 1 - OP_CD_VAL:'20'
OP_CODE: callvirt - OP_CD_NAME: callvirt - OP_CD_TYPE: Objmodel - OP_CD_SIZE: 1 - OP_CD_VAL:'111'
OP_CODE: nop - OP_CD_NAME: nop - OP_CD_TYPE: Primitive - OP_CD_SIZE: 1 - OP_CD_VAL:'0'
OP_CODE: ret - OP_CD_NAME: ret - OP_CD_TYPE: Primitive - OP_CD_SIZE: 1 - OP_CD_VAL:'42'

What do you think?

[ghost]

Very interesting that the test is the only one that also actually subscribes 2 events(Action<TPublisher> eventSubscribtion and Action<EventSubscribers> toSubscribers).

[Test]
public void Can_publish_events_via_AllTypes()
{
	container.Register(
		Classes.FromAssemblyContaining<SimpleListener>()
			.BasedOn<SimplePublisher>()
			.Configure(r => r.PublishEvent<SimplePublisher>(p => p.Event += null,
															x => x.To("foo"))),
		Component.For<ListenerWithOnEventMethod>().Named("foo"));

	var subscriber = container.Resolve<ListenerWithOnEventMethod>("foo");
	var publisher = container.Resolve<SimplePublisher>();

	publisher.Trigger();

	Assert.IsTrue(subscriber.Listened);
	Assert.AreSame(publisher, subscriber.Sender);
}

This is the only test in this fixture that subscribes to this overload. The rest don't. When I find usages on 'PublishEvent' in the test above I get "This is the only usage" message from Resharper.

Think we have found our smoking gun. Thoughts welcome guys.

[kkozmic]

I'm not surprised there aren't other tests for this overload. It's an infrequently used facility and the syntax for it, due to limitations of the language was never pretty.

I guess I'm just curious why it broke on this newer runtime/compiler if it worked previously. Clearly the IL generated must have changed. The IL you posted @Fir3pho3nixx makes perfect sense, to the degree that I'm not sure what else it could have looked like. In other words, how did it ever work?
I wanted to grab a copy of the test assembly from the build server (I don't have any Windows machine at the moment) but looks like there's an issue with your build server (http://builds.castleproject.org/) or I'm looking in the wrong place /cc @hammett

[jonorossi]

@kkozmic sorry about the build server, I've been powering them off while I wasn't using them, they are on now.

[ghost]

Would be handy to see builds in TeamCity. That way I know I have not broken anything.

[ghost]

@kkozmic - Should we merge the PR?

[kkozmic]

Sure. Go for it

On Tue, 14 Feb 2017, 09:15 Gav ***@***.***> wrote: @kkozmic <https://github.com/kkozmic> - Should we merge the PR? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#168 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAHMt7ilesfMDeVQAsYZ2CwfJgjkY-Ppks5rcNYFgaJpZM4L7jM3> .

-- sent from my phone Krzysztof

[jonorossi]

Should we merge the PR?

@Fir3pho3nixx I was just waiting for the changelog updates you made in #169 to be added here.

[ghost]

@jonorossi - Changes are in!

[jonorossi]

Thanks, merged.