Skip to content
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

ResolveAll is broken #17

Closed
brianlagunas opened this issue Nov 7, 2017 · 9 comments
Closed

ResolveAll is broken #17

brianlagunas opened this issue Nov 7, 2017 · 9 comments

Comments

@brianlagunas
Copy link

I believe you have introduced a breaking change in which the UnityCOntainerExtensions.ResolveAll does not call the correct method. See: https://github.com/unitycontainer/abstractions/blob/master/src/Utility/UnityContainerExtensions.cs#L519

The line should read
var result = (container ?? throw new ArgumentNullException(nameof(container))).ResolveAll((type ?? throw new ArgumentNullException(nameof(type

The key method missing being ResolveAll.

This was discovered when Prism upgraded to v5 and our unit tests started failing.

@brianlagunas
Copy link
Author

Actually, this looks like this is related to the interface of IUnityContainer changing, which seems to have broken Prism. Still investigating.

@ENikS
Copy link
Contributor

ENikS commented Nov 7, 2017

Could you be more specific: what is failing and under what circumstances.
The code you referring to is working fine in all existing tests. Granted these are the original tests and only cover 87% of code...

Could you give me a test case where it fails?

@brianlagunas
Copy link
Author

brianlagunas commented Nov 7, 2017

This test fails: https://github.com/PrismLibrary/Prism/blob/master/Source/Wpf/Prism.Unity.Wpf.Tests/UnityServiceLocatorAdapterFixture.cs#L34

This actually seems to be related to the fact that the IUnityContainer interface was changed. ResolveAll is no longer on the interface, which means the new extension method is being used instead of the interface method (which has been removed), hence breaking our tests.

Do you have a list of all the breaking changes you have made?

@ENikS
Copy link
Contributor

ENikS commented Nov 7, 2017

Looking at the coverage date I can see it is being tested properly: https://codecov.io/gh/unitycontainer/unity/src/master/src/Unity/Container/UnityContainerExtensions.cs

@ENikS
Copy link
Contributor

ENikS commented Nov 7, 2017

I listed all breaking changes in release notes every time there was a change.

@brianlagunas
Copy link
Author

brianlagunas commented Nov 7, 2017

@ENikS
Copy link
Contributor

ENikS commented Nov 7, 2017

I see the problem. Mocks will not work.

@ENikS
Copy link
Contributor

ENikS commented Nov 7, 2017

https://github.com/unitycontainer/unity/releases

Did not have time to update these everywhere

@brianlagunas
Copy link
Author

I see... I will have to find a way to update my tests to account for all the breaking changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants