Skip to content

Running tests that depend on string[] TypeDescriptor converters fail. #427

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

Closed
NTaylorMullen opened this issue Feb 1, 2017 · 2 comments
Closed

Comments

@NTaylorMullen
Copy link

NTaylorMullen commented Feb 1, 2017

Description

In MVC there's a ModelMetadata system that inspects types and tracks certain information about said types. One of the things it tracks is IsComplexType. It computes this by doing: !TypeDescriptor.GetConverter(ModelType).CanConvertFrom(typeof(string));, code for ref. This works for most scenarios but when ModelType is string[] IsComplexType ends up returning false when it should be true.

Looking through the vstest code it looks like it's due to how you guys register your converter for string arrays. The convert is a little too simple to handle string arrays properly.

This breaks the following MVC tests:

Working around this for now by skipping the test in core. This somehow works in net451 tests because the GetConverter call returns an array converter instead of the above mentioned CustomStringArrayConverter.

Steps to reproduce

Expected behavior

  • Tests pass

Actual behavior

  • Tests fail

Environment

.NET Command Line Tools (1.0.0-rc4-004616)

Product Information:
 Version:            1.0.0-rc4-004616
 Commit SHA-1 hash:  56d8071361

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.14393
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Users\nimullen\AppData\Local\Microsoft\dotnet\sdk\1.0.0-rc4-004616
@codito
Copy link
Contributor

codito commented Feb 14, 2017

Triage: this issue is a core part of test platform, but there isn't a workaround. We should investigate the fix for this (may be for RTM).

@codito
Copy link
Contributor

codito commented Feb 22, 2017

Fixed with b78eafa in master, bd04f21 in 15.0-rtm.

dougbu added a commit to aspnet/Mvc that referenced this issue Feb 23, 2017
…s from dev

- manually recreate *.sln files to correct VS versions and use old project GUIDs
  - add missing lines to the .sln files; build configuration likely not used before
- manually migrate RazorPagesWebSite project
- remove `ToolsVersion` attributes
- add repo.props file; never test TestCommon project and only build Mvc.sln
- add dependencies.props file to match other repos

Test-related:
- add `$(PreserveCompilationContext)` to test projects that examine their own dependencies
- remove comments from xunit.runner.json files; xUnit does not support them
- remove .notest files
- work around inability to deserialize a odd `ref` type
  - xUnit and vstest now serialize / deserialze test data more often
- work around microsoft/vstest#419 instead of skipping test
- enable testes that hit microsoft/vstest#427; bug has been fixed
- add `<Service>`s to test projects
dougbu added a commit to aspnet/Mvc that referenced this issue Feb 26, 2017
…s from dev

- manually recreate *.sln files to correct VS versions and use old project GUIDs
  - add missing lines to the .sln files; build configuration likely not used before
- manually migrate RazorPagesWebSite project
- remove `ToolsVersion` attributes
- add repo.props file; never test TestCommon project and only build Mvc.sln
- add dependencies.props file to match other repos

Test-related:
- add `$(PreserveCompilationContext)` to test projects that examine their own dependencies
- remove comments from xunit.runner.json files; xUnit does not support them
- remove .notest files
- work around inability to deserialize a odd `ref` type
  - xUnit and vstest now serialize / deserialze test data more often
- work around microsoft/vstest#419 instead of skipping test
- enable testes that hit microsoft/vstest#427; bug has been fixed
- add `<Service>`s to test projects
dougbu added a commit to aspnet/Mvc that referenced this issue Feb 27, 2017
…s from dev

- manually recreate *.sln files to correct VS versions and use old project GUIDs
  - add missing lines to the .sln files; build configuration likely not used before
- manually migrate RazorPagesWebSite project
- remove `ToolsVersion` attributes
- add repo.props file; never test TestCommon project and only build Mvc.sln
- remove aspnet/KoreBuild#182 workaround; bug fixed
- remove `Microsoft.DotNet.InternalAbstractions` and `System.Xml.XmlDocument` dependencies
- stop floating `$(CoreFxVersion)` and `Microsoft.Extensions.DependencyModel` dependencies
- add dependencies.props file to match other repos

Test-related:
- re-enable .NET Framework run of the functional tests
  - disable shadow copying
- make Microsoft.AspNetCore.Mvc.TestDiagnosticListener a regular class library
- add support for `/p:GenerateBaselines=true` for functional and Razor.Host tests
- add `$(PreserveCompilationContext)` to test projects that examine their own dependencies
- remove `$(RuntimeIdentifier)` settings
- remove comments from xunit.runner.json files; xUnit does not support them
- remove .notest files
- separate `GetCSharpTypeName_ReturnsCorrectTypeNames_ForOutParameter()` test
- work around inability to deserialize a odd `ref` type
  - xUnit and vstest now serialize / deserialze test data more often
- work around microsoft/vstest#419 instead of skipping test
- enable tests that hit microsoft/vstest#427; bug has been fixed
- add `<Service>`s to test projects

nits:
- remove all web.config files
- remove conditional compilation from class libraries
- remove unnecessary project properties from `UserClassLibrary`
- move a few properties to the top of `RazorWebSite` .csproj file
- remove some trailing whitespace in .csproj files
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants