Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Cleanup services #2034

Closed
wants to merge 1 commit into from
Closed

Cleanup services #2034

wants to merge 1 commit into from

Conversation

yishaigalatzer
Copy link
Contributor

  1. Make explicit registration for nested provider manager (solves a problem with ninject for default MV MvcServices.GetDefaultServices breaks Ninject #953)
  2. Remove registation for async nested provider manager
  3. Make Action selector a singleton (should assist in perf), and cleanup related interface to allow that to happen
  4. Fix up the related tests

@ghost ghost added the cla-not-required label Feb 18, 2015
@@ -860,7 +861,7 @@ private static RouteContext CreateRouteContext(string httpMethod)
var routeData = new RouteData();
routeData.Routers.Add(new Mock<IRouter>(MockBehavior.Strict).Object);

var httpContext = new Mock<HttpContext>(MockBehavior.Strict);
var httpContext = new Mock<HttpContext>(MockBehavior.Loose);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes me nervous. Can you just set the properties that are being used in the test (assuming it's the service provider).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes a big messy to the tests, I'm not sure how important this is, lets talk in person.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are going to make it strict, and pass services, as well as remove setting services on HttpContext from the test above

@rynowak
Copy link
Member

rynowak commented Feb 19, 2015

:shipit: after looking at changing mock back to strict

@yishaigalatzer yishaigalatzer deleted the CleanupServices branch February 24, 2015 04:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants