Skip to content

Commit a101590

Browse files
Fix "label column cannot start at the end of the line" error in Console reporting (#1224)
Co-authored-by: Pascal Berger <[email protected]>
1 parent afa590d commit a101590

9 files changed

+430
-87
lines changed

src/Cake.Issues.Reporting.Console.Tests/Cake.Issues.Reporting.Console.Tests.csproj

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,11 @@
33
<Description>Tests for the Cake.Issues.Reporting.Console addin</Description>
44
</PropertyGroup>
55

6+
<ItemGroup>
7+
<PackageReference Include="Spectre.Console.Testing" />
8+
<PackageReference Include="Verify.Xunit" />
9+
</ItemGroup>
10+
611
<ItemGroup>
712
<None Remove="Testfiles\**\*" />
813
</ItemGroup>

src/Cake.Issues.Reporting.Console.Tests/ConsoleIssueReportFixture.cs

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,17 @@
44
using Cake.Core.Diagnostics;
55
using Cake.Core.IO;
66
using Cake.Issues.Serialization;
7+
using Spectre.Console.Testing;
78

89
internal class ConsoleIssueReportFixture
910
{
11+
public TestConsole Console { get; set; } = new();
12+
1013
public FakeLog Log { get; set; } = new() { Verbosity = Verbosity.Normal };
1114

1215
public ConsoleIssueReportFormatSettings ConsoleIssueReportFormatSettings { get; set; } = new();
1316

14-
public string CreateReportForTestfile(string fileResourceName, DirectoryPath repositoryRootPath)
17+
public void CreateReportForTestfile(string fileResourceName, DirectoryPath repositoryRootPath)
1518
{
1619
fileResourceName.NotNullOrWhiteSpace();
1720

@@ -28,22 +31,23 @@ public string CreateReportForTestfile(string fileResourceName, DirectoryPath rep
2831
}
2932

3033
var issues = reader.ReadToEnd().DeserializeToIssues();
31-
return this.CreateReport(issues, repositoryRootPath);
34+
this.CreateReport(issues, repositoryRootPath);
3235
}
3336
}
3437

35-
public string CreateReport(IEnumerable<IIssue> issues, DirectoryPath repositoryRootPath)
38+
public void CreateReport(IEnumerable<IIssue> issues, DirectoryPath repositoryRootPath)
3639
{
3740
var generator =
38-
new ConsoleIssueReportGenerator(this.Log, this.ConsoleIssueReportFormatSettings);
41+
new ConsoleIssueReportGenerator(
42+
this.Console,
43+
this.Log,
44+
this.ConsoleIssueReportFormatSettings);
3945

4046
var createIssueReportSettings =
4147
new CreateIssueReportSettings(repositoryRootPath, string.Empty);
4248
_ = generator.Initialize(createIssueReportSettings);
4349
_ = generator.CreateReport(issues);
4450

45-
// TODO Return console output
46-
return string.Empty;
4751
}
4852

4953
public void TestReportCreation(Action<ConsoleIssueReportFormatSettings> settings)
@@ -52,20 +56,17 @@ public void TestReportCreation(Action<ConsoleIssueReportFormatSettings> settings
5256
settings(this.ConsoleIssueReportFormatSettings);
5357

5458
// When
55-
var result =
56-
this.CreateReport(
57-
[
58-
IssueBuilder
59-
.NewIssue("Message Foo", "ProviderType Foo", "ProviderName Foo")
60-
.InFile(@"src\Cake.Issues.Reporting.Generic.Tests\Foo.cs", 10)
61-
.OfRule("Rule Foo")
62-
.WithPriority(IssuePriority.Warning)
63-
.Create(),
64-
],
65-
@"c:\Source\Cake.Issues.Reporting.Console");
59+
this.CreateReport(
60+
[
61+
IssueBuilder
62+
.NewIssue("Message Foo", "ProviderType Foo", "ProviderName Foo")
63+
.InFile(@"src\Cake.Issues.Reporting.Generic.Tests\Foo.cs", 10)
64+
.OfRule("Rule Foo")
65+
.WithPriority(IssuePriority.Warning)
66+
.Create(),
67+
],
68+
@"c:\Source\Cake.Issues.Reporting.Console");
6669

6770
// Then
68-
// Currently only checks if generation failed or not without checking actual output.
69-
_ = result.ShouldNotBeNull();
7071
}
7172
}

src/Cake.Issues.Reporting.Console.Tests/ConsoleIssueReportGeneratorTests.cs

Lines changed: 96 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,48 @@
11
namespace Cake.Issues.Reporting.Console.Tests;
22

3+
using System.Text.RegularExpressions;
4+
using Cake.Core.Diagnostics;
5+
using Spectre.Console;
6+
using Spectre.Console.Testing;
37
using Xunit.Abstractions;
48

5-
public sealed class ConsoleIssueReportGeneratorTests
9+
public sealed partial class ConsoleIssueReportGeneratorTests
610
{
711
public sealed class TheCtor
812
{
13+
[Fact]
14+
public void Should_Throw_If_Console_Is_Null()
15+
{
16+
// Given
17+
IAnsiConsole console = null;
18+
var log = new FakeLog();
19+
var settings = new ConsoleIssueReportFormatSettings();
20+
21+
// When
22+
var result = Record.Exception(() =>
23+
new ConsoleIssueReportGenerator(
24+
console,
25+
log,
26+
settings));
27+
28+
// Then
29+
result.IsArgumentNullException("console");
30+
}
31+
932
[Fact]
1033
public void Should_Throw_If_Log_Is_Null()
1134
{
12-
// Given / When
35+
// Given
36+
var console = new TestConsole();
37+
ICakeLog log = null;
38+
var settings = new ConsoleIssueReportFormatSettings();
39+
40+
// When
1341
var result = Record.Exception(() =>
1442
new ConsoleIssueReportGenerator(
15-
null,
16-
new ConsoleIssueReportFormatSettings()));
43+
console,
44+
log,
45+
settings));
1746

1847
// Then
1948
result.IsArgumentNullException("log");
@@ -22,18 +51,24 @@ public void Should_Throw_If_Log_Is_Null()
2251
[Fact]
2352
public void Should_Throw_If_Settings_Are_Null()
2453
{
25-
// Given / When
54+
// Given
55+
var console = new TestConsole();
56+
var log = new FakeLog();
57+
ConsoleIssueReportFormatSettings settings = null;
58+
59+
// When
2660
var result = Record.Exception(() =>
2761
new ConsoleIssueReportGenerator(
28-
new FakeLog(),
29-
null));
62+
console,
63+
log,
64+
settings));
3065

3166
// Then
3267
result.IsArgumentNullException("settings");
3368
}
3469
}
3570

36-
public sealed class TheInternalCreateReportMethod
71+
public sealed partial class TheInternalCreateReportMethod
3772
{
3873
public static IEnumerable<object[]> ReportFormatSettingsCombinations =>
3974
from b1 in boolArray
@@ -68,7 +103,7 @@ public void Should_Generate_Report(
68103
};
69104

70105
// When
71-
_ = fixture.CreateReportForTestfile(
106+
fixture.CreateReportForTestfile(
72107
"Testfiles.issues.json",
73108
@"c:\Source\Cake.Issues.Reporting.Console");
74109

@@ -98,15 +133,21 @@ public void Should_Generate_Report_With_No_Issues(
98133
};
99134

100135
// When
101-
_ = fixture.CreateReport(
136+
fixture.CreateReport(
102137
[],
103138
@"c:\Source\Cake.Issues.Reporting.Console");
104139

105140
// Then
106141
}
107142

108-
public sealed class WithShowDiagnosticsEnabled(ITestOutputHelper output)
143+
public sealed partial class WithShowDiagnosticsEnabled(ITestOutputHelper output)
109144
{
145+
// (?<=┌─\[) — positive lookbehind to assert the match is preceded by ┌─[
146+
// [^\]]+ — matches one or more characters that are not a closing bracket ]
147+
// (?=\]) — positive lookahead to assert the match is followed by ]
148+
[GeneratedRegex(@"(?<=┌─\[)[^\]]+(?=\])")]
149+
private static partial Regex DiagnosticRegEx();
150+
110151
[Fact]
111152
public void Should_Filter_Issues_Without_FilePath()
112153
{
@@ -128,7 +169,7 @@ public void Should_Filter_Issues_Without_FilePath()
128169
};
129170

130171
// When
131-
_ = fixture.CreateReport(issues, @"c:\Source\Cake.Issues.Reporting.Console");
172+
fixture.CreateReport(issues, @"c:\Source\Cake.Issues.Reporting.Console");
132173

133174
// Then
134175
fixture.Log.Entries.ShouldContain(x => x.Message == "1 issue(s) were filtered because they either don't belong to a file or the file does not exist.");
@@ -155,7 +196,7 @@ public void Should_Filter_Issues_Where_File_Does_Not_Exist()
155196
};
156197

157198
// When
158-
_ = fixture.CreateReport(issues, @"c:\Source\Cake.Issues.Reporting.Console");
199+
fixture.CreateReport(issues, @"c:\Source\Cake.Issues.Reporting.Console");
159200

160201
// Then
161202
fixture.Log.Entries.ShouldContain(x => x.Message == "1 issue(s) were filtered because they either don't belong to a file or the file does not exist.");
@@ -187,7 +228,7 @@ public void Should_Not_Filter_Issues_With_Existing_File()
187228
};
188229

189230
// When
190-
_ = fixture.CreateReport(issues, directory);
231+
fixture.CreateReport(issues, directory);
191232

192233
// Then
193234
fixture.Log.Entries.ShouldContain(x => x.Message == "0 issue(s) were filtered because they either don't belong to a file or the file does not exist.");
@@ -218,7 +259,7 @@ public void Should_Work_Without_Priority()
218259
.Create(),
219260
};
220261
// When
221-
_ = fixture.CreateReport(issues, directory);
262+
fixture.CreateReport(issues, directory);
222263

223264
// Then
224265
}
@@ -249,10 +290,49 @@ public void Should_Work_With_Priority()
249290
.Create(),
250291
};
251292
// When
252-
_ = fixture.CreateReport(issues, directory);
293+
fixture.CreateReport(issues, directory);
253294

254295
// Then
255296
}
297+
298+
[Fact]
299+
public Task Should_Work_With_Issue_On_End_Of_Line()
300+
{
301+
// Given
302+
using var tempSourceFile = new TemporarySourceFile("Testfiles.TestFile.txt");
303+
var filePath = tempSourceFile.FilePath;
304+
output.WriteLine($"File path: {filePath}");
305+
var directory = Path.GetDirectoryName(filePath)!;
306+
var fileName = Path.GetFileName(filePath);
307+
var fixture = new ConsoleIssueReportFixture
308+
{
309+
ConsoleIssueReportFormatSettings =
310+
{
311+
ShowDiagnostics = true
312+
}
313+
};
314+
var issues =
315+
new List<IIssue>
316+
{
317+
IssueBuilder
318+
.NewIssue("Message Foo", "ProviderType Foo", "ProviderName Foo")
319+
.InFile(fileName, 1, 57) // Position after the last character on line 1
320+
.Create(),
321+
};
322+
// When
323+
fixture.CreateReport(issues, directory);
324+
325+
// Then
326+
// Add a scrubber that replaces the dynamic ID in the output
327+
var settings = new VerifySettings();
328+
settings.AddScrubber(builder =>
329+
{
330+
var updated = DiagnosticRegEx().Replace(builder.ToString(), "<DYNAMIC_ID>");
331+
332+
_ = builder.Clear().Append(updated);
333+
});
334+
return Verify(fixture.Console.Output, settings);
335+
}
256336
}
257337
}
258338
}

0 commit comments

Comments
 (0)