TestMethodIdentifierProperty *TypeFullName is not nullable

[thomhurst]

When specifying TypeFullNames in the TestMethodIdentifier, it expects non-null type names.

However, Type.FullName is a nullable property.

I actually didn't know when it could be null, so asked co-pilot and was told:

Type.FullName can be null for certain types in .NET, such as:

• Generic type parameters (e.g., T in List)
• Array types of generic parameters
• Open generic types (e.g., typeof(List<>))
• Dynamic types or types created at runtime without a full name

This is by design in the .NET type system. Always check for null when using Type.FullName to avoid NullReferenceException

Given this is a possibility for tests (because users end up specifying params, who knows what they'll put in there.), should these not be nullable?

[Youssef1313]

I don't think TestMethodIdentifierProperty.TypeName should be nullable. For all cases that are potentially valid for testing, I think FullName is always going to be non-null. Do you have a specific use case where this is null? (note: the case of open generics pointed out by Copilot is unlikely to be correct)

[thomhurst]

The Test Type can probably stay not-nullable, as it'll always be a named type.

But copilot mentions dynamic, so I guess this?

    public async Task DynamicTest(dynamic value)
    {
        ...
    }

or

    public dynamic DynamicTest()
    {
        ...
    }

[thomhurst]

Also this?

    public async Task DynamicTest<T>(T value)
    {
        ...
    }

[Youssef1313]

dynamic should be treated as System.Object.

For generics, it's more complicated. Unfortunately, we are in a position today where TestMethodIdentifierProperty isn't well standardized. And, that's all because VSTest history and Test Explorer history that we had to carry over.

To give some history:

1. Test Explorer needs source information (file path and line number) so that when you double click a test it takes you to the location in source.
2. This used to be done by test frameworks/adapters (e.g, MSTest/xUnit/NUnit) by reading PDB files to find the source information.
3. This was very expensive, and had a big performance impact on very large repos, like dotnet/roslyn.
4. Test Explorer implemented a way to calculate source information by FullyQialifiedName, which is cheaper than reading PDBs at runtime. Then, Test Explorer would pass runsettings that has CollectSourceInformation set to false, to tell the framework/adapter "Don't do the expensive PDB processing, I know how to get source information myself" (this is C#/VB only, and as of today, isn't part of VSCode DevKit)
5. FullyQualifiedName of VSTest was already not standardized.
6. VS ended up having special casing for different frameworks to interpret FQN the way each framework does it.
7. VSTest attempted to standardize this, by introducing ManagedType/ManagedMethod. Spec.
8. Only MSTest ended up using this "standardized" approach. However, I personally don't like this spec, and I don't want it to leak into MTP.
9. TE today uses ManagedType/ManagedMethod if they exist (only MSTest today), otherwise, it falls back to FullyQualifiedName.
10. When MTP was implemented, Test Explorer was done in a way that assumes that location.type and location.method (which match what TestMethodIdentifierProperty provides here) are following the ManagedType/ManagedMethod spec.
11. Today's situation:
    • TUnit: AFAIK, always provides source info via TestFileLocationProperty (efficiently via source gen). That means TestMethodIdentifierProperty is not used for calculating source info.
    • MSTest: When using MTP, we don't follow the ManagedType/ManagedMethod spec. But so far, we didn't see user complains or broken scenarios. When using VSTest, we follow ManagedType/ManagedMethod spec.
    • NUnit: When using MTP, it doesn't create TestMethodIdentifierProperty at all. Because it uses a hacky "vstestProvider" capability we have in the bridge, VS is able to use the FQN logic. It's almost the same for VSTest, it also doesn't provide ManagedType/ManagedMethod and Test Explorer uses FQN.
    • xUnit: When using VSTest, it doesn't send ManagedType/ManagedMethod. So Test Explorer uses FQN. When using MTP:
      • MTP with current latest stable of xunit.v3 (2.0.3 at time of writing): TestMethodIdentifierProperty doesn't follow the spec. And source info via PDB processing is done only for F# and DevKit.
      • MTP with next release of xunit.v3 (3.x): It will start relying on caller info attributes to get the source information, so that it can always calculate source info correctly and efficiently.

My ultimate goal is to get to a position where TestMethodIdentifierProperty is never used for calculating source information, and getting all frameworks to provide source information efficiently, either via source gen (like TUnit), or via source info (like what xunit.v3 will be doing in future). For MSTest v4, we are tracking that in #5500.

IMPORTANT: Lots of the above are very old history before I joined Microsoft, so there might be stuff in "wrong" order, but the idea is the same.

Unfortunately, as of today, I don't know what "guidance" should we provide for TestMethodIdentifierProperty as it's not very standardized :/

[thomhurst]

Thanks for the explanation! Another thing it's used for is displaying the test explorer.

It uses the names/types/etc to determine what to show.

But if we manage to get momentum on the other issue, where Test Frameworks themselves control that (by some sort of new property? TestExplorerProperty(params string[] expandableSections) then would the method identifier property even be used for anything anymore? You might be able to kill it off.

[Youssef1313]

Yes, it's used to control the display if VSTest's "hierarchy" property isn't sent. But the lack of a standardization is mostly non-issue (or let's say very very small issue) when it comes to display.

TestExplorerProperty(params string[] expandableSections)

The signature on the current form will be problematic as Test Explorer cannot show arbitrary number of nodes. But we can do something with a fixed number of parameters (I think it's 4 but I need to double check). And I think it's really worth implementing this. I would even vote for really arbitrary trees generated by test frameworks, and not limit to a given number of levels. But.. Test Explorer is so tricky :(