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.

Sign up for GitHub

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

Jump to bottom

Add non-strict search flag for legacy unit test providers #64375

Merged
merged 2 commits into from
Sep 30, 2022
Merged

Add non-strict search flag for legacy unit test providers #64375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8fe6702
Add non-strict search flag for legacy unit test providers
CyrusNajmabadi Sep 29, 2022
730bcf0
Add docs
CyrusNajmabadi Sep 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions src/EditorFeatures/Test2/UnitTesting/UnitTestingSearchHelpersTests_CSharp.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,14 @@ class Outer<T>
}", UnitTestingSearchQuery.ForType("Outer"), host)
End Function

<Theory, CombinatorialData>
Public Async Function CS_TestGenericType2_NonStrict(host As TestHost) As Task
Await TestCSharp("
class [|Outer|]<T>
{
}", UnitTestingSearchQuery.ForType("Outer", strict:=False), host)
End Function

<Theory, CombinatorialData>
Public Async Function CS_TestGenericType3(host As TestHost) As Task
Await TestCSharp("
Expand All @@ -48,6 +56,14 @@ class Outer<T>
}", UnitTestingSearchQuery.ForType("Outer`2"), host)
End Function

<Theory, CombinatorialData>
Public Async Function CS_TestGenericType3_NonStrict(host As TestHost) As Task
Await TestCSharp("
class [|Outer|]<T>
{
}", UnitTestingSearchQuery.ForType("Outer`2", strict:=False), host)
End Function

<Theory, CombinatorialData>
Public Async Function CS_TestNestedType1(host As TestHost) As Task
Await TestCSharp("
Expand Down Expand Up @@ -168,6 +184,15 @@ class Outer
}", UnitTestingSearchQuery.ForMethod("Outer", "Goo", methodArity:=1, methodParameterCount:=0), host)
End Function

<Theory, CombinatorialData>
Public Async Function CS_TestMethod2_NonStrict(host As TestHost) As Task
Await TestCSharp("
class Outer
{
void [|Goo|]() { }
}", UnitTestingSearchQuery.ForMethod("Outer", "Goo", methodArity:=1, methodParameterCount:=0, strict:=False), host)
End Function

<Theory, CombinatorialData>
Public Async Function CS_TestMethod3(host As TestHost) As Task
Await TestCSharp("
Expand All @@ -177,6 +202,15 @@ class Outer
}", UnitTestingSearchQuery.ForMethod("Outer", "Goo", methodArity:=0, methodParameterCount:=1), host)
End Function

<Theory, CombinatorialData>
Public Async Function CS_TestMethod3_NonStrict(host As TestHost) As Task
Await TestCSharp("
class Outer
{
void [|Goo|]() { }
}", UnitTestingSearchQuery.ForMethod("Outer", "Goo", methodArity:=0, methodParameterCount:=1, strict:=False), host)
End Function

<Theory, CombinatorialData>
Public Async Function CS_TestMethod4(host As TestHost) As Task
Await TestCSharp("
Expand Down
48 changes: 27 additions & 21 deletions src/Features/Core/Portable/ExternalAccess/UnitTesting/API/UnitTestingSearchHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// See the LICENSE file in the project root for more information.

using System.Collections.Immutable;
using System.Dynamic;
using System.Globalization;
using System.Linq;
using System.Threading;
Expand Down Expand Up @@ -138,31 +139,36 @@ private static async Task<ImmutableArray<UnitTestingDocumentSpan>> GetSourceLoca
var index = await TopLevelSyntaxTreeIndex.GetRequiredIndexAsync(document, cancellationToken).ConfigureAwait(false);
foreach (var info in index.DeclaredSymbolInfos)
{
// Fast check to see if this looks like a candidate.
if (info.TypeParameterCount == symbolArity &&
info.Name == symbolName)
// Fast checks to see if this looks like a candidate.

// In non-strict mode, allow the type-parameter count to be mismatched.
if (query.Strict && info.TypeParameterCount != symbolArity)
continue;

if (info.Name != symbolName)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For VB the name could theoretically be different case. Is it possible to take that into account here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually never-mind, we'll probably require that the supplied name should match the casing present in the code in such cases. But one potential option may be to allow different casing for VB when strict is false.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that seems like it would be very odd for the name to not match. but sure, i can add that :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup doing that with strict=true would certainly be odd. But it would be nice to find closest match with strict=false :)

Note that I am only proposing with behavior when strict=off and also only when the language is VB :)

continue;

// If it's a method, the parameter count much match.
if (query.MethodName != null)
{
// If it's a method, the parameter count much match.
if (query.MethodName != null)
{
if (info.Kind is not (DeclaredSymbolInfoKind.Method or DeclaredSymbolInfoKind.ExtensionMethod) ||
info.ParameterCount != query.MethodParameterCount)
{
continue;
}
}

// Looking promising so far. Check that the container matches what the caller needs.
if (info.FullyQualifiedContainerName != container)
if (info.Kind is not (DeclaredSymbolInfoKind.Method or DeclaredSymbolInfoKind.ExtensionMethod))
continue;

// Unit testing needs to know the final span a location may be mapped to (e.g. with `#line` taken
// into consideration). So map that information here for them.
tree ??= await document.GetRequiredSyntaxTreeAsync(cancellationToken).ConfigureAwait(false);
var mappedSpan = tree.GetMappedLineSpan(info.Span, cancellationToken);

result.Add(new UnitTestingDocumentSpan(new DocumentSpan(document, info.Span), mappedSpan));
// In non-strict mode, allow the parameter count to be mismatched.
if (query.Strict && info.ParameterCount != query.MethodParameterCount)
continue;
}

// Looking promising so far. Check that the container matches what the caller needs.
if (info.FullyQualifiedContainerName != container)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question about case sensitivity for VB

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup. i'll find a way to handle that.

continue;

// Unit testing needs to know the final span a location may be mapped to (e.g. with `#line` taken
// into consideration). So map that information here for them.
tree ??= await document.GetRequiredSyntaxTreeAsync(cancellationToken).ConfigureAwait(false);
var mappedSpan = tree.GetMappedLineSpan(info.Span, cancellationToken);

result.Add(new UnitTestingDocumentSpan(new DocumentSpan(document, info.Span), mappedSpan));
}

return result.ToImmutable();
Expand Down
20 changes: 15 additions & 5 deletions src/Features/Core/Portable/ExternalAccess/UnitTesting/API/UnitTestingSearchQuery.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,18 +35,28 @@ internal sealed class UnitTestingSearchQuery
[DataMember(Order = 3)]
public readonly int MethodParameterCount;

public static UnitTestingSearchQuery ForType(string fullyQualifiedTypeName)
=> new(fullyQualifiedTypeName, methodName: null, methodArity: 0, methodParameterCount: 0);
/// <summary>
/// Whether or not this is a strict search or not. Strict searches require matching arity and parameter counts,
/// while non-strict searches do not. Non-strict searches are useful in cases where the initial searching data
/// may not be produced in a well formed fashion (for example, some legacy test providers that do not follow:
/// https://github.com/microsoft/vstest-docs/blob/main/RFCs/0017-Managed-TestCase-Properties.md).
/// </summary>
[DataMember(Order = 4)]
public readonly bool Strict;

public static UnitTestingSearchQuery ForType(string fullyQualifiedTypeName, bool strict = true)
=> new(fullyQualifiedTypeName, methodName: null, methodArity: 0, methodParameterCount: 0, strict);

public static UnitTestingSearchQuery ForMethod(string fullyQualifiedTypeName, string methodName, int methodArity, int methodParameterCount)
=> new(fullyQualifiedTypeName, methodName, methodArity, methodParameterCount);
public static UnitTestingSearchQuery ForMethod(string fullyQualifiedTypeName, string methodName, int methodArity, int methodParameterCount, bool strict = true)
=> new(fullyQualifiedTypeName, methodName, methodArity, methodParameterCount, strict);

private UnitTestingSearchQuery(string fullyQualifiedTypeName, string? methodName, int methodArity, int methodParameterCount)
private UnitTestingSearchQuery(string fullyQualifiedTypeName, string? methodName, int methodArity, int methodParameterCount, bool strict)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One other thing we care about is presence of attributes on the method. Is it possible to introduce a flag to filter out methods that don't have any attributes? (Or perhaps hard code to exclude non-attributed methods when strict is true)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. i'll find a way to add those smarts in.

{
FullyQualifiedTypeName = fullyQualifiedTypeName;
MethodName = methodName;
MethodArity = methodArity;
MethodParameterCount = methodParameterCount;
Strict = strict;
}
}
}