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

Test methods in a base class defined in a different assembly are not navigable in Test Explorer #163

Closed
AbhitejJohn opened this issue Apr 23, 2017 · 13 comments
Labels
Help-Wanted The issue is up-for-grabs, and can be claimed by commenting Type: Bug

Comments

@AbhitejJohn
Copy link
Contributor

AbhitejJohn commented Apr 23, 2017

Pulled this out of #23.

Description

After turning on EnableBaseClassTestMethodsFromOtherAssemblies fixed in #23, new tests defined in the base class start appearing in the Test Explorer. These tests show up under the External node in Test Explorer instead of the derived project. This is happening because the DIA source information logic is searching for these methods in the derived assembly and cannot find it.

Steps to reproduce

Add the following runsettings

<RunSettings>    
  <MSTest> 
    <EnableBaseClassTestMethodsFromOtherAssemblies>true</EnableBaseClassTestMethodsFromOtherAssemblies> 
  </MSTest> 
</RunSettings>
  1. Create two test projects - one with a BaseTest class and another with a DerivedTest class with the DerivedTest extending BaseTest.
  2. Add test methods in both the classes.
  3. Discover tests from the project containing DerivedTest.
  4. Double click the BaseTest tests.

Expected behavior

The user should be taken to the tests definition.

Actual behavior

Nothing happens.

@AbhitejJohn AbhitejJohn added Type: Bug Help-Wanted The issue is up-for-grabs, and can be claimed by commenting labels Apr 23, 2017
@AbhitejJohn
Copy link
Contributor Author

From @AbhitejJohn

@ajryan , do you want to give a shot at fixing that? It is to do with the assembly full path we pass in here. If we pass in the (base)assembly where the test method is actually defined this might be fixed.

@AbhitejJohn
Copy link
Contributor Author

From @ajryan

sure, I'll take a look and come back with questions

@AbhitejJohn
Copy link
Contributor Author

From @AbhitejJohn

Here is little more context on how the 2nd part works: This code gets the line number and file name where a test is defined based on the source. We use DIA internally to populate this data(DIA in-turn works on pdb files). In our case, we need to be searching for the base test methods in the base assembly and not in the derived test assembly. After we make a change to set the appropriate source on a test element as I detailed above, we would also need to change this code to pass in the source from the test element so it gets the right data. Hope that makes sense.

@AbhitejJohn
Copy link
Contributor Author

From @ajryan

Thanks for the pointers, this makes sense. I think TestMethod should carry a DeclaringAssemblyName parallel to the way it exposes DeclaringClassFullName. I'll thread it through from the discovery end and then make sure the navigator can locate the source correctly.

@AbhitejJohn
Copy link
Contributor Author

From @AbhitejJohn

@ajryan: Yes, that should be great. There could be issues in figuring out the location of the assembly though, MSTest.CoreAdapter being a PCL. We might need to pass it through another API in IFileOperations.

@AbhitejJohn
Copy link
Contributor Author

From @ajryan

About determining assembly location from TypeEnumerator in the PCL... I now understand your comment about about farming it out to PlatformServices/IFileOperations, I came to the same conclusion while I was working it through.
For the concrete implementations:
The Desktop flavor has Assembly.Location available.
For NetCore, it's currently targeting NetStandard1.3 -- if we can move up to NetStandard1.5, it will be available there. Not sure what the restrictions are.
For UWP, it's simply not an option. Seems like this is OK since navigation session is not a thing here.
I am doing exploratory testing locally in VS2017 with nuget packages built with my changes. I think things are looking good with the base methods appearing under the derived class assembly, meanwhile navigating to source of the base methods locates the correct file+line.
One thing I noticed, somewhat related to the ClassInitialize discussion -- when executing an inherited remote assembly base method: ClassInitialize is called correctly on the base assembly class and a TestContext is passed, but WriteLine calls to that context are not captured as output of the base method.
You can see my progress so far here: ajryan/testfx@5c0d2b6

@AbhitejJohn
Copy link
Contributor Author

From @AbhitejJohn

That sounds great @ajryan . Looked through your changes as well - looks good.
if we can move up to NetStandard1.5, it will be available there.
We can surely move over to ns1.5. We just wanted to leave it at a minimal standard as a principle.
UWP unfortunately does not seem to have an API to get file path(By design). We can leave it as is.
ClassInitialize is called correctly on the base assembly class and a TestContext is passed, but WriteLine calls to that context are not captured as output of the base method.
Oh I see, this would be coming out in one of the test methods of the derived class. Will file an issue for this one.

@ajryan
Copy link
Contributor

ajryan commented Apr 30, 2017

Sorry for going dark on this. Pushing PlatformServices.NetCore up to NetStandard1.5 makes it incompatible with its NuGet dependency Microsoft.TestPlatform.ObjectModel. v11 What are the implications of PlatformService.NetCore moving up to Microsoft.TestPlatform.Objectmodel v15? The alternative is only having the PlatformServices.Desktop flavor be able to correctly navigate to other-assembly base class methods.

@AbhitejJohn
Copy link
Contributor Author

@ajryan: It looks like the new PlatformServices.NetCore.csproj has a conditional target fallback. Please remove the condition element in the below

<PackageTargetFallback Condition=" '$(TargetFramework)' == 'netstandard1.3' ">$(PackageTargetFallback);portable-net45+win8+wpa81+wp8</PackageTargetFallback>

so it reads like below

<PackageTargetFallback>$(PackageTargetFallback);portable-net45+win8+wpa81+wp8</PackageTargetFallback>

@ajryan
Copy link
Contributor

ajryan commented May 2, 2017

Ah, that did it! Wow, never heard of PackageTargetFallback before.

Just adding some tests.

@AbhitejJohn
Copy link
Contributor Author

This has been fixed in the 1.1.18 release of the Framework/Adapter pair. Here are the release notes.

@paulmccormack
Copy link

I am still unable to get this feature to work even after adding the setting to the runsettings file. This is in Visual Studio 2017. I am using MS Test v2 v 1.4.0

Is any other setup needed?

singhsarab pushed a commit to singhsarab/testfx that referenced this issue Apr 8, 2019
@jayaranigarg
Copy link
Member

@paulmccormack : We request you not to raise concerns on closed issues, we don't monitor them. Please create a new issue with details if you need our assistance.

@microsoft microsoft locked and limited conversation to collaborators Apr 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Help-Wanted The issue is up-for-grabs, and can be claimed by commenting Type: Bug
Projects
None yet
Development

No branches or pull requests

4 participants