BindParameter_WithTypeProperty_IsNotBound test looks bogus

[SteveSandersonMS]

As part of #6055, I fixed ModelBindingTestHelper.GetParameterBinder so that it now passes through the MvcOptions you give it into the parameter binder that it sets up. Previously it was just disregarding your options and in effect passing defaults.

Since doing this, the test BindParameter_WithTypeProperty_IsNotBound now fails. From what I can understand, this test is checking that with the default options, ExcludeBindingMetadataProvider prevents binding to properties of type System.Type. It does this check by registering a custom IModelBinderProvider that throws if it sees System.Type, so that the test would fail if a parameter if that type was bound.

However, it looks to me like the test was not really checking anything. Because ModelBindingTestHelper.GetParameterBinder was ignoring the options, this meant that the custom IModelBinderProvider was not really even registered. So the test could never fail, whether or not the product code was really working properly.

So either:

• The test implementation is wrong, and is failing falsely even though the product code is correct
• Or, the underlying product code is broken and the fixed test correctly detects this

In the meantime, I'm marking this test to be skipped so I can push #6055.

[SteveSandersonMS]

@sebastienros It looks like you'll have the most context on this since you implemented 2c639f8. Could you comment on what you think might be the issue here?

[sebastienros]

Will look at the test, in the meantime just to give some context, model binding would be utterly slow when a Type property would be on a view model because of the depth of its property graph. This perf issue has be fixed later, but the idea was to just be able to ignore specific types, and we also decided to add Type by default.

[rynowak]

This perf issue has be fixed later

I'm not sure about that 😆

I'd also mention that as much as possible the integration tests are trying to test the default configuration of MVC, with some shortcuts where a scenario would require extensibility to test.