Fix SetSnapShot CopyTo variance failure

[fredericDelaporte]

This failure causes tests to fail when upgrading the NUnit engine.

[bahusoid]

ICollection.CopyTo implementation is broken by #2394
Shouldn't it be treated as 5.3 regression?

[hazzik]

I've added test for the incompatible array type. .NET 4.x and .NET Core behave differently. The .NET 4.x throws ArgumentException while .NET Core throws InvalidCastException. Not sure if we need/want to make this consistent.

[fredericDelaporte]

While technically a regression from 5.3.0, SetSnapShot is an internal technical class which NHibernate current internal uses do not suffer from the introduced bug. So, it appears there is no regression from an user standpoint. Only the NHibernate.Test project has an issue with this "regression", and only if upgrading our testing framework, which works on that internal class.

That is why targeting master seems enough to me. But why not targeting 5.3.x.

About:

I've added test for the incompatible array type. .NET 4.x and .NET Core behave differently. The .NET 4.x throws ArgumentException while .NET Core throws InvalidCastException. Not sure if we need/want to make this consistent.

I do not see a need for that, due to this class being internal and having its current use cases without a possibility of triggering these exceptions.

[hazzik]

I do not see a need for that

Sure. Feel free to drop my test then.

[bahusoid]

is an internal technical class which NHibernate current internal uses do not suffer from the introduced bug

Yeah but it's still exposed to user via IPersistentCollection.GetSnapshot call. I think it's worth fixing at least in 5.4.

[fredericDelaporte]

Since instances of that type can after all be accessed by user code through a public interface (which I had not spotted), re-targeted to 5.3.x, invalid array test adapted to accept the .Net Core case as it is currently.

[hazzik]

Closed and opened PR to fix wrong target on Generate Async code