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

Abstract class TestContext has different definitions for IDictionary properties between netframework and netstandard #474

Closed
passionatedevelopermsft opened this issue Aug 7, 2018 · 5 comments
Labels
Help-Wanted The issue is up-for-grabs, and can be claimed by commenting

Comments

@passionatedevelopermsft
Copy link

Description

The abstract class TestContext is not same between netframework and netstandard. This causes TypeLoaderReflection exceptions when an attempt is made to load an implementation of TestContext created targeting a different platform than the consuming service. More details in steps to reproduce -

Steps to reproduce

Create a dotnetstandard library that depends on MsTest.TestFramework. In here, create an implementation of TestContext, lets call it TestContextImpl . You will end up overriding IDictionary<string, object>. Create a nuget package for this library.

Create a new aspnet core application targeting netframework 4.6, add nuget dependency on the library created above. Attempt to hit sample controller and it will trigger MVC to load all types, including TestContextImpl which will blow up with a TypeLoadReflectionException because the abstract class definition is used from netframework 46 which does not know about IDictionary<string, object> properties but instead knows about IDictionary properties.

Expected behavior

No TypeLoadReflectionException should happen and the abstract class should be consistent across platform

Actual behavior

Unable to load an implementation of TestContext because abstract IDictionary was not available in the implementation based on new TestContext

Environment

Windows 10, MSTest version 1.2.1

@passionatedevelopermsft passionatedevelopermsft changed the title Abstract class TestContext has different implementations for IDictionary properties between netframework and netstandard Abstract class TestContext has different definitions for IDictionary properties between netframework and netstandard Aug 7, 2018
@jayaranigarg
Copy link
Member

@aakshayb : Acknowledging this to be a bug in framework. This seems like a simple fix. We would be happy to help you if you would like to take a shot at fixing this issue.

@jayaranigarg jayaranigarg added Type: Bug Help-Wanted The issue is up-for-grabs, and can be claimed by commenting labels Aug 30, 2018
@parrainc
Copy link
Contributor

parrainc commented Aug 30, 2018

I was about to open an issue related with this, but instead of the Properties prop, there are also a few methods in the TestContext class for netframework that are not available in netstandard version. In fact, already started working on migrating most of them to the netstandard implementation.

@jayaranigarg if @aakshayb have not started to work on this issue yet, i'd like to grab it. So I can merge the work I've done in my local branch with the changes required to fix this issue :)

Let me know if it is fine.

[EDIT]
First, this PR #411 should be merged into master, so we get a newer version of the netstandard, because the one used (1.0) is missing some functionalities. I partially fixed an issue #394 but I'm waiting for the PR to be merged :)

@passionatedevelopermsft
Copy link
Author

@parrainc Please feel free to address this in your PR. I have no bandwidth to fix this here at this time.
I worked around this problem in my own code base.
Thanks

@jayaranigarg
Copy link
Member

@parrainc : You are observing some methods missing from netstandard version of TestContext because they would make sense only when we implement [DeploymentItemAttribute] for netstandard projects. Since [DeploymentItemAttribute] is implemented for netframework, hence you will find more properties/methods there.

You can go ahead and pick this up. We will plan to push PR #411 soon to unblock this change. Please feel free to reach out to us if you need any help on this.

@jayaranigarg
Copy link
Member

Fixed as part of this PR : #563

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Help-Wanted The issue is up-for-grabs, and can be claimed by commenting
Projects
None yet
Development

No branches or pull requests

3 participants