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

Use Microsoft.CodeAnalysis.Testing for analyzer testing #122

Merged
merged 4 commits into from
Sep 3, 2019
Merged

Use Microsoft.CodeAnalysis.Testing for analyzer testing #122

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1e24153
Revert unnecessary versioning changes that impact downstream consumers
sharwell Aug 28, 2019
7e6c633
Update to Microsoft.CodeAnalysis.Testing
sharwell Jun 14, 2019
1d97cbd
Fix analyzer tests for .NET Core
sharwell Sep 2, 2019
d0da7b1
Move using directives outside namespace
sharwell Sep 2, 2019
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
1 change: 1 addition & 0 deletions .editorconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ insert_final_newline = true

# Sort using and Import directives with System.* appearing first
dotnet_sort_system_directives_first = true
csharp_using_directive_placement = outside_namespace:warning

# Avoid "this." and "Me." if not necessary
dotnet_style_qualification_for_field = false:suggestion
Expand Down
1 change: 1 addition & 0 deletions NuGet.Config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@
<clear/>
<add key="myget.org/F/xunit" value="https://www.myget.org/F/xunit/api/v3/index.json" protocolVersion="3" />
<add key="nuget.org" value="https://api.nuget.org/v3/index.json" protocolVersion="3" />
<add key="roslyn-analyzers" value="https://dotnet.myget.org/F/roslyn-analyzers/api/v3/index.json" protocolVersion="3" />
</packageSources>
</configuration>
7 changes: 4 additions & 3 deletions src/xunit.analyzers.fixes/xunit.analyzers.fixes.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,15 @@
<GenerateAssemblyProductAttribute>false</GenerateAssemblyProductAttribute>
<GenerateAssemblyTitleAttribute>false</GenerateAssemblyTitleAttribute>
<GenerateAssemblyVersionAttribute>false</GenerateAssemblyVersionAttribute>
<PackageTargetFallback>portable-net45+win8</PackageTargetFallback>
<RootNamespace>Xunit.Analyzers</RootNamespace>
<TargetFramework>netstandard1.3</TargetFramework>
<TargetFramework>netstandard1.1</TargetFramework>
<DebugType>full</DebugType>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.CodeAnalysis.Analyzers" Version="2.9.4" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="2.10.0" />
<PackageReference Include="Microsoft.CodeAnalysis.Analyzers" Version="2.9.4" PrivateAssets="all" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="1.2.2" />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

📝 This version has a direct impact on downstream consumers. It should only be updated when necessary to use a new API, and even then alternatives should be used when possible.

</ItemGroup>

<ItemGroup>
Expand Down
4 changes: 2 additions & 2 deletions src/xunit.analyzers.nuspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
<tags>xunit.analyzers, analyzers, roslyn, xunit, xunit.net</tags>
</metadata>
<files>
<file src="xunit.analyzers.fixes\bin\$Configuration$\netstandard1.3\xunit.analyzers.dll" target="analyzers\dotnet\cs\" />
<file src="xunit.analyzers.fixes\bin\$Configuration$\netstandard1.3\xunit.analyzers.fixes.dll" target="analyzers\dotnet\cs\" />
<file src="xunit.analyzers.fixes\bin\$Configuration$\netstandard1.1\xunit.analyzers.dll" target="analyzers\dotnet\cs\" />
<file src="xunit.analyzers.fixes\bin\$Configuration$\netstandard1.1\xunit.analyzers.fixes.dll" target="analyzers\dotnet\cs\" />
<file src="xunit.analyzers.fixes\tools\*.ps1" target="tools\" />
</files>
</package>
2 changes: 1 addition & 1 deletion src/xunit.analyzers/InlineDataMustMatchTheoryParameters.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public InlineDataMustMatchTheoryParameters()
}

/// <summary>For testing purposes only.</summary>
public InlineDataMustMatchTheoryParameters(string assemblyVersion) : base(new Version(assemblyVersion))
protected InlineDataMustMatchTheoryParameters(string assemblyVersion) : base(new Version(assemblyVersion))
{
}

Expand Down
2 changes: 1 addition & 1 deletion src/xunit.analyzers/TheoryMethodCannotHaveDefaultParameter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public class TheoryMethodCannotHaveDefaultParameter : XunitDiagnosticAnalyzer
public TheoryMethodCannotHaveDefaultParameter() { }

/// <summary>For testing purposes only.</summary>
public TheoryMethodCannotHaveDefaultParameter(string assemblyVersion) : base(new Version(assemblyVersion)) { }
protected TheoryMethodCannotHaveDefaultParameter(string assemblyVersion) : base(new Version(assemblyVersion)) { }

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics =>
ImmutableArray.Create(Descriptors.X1023_TheoryMethodCannotHaveDefaultParameter);
Expand Down
2 changes: 1 addition & 1 deletion src/xunit.analyzers/TheoryMethodCannotHaveParamsArray.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public class TheoryMethodCannotHaveParamsArray : XunitDiagnosticAnalyzer
public TheoryMethodCannotHaveParamsArray() { }

/// <summary>For testing purposes only.</summary>
public TheoryMethodCannotHaveParamsArray(string assemblyVersion) : base(new Version(assemblyVersion)) { }
protected TheoryMethodCannotHaveParamsArray(string assemblyVersion) : base(new Version(assemblyVersion)) { }

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics =>
ImmutableArray.Create(Descriptors.X1022_TheoryMethodCannotHaveParameterArray);
Expand Down
7 changes: 4 additions & 3 deletions src/xunit.analyzers/xunit.analyzers.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,15 @@
<GenerateAssemblyProductAttribute>false</GenerateAssemblyProductAttribute>
<GenerateAssemblyTitleAttribute>false</GenerateAssemblyTitleAttribute>
<GenerateAssemblyVersionAttribute>false</GenerateAssemblyVersionAttribute>
<PackageTargetFallback>portable-net45+win8</PackageTargetFallback>
<RootNamespace>Xunit.Analyzers</RootNamespace>
<TargetFramework>netstandard1.3</TargetFramework>
<TargetFramework>netstandard1.1</TargetFramework>
<DebugType>full</DebugType>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.CodeAnalysis.Analyzers" Version="2.9.4" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="2.10.0" />
<PackageReference Include="Microsoft.CodeAnalysis.Analyzers" Version="2.9.4" PrivateAssets="all" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="1.2.2" />
</ItemGroup>

</Project>
104 changes: 45 additions & 59 deletions test/xunit.analyzers.tests/AssertCollectionContainsShouldNotUseBoolCheckTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
using System.Collections.Generic;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Verify = Xunit.Analyzers.CSharpVerifier<Xunit.Analyzers.AssertCollectionContainsShouldNotUseBoolCheck>;

namespace Xunit.Analyzers
{
public class AssertCollectionContainsShouldNotUseBoolCheckTests
{
private readonly DiagnosticAnalyzer analyzer = new AssertCollectionContainsShouldNotUseBoolCheck();

public static TheoryData<string> Collections { get; } = new TheoryData<string>
{
"new System.Collections.Generic.List<int>()",
Expand All @@ -21,165 +17,155 @@ public class AssertCollectionContainsShouldNotUseBoolCheckTests
"System.Linq.Enumerable.Empty<int>()"
};

private static void CheckDiagnostics(IEnumerable<Diagnostic> diagnostics)
Copy link
Member

Choose a reason for hiding this comment

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

We appear to be losing a significant part of the testing here; namely, that we are validating the code, message, and severity of the check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are unlikely to change in a way that leads to a regression. With that said, there are a few things to keep in mind:

  1. Both the ID and the severity are getting tested (it asserts that they match the values provided in the diagnostic descriptor, which is not always a given).
  2. It is possible to write a more verbose test that covers any of these explicitly, and/or other situations that matter more for the library such as an assertion that no two analyzer types use the same ID.
  3. The new tests add dozens of new assertions covering cases that were not covered at all previously, including but not limited to the spans where the diagnostics are reported.

I'd recommend keeping most tests in this simplified form, and adding a minimal set of new tests to cover cases that might be prone to regression.

{
Assert.Collection(diagnostics, d =>
{
Assert.Equal("Do not use Contains() to check if a value exists in a collection.", d.GetMessage());
Assert.Equal("xUnit2017", d.Id);
Assert.Equal(DiagnosticSeverity.Warning, d.Severity);
});
}

[Theory]
[MemberData(nameof(Collections))]
public async void FindsWarningForTrueCollectionContainsCheck(string collection)
{
var diagnostics = await CodeAnalyzerHelper.GetDiagnosticsAsync(analyzer,
var source =
@"class TestClass { void TestMethod() {
Xunit.Assert.True(" + collection + @".Contains(1));
} }");
[|Xunit.Assert.True(" + collection + @".Contains(1))|];
} }";

CheckDiagnostics(diagnostics);
await Verify.VerifyAnalyzerAsync(source);
}

[Theory]
[MemberData(nameof(Collections))]
public async void FindsWarningForFalseCollectionContainsCheck(string collection)
{
var diagnostics = await CodeAnalyzerHelper.GetDiagnosticsAsync(analyzer,
var source =
@"class TestClass { void TestMethod() {
Xunit.Assert.False(" + collection + @".Contains(1));
} }");
[|Xunit.Assert.False(" + collection + @".Contains(1))|];
} }";

CheckDiagnostics(diagnostics);
await Verify.VerifyAnalyzerAsync(source);
}

[Theory]
[MemberData(nameof(Enumerables))]
public async void FindsWarningForTrueLinqContainsCheck(string enumerable)
{
var diagnostics = await CodeAnalyzerHelper.GetDiagnosticsAsync(analyzer,
var source =
@"using System.Linq;
class TestClass { void TestMethod() {
Xunit.Assert.True(" + enumerable + @".Contains(1));
} }");
[|Xunit.Assert.True(" + enumerable + @".Contains(1))|];
} }";

CheckDiagnostics(diagnostics);
await Verify.VerifyAnalyzerAsync(source);
}

[Theory]
[MemberData(nameof(Enumerables))]
public async void FindsWarningForTrueLinqContainsCheckWithEqualityComparer(string enumerable)
{
var diagnostics = await CodeAnalyzerHelper.GetDiagnosticsAsync(analyzer,
var source =
@"using System.Linq;
class TestClass { void TestMethod() {
Xunit.Assert.True(" + enumerable + @".Contains(1, System.Collections.Generic.EqualityComparer<int>.Default));
} }");
[|Xunit.Assert.True(" + enumerable + @".Contains(1, System.Collections.Generic.EqualityComparer<int>.Default))|];
} }";

CheckDiagnostics(diagnostics);
await Verify.VerifyAnalyzerAsync(source);
}

[Theory]
[MemberData(nameof(Enumerables))]
public async void FindsWarningForFalseLinqContainsCheck(string enumerable)
{
var diagnostics = await CodeAnalyzerHelper.GetDiagnosticsAsync(analyzer,
var source =
@"using System.Linq;
class TestClass { void TestMethod() {
Xunit.Assert.False(" + enumerable + @".Contains(1));
} }");
[|Xunit.Assert.False(" + enumerable + @".Contains(1))|];
} }";

CheckDiagnostics(diagnostics);
await Verify.VerifyAnalyzerAsync(source);
}

[Theory]
[MemberData(nameof(Enumerables))]
public async void FindsWarningForFalseLinqContainsCheckWithEqualityComparer(string enumerable)
{
var diagnostics = await CodeAnalyzerHelper.GetDiagnosticsAsync(analyzer,
var source =
@"using System.Linq;
class TestClass { void TestMethod() {
Xunit.Assert.False(" + enumerable + @".Contains(1, System.Collections.Generic.EqualityComparer<int>.Default));
} }");
[|Xunit.Assert.False(" + enumerable + @".Contains(1, System.Collections.Generic.EqualityComparer<int>.Default))|];
} }";

CheckDiagnostics(diagnostics);
await Verify.VerifyAnalyzerAsync(source);
}

[Theory]
[MemberData(nameof(Collections))]
public async void DoesNotFindWarningForTrueCollectionContainsCheckWithAssertionMessage(string collection)
{
var diagnostics = await CodeAnalyzerHelper.GetDiagnosticsAsync(analyzer,
var source =
@"class TestClass { void TestMethod() {
Xunit.Assert.True(" + collection + @".Contains(1), ""Custom message"");
} }");
} }";

Assert.Empty(diagnostics);
await Verify.VerifyAnalyzerAsync(source);
}

[Theory]
[MemberData(nameof(Collections))]
public async void DoesNotFindWarningForFalseCollectionContainsCheckWithAssertionMessage(string collection)
{
var diagnostics = await CodeAnalyzerHelper.GetDiagnosticsAsync(analyzer,
var source =
@"class TestClass { void TestMethod() {
Xunit.Assert.False(" + collection + @".Contains(1), ""Custom message"");
} }");
} }";

Assert.Empty(diagnostics);
await Verify.VerifyAnalyzerAsync(source);
}

[Theory]
[MemberData(nameof(Enumerables))]
public async void DoesNotFindWarningForTrueLinqContainsCheckWithAssertionMessage(string enumerable)
{
var diagnostics = await CodeAnalyzerHelper.GetDiagnosticsAsync(analyzer,
var source =
@"using System.Linq;
class TestClass { void TestMethod() {
Xunit.Assert.True(" + enumerable + @".Contains(1), ""Custom message"");
} }");
} }";

Assert.Empty(diagnostics);
await Verify.VerifyAnalyzerAsync(source);
}

[Theory]
[MemberData(nameof(Enumerables))]
public async void DoesNotFindWarningForFalseLinqContainsCheckWithAssertionMessage(string enumerable)
{
var diagnostics = await CodeAnalyzerHelper.GetDiagnosticsAsync(analyzer,
var source =
@"using System.Linq;
class TestClass { void TestMethod() {
Xunit.Assert.False(" + enumerable + @".Contains(1), ""Custom message"");
} }");
} }";

Assert.Empty(diagnostics);
await Verify.VerifyAnalyzerAsync(source);
}

[Fact]
public async void DoesNotCrashForCollectionWithDifferentTypeParametersThanICollectionImplementation_ZeroParameters()
{
var diagnostics = await CodeAnalyzerHelper.GetDiagnosticsAsync(analyzer,
var source =
@"using System.Collections.Generic;
class IntList : List<int> { }
class TestClass { void TestMethod() {
Xunit.Assert.False(new IntList().Contains(1));
} }");
[|Xunit.Assert.False(new IntList().Contains(1))|];
} }";

CheckDiagnostics(diagnostics);
await Verify.VerifyAnalyzerAsync(source);
}

[Fact]
public async void DoesNotCrashForCollectionWithDifferentTypeParametersThanICollectionImplementation_TwoParameters()
{
var diagnostics = await CodeAnalyzerHelper.GetDiagnosticsAsync(analyzer,
var source =
@"using System.Collections.Generic;
class TestClass { void TestMethod() {
Xunit.Assert.False(new Dictionary<int, int>().ContainsKey(1));
} }");
} }";

Assert.Empty(diagnostics);
await Verify.VerifyAnalyzerAsync(source);
}
}
}
}
24 changes: 8 additions & 16 deletions test/xunit.analyzers.tests/AssertEmptyCollectionCheckShouldNotBeUsedTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Verify = Xunit.Analyzers.CSharpVerifier<Xunit.Analyzers.AssertEmptyCollectionCheckShouldNotBeUsed>;

namespace Xunit.Analyzers
{
public class AssertEmptyCollectionCheckShouldNotBeUsedTests
{
private readonly DiagnosticAnalyzer analyzer = new AssertEmptyCollectionCheckShouldNotBeUsed();

public static TheoryData<string> Collections { get; } = new TheoryData<string>
{
"new int[0]",
Expand All @@ -20,28 +17,23 @@ public class AssertEmptyCollectionCheckShouldNotBeUsedTests
[MemberData(nameof(Collections))]
public async void FindsWarningForCollectionCheckWithoutAction(string collection)
{
var diagnostics = await CodeAnalyzerHelper.GetDiagnosticsAsync(analyzer,
var source =
@"class TestClass { void TestMethod() {
Xunit.Assert.Collection(" + collection + @");
} }");
[|Xunit.Assert.Collection(" + collection + @")|];
} }";

Assert.Collection(diagnostics, d =>
{
Assert.Equal("Do not use Assert.Collection() to check for empty collections.", d.GetMessage());
Assert.Equal("xUnit2011", d.Id);
Assert.Equal(DiagnosticSeverity.Warning, d.Severity);
});
await Verify.VerifyAnalyzerAsync(source);
}

[Theory]
[MemberData(nameof(Collections))]
public async void DoesNotFindWarningForCollectionCheckWithAction(string collection)
{
var diagnostics = await CodeAnalyzerHelper.GetDiagnosticsAsync(analyzer,
var source =
@"class TestClass { void TestMethod() {
Xunit.Assert.Collection(" + collection + @", i => Xunit.Assert.True(true));
} }");
Assert.Empty(diagnostics);
} }";
await Verify.VerifyAnalyzerAsync(source);
}
}
}
Loading