Skip to content

refactor: improve table row parsing by introducing SplitTableRow method#743

Merged
vbreuss merged 1 commit intomainfrom
topic/fix-table-formatting-in-benchmark-comment
May 1, 2026
Merged

refactor: improve table row parsing by introducing SplitTableRow method#743
vbreuss merged 1 commit intomainfrom
topic/fix-table-formatting-in-benchmark-comment

Conversation

@vbreuss
Copy link
Copy Markdown
Member

@vbreuss vbreuss commented May 1, 2026

This pull request refactors how table rows are split and parsed in the benchmark comment generation logic. The main change is the introduction of a new helper method to consistently handle table row splitting, improving code clarity and reducing duplication.

Refactoring and code quality improvements:

  • Introduced a new helper method SplitTableRow to standardize how table rows are split and parsed, ensuring consistent handling of table formatting. (Pipeline/Build.Benchmarks.cs)
  • Updated DetermineDroppedColumnIndices and RemoveColumns to use the new SplitTableRow method instead of directly splitting on the | character, which simplifies the logic and makes it more robust.

@vbreuss vbreuss self-assigned this May 1, 2026
Copilot AI review requested due to automatic review settings May 1, 2026 12:41
@vbreuss vbreuss added the bug Something isn't working label May 1, 2026
@vbreuss vbreuss merged commit b8e56d9 into main May 1, 2026
15 checks passed
@vbreuss vbreuss deleted the topic/fix-table-formatting-in-benchmark-comment branch May 1, 2026 12:42
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the benchmark markdown-table parsing in the Nuke build pipeline by centralizing row splitting logic into a new SplitTableRow helper.

Changes:

  • Introduced SplitTableRow to parse |-delimited markdown table rows by trimming entries and stripping leading/trailing empty segments.
  • Updated DetermineDroppedColumnIndices and RemoveColumns to use SplitTableRow instead of inline Split(...).

Comment on lines +225 to +236
static string[] SplitTableRow(string line)
{
string[] parts = line.Split('|', StringSplitOptions.TrimEntries);
if (parts.Length < 2)
{
return [];
}

string[] tokens = new string[parts.Length - 2];
Array.Copy(parts, 1, tokens, 0, tokens.Length);
return tokens;
}
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

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

SplitTableRow is now the canonical parser for markdown table rows, but MakeLineBold still uses its own Split("|", TrimEntries | RemoveEmptyEntries) logic. This risks inconsistent tokenization (e.g., empty cells are preserved by SplitTableRow but dropped by RemoveEmptyEntries) and makes future changes harder. Consider switching MakeLineBold to also use SplitTableRow (or otherwise centralizing the row-splitting contract in one place).

Copilot uses AI. Check for mistakes.
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 1, 2026

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 1, 2026

🚀 Benchmark Results

Details

BenchmarkDotNet v0.15.8, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
AMD EPYC 7763 2.45GHz, 1 CPU, 4 logical and 2 physical cores
.NET SDK 10.0.203
[Host] : .NET 10.0.7 (10.0.7, 10.0.726.21808), X64 RyuJIT x86-64-v3

Job=InProcess Toolchain=InProcessEmitToolchain IterationCount=15
LaunchCount=1 WarmupCount=10

Method Mean Error StdDev Ratio Gen2 Allocated Alloc Ratio
Mockolate 187.25 ns 3.386 ns 3.167 ns 1.00 - 1048 B 1.00
Imposter 275.04 ns 3.209 ns 2.845 ns 1.47 - 2248 B 2.15
TUnit.Mocks 40.30 ns 0.571 ns 0.506 ns 0.22 - 224 B 0.21
Moq 1,416.67 ns 10.486 ns 9.808 ns 7.57 - 2096 B 2.00
NSubstitute 1,908.16 ns 15.895 ns 13.273 ns 10.19 - 5048 B 4.82
FakeItEasy 1,764.89 ns 30.577 ns 28.602 ns 9.43 0.0019 2763 B 2.64
Details

BenchmarkDotNet v0.15.8, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
AMD EPYC 7763 2.97GHz, 1 CPU, 4 logical and 2 physical cores
.NET SDK 10.0.203
[Host] : .NET 10.0.7 (10.0.7, 10.0.726.21808), X64 RyuJIT x86-64-v3

Job=InProcess Toolchain=InProcessEmitToolchain IterationCount=15
LaunchCount=1 WarmupCount=10

Method Mean Error StdDev Ratio Allocated Alloc Ratio
Event_Mockolate 311.0 ns 7.35 ns 6.87 ns 1.00 1.83 KB 1.00
Event_Moq 15,728.8 ns 73.18 ns 64.87 ns 50.60 12.51 KB 6.84
Event_NSubstitute 5,746.1 ns 47.66 ns 42.25 ns 18.49 9.05 KB 4.95
Event_FakeItEasy 213,522.8 ns 1,764.88 ns 1,650.87 ns 686.98 15.39 KB 8.42
Event_Imposter 1,541.2 ns 57.90 ns 54.16 ns 4.96 8.8 KB 4.82
Event_TUnitMocks 209.6 ns 3.95 ns 3.50 ns 0.67 1.37 KB 0.75
Details

BenchmarkDotNet v0.15.8, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
AMD EPYC 7763 2.45GHz, 1 CPU, 4 logical and 2 physical cores
.NET SDK 10.0.203
[Host] : .NET 10.0.7 (10.0.7, 10.0.726.21808), X64 RyuJIT x86-64-v3

Job=InProcess Toolchain=InProcessEmitToolchain IterationCount=15
LaunchCount=1 WarmupCount=10

Method N Mean Error StdDev Ratio Allocated Alloc Ratio
Indexer_Mockolate 1 1,010.4 ns 8.93 ns 7.92 ns 1.00 3.9 KB 1.00
Indexer_Moq 1 217,946.1 ns 1,135.19 ns 1,061.86 ns 215.72 20.37 KB 5.23
Indexer_NSubstitute 1 9,413.1 ns 21.14 ns 17.65 ns 9.32 12.78 KB 3.28
Indexer_FakeItEasy 1 12,061.0 ns 41.41 ns 38.74 ns 11.94 13.88 KB 3.56
Indexer_Imposter 1 985.1 ns 6.28 ns 4.90 ns 0.98 5.16 KB 1.32
Indexer_Mockolate 10 2,513.2 ns 30.34 ns 26.90 ns 1.00 4.95 KB 1.00
Indexer_Moq 10 231,790.4 ns 1,935.56 ns 1,616.28 ns 92.24 29.89 KB 6.04
Indexer_NSubstitute 10 22,812.7 ns 47.62 ns 39.76 ns 9.08 25.58 KB 5.16
Indexer_FakeItEasy 10 26,156.3 ns 57.55 ns 51.02 ns 10.41 35.47 KB 7.16
Indexer_Imposter 10 2,515.7 ns 40.06 ns 35.51 ns 1.00 7.97 KB 1.61
Details

BenchmarkDotNet v0.15.8, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
AMD EPYC 9V74 2.60GHz, 1 CPU, 4 logical and 2 physical cores
.NET SDK 10.0.203
[Host] : .NET 10.0.7 (10.0.7, 10.0.726.21808), X64 RyuJIT x86-64-v3

Job=InProcess Toolchain=InProcessEmitToolchain IterationCount=15
LaunchCount=1 WarmupCount=10

Method N Mean Error StdDev Ratio Allocated Alloc Ratio
Method_Mockolate 1 448.9 ns 3.17 ns 2.96 ns 1.00 2.15 KB 1.00
Method_Moq 1 130,250.6 ns 376.82 ns 334.04 ns 290.16 14.58 KB 6.78
Method_NSubstitute 1 5,273.8 ns 20.47 ns 17.09 ns 11.75 9.12 KB 4.24
Method_FakeItEasy 1 5,249.5 ns 8.72 ns 8.16 ns 11.69 8.11 KB 3.78
Method_Imposter 1 548.5 ns 2.21 ns 2.07 ns 1.22 4.04 KB 1.88
Method_TUnitMocks 1 705.0 ns 1.48 ns 1.39 ns 1.57 2.9 KB 1.35
Method_Mockolate 10 683.6 ns 2.80 ns 2.48 ns 1.00 2.36 KB 1.00
Method_Moq 10 136,072.5 ns 810.55 ns 758.19 ns 199.06 18.48 KB 7.83
Method_NSubstitute 10 8,138.4 ns 23.59 ns 22.07 ns 11.91 12.07 KB 5.12
Method_FakeItEasy 10 8,286.9 ns 55.00 ns 48.76 ns 12.12 16.05 KB 6.80
Method_Imposter 10 1,148.4 ns 7.19 ns 6.37 ns 1.68 5.52 KB 2.34
Method_TUnitMocks 10 1,773.9 ns 2.42 ns 2.02 ns 2.59 4.49 KB 1.90
Details

BenchmarkDotNet v0.15.8, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
AMD EPYC 9V74 2.60GHz, 1 CPU, 4 logical and 2 physical cores
.NET SDK 10.0.203
[Host] : .NET 10.0.7 (10.0.7, 10.0.726.21808), X64 RyuJIT x86-64-v3

Job=InProcess Toolchain=InProcessEmitToolchain IterationCount=15
LaunchCount=1 WarmupCount=10

Method N Mean Error StdDev Ratio Allocated Alloc Ratio
Property_Mockolate 1 556.0 ns 15.51 ns 14.51 ns 1.00 2.46 KB 1.00
Property_Moq 1 10,334.9 ns 71.18 ns 63.10 ns 18.60 10.27 KB 4.17
Property_NSubstitute 1 7,197.3 ns 85.63 ns 80.10 ns 12.95 11.45 KB 4.65
Property_FakeItEasy 1 7,362.6 ns 113.94 ns 106.58 ns 13.25 11.24 KB 4.57
Property_Imposter 1 469.8 ns 5.67 ns 5.30 ns 0.85 3.13 KB 1.27
Property_TUnitMocks 1 759.1 ns 15.27 ns 14.28 ns 1.37 2.51 KB 1.02
Property_Mockolate 10 1,069.5 ns 21.04 ns 19.69 ns 1.00 2.95 KB 1.00
Property_Moq 10 16,536.3 ns 76.50 ns 71.56 ns 15.47 17.03 KB 5.77
Property_NSubstitute 10 16,582.4 ns 79.43 ns 70.42 ns 15.51 21.08 KB 7.14
Property_FakeItEasy 10 17,161.2 ns 233.03 ns 217.98 ns 16.05 30.81 KB 10.43
Property_Imposter 10 1,086.8 ns 16.66 ns 15.58 ns 1.02 4.67 KB 1.58
Property_TUnitMocks 10 2,296.4 ns 22.36 ns 18.67 ns 2.15 4.66 KB 1.58
Details

BenchmarkDotNet v0.15.8, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
AMD EPYC 9V74 2.60GHz, 1 CPU, 4 logical and 2 physical cores
.NET SDK 10.0.203
[Host] : .NET 10.0.7 (10.0.7, 10.0.726.21808), X64 RyuJIT x86-64-v3

Job=InProcess Toolchain=InProcessEmitToolchain IterationCount=15
LaunchCount=1 WarmupCount=10

Method Mean Error StdDev Ratio Allocated Alloc Ratio
Callback_Mockolate 371.4 ns 3.55 ns 3.32 ns 1.00 1.79 KB 1.00
Callback_Moq 69,303.3 ns 315.18 ns 263.19 ns 186.63 8.88 KB 4.97
Callback_NSubstitute 4,133.4 ns 26.24 ns 23.26 ns 11.13 7.74 KB 4.33
Callback_FakeItEasy 4,316.2 ns 18.51 ns 16.41 ns 11.62 6.81 KB 3.80
Callback_Imposter 421.2 ns 4.11 ns 3.65 ns 1.13 2.38 KB 1.33
Callback_TUnitMocks 647.3 ns 5.74 ns 5.09 ns 1.74 2.63 KB 1.47

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 2, 2026

This is addressed in release v3.1.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working state: released The issue is released

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants