Skip to content

Commit 4205cd3

Browse files
authored
.Net: Increase code coverage and add threshold to build pipeline (microsoft#4610)
### Motivation and Context <!-- Thank you for your contribution to the semantic-kernel repo! Please help reviewers and future users, providing the following information: 1. Why is this change required? 2. What problem does it solve? 3. What scenario does it contribute to? 4. If it fixes an open issue, please link to the issue here. --> Resolves: microsoft#4372 Added more tests and increased code coverage for production packages to meet 80% threshold. Added new steps to PR build pipeline to check code coverage for production packages and trigger a failure if code coverage in PR is lower than defined threshold. Example of code coverage output: ![image](https://github.com/microsoft/semantic-kernel/assets/13853051/2a271dcf-5e87-4006-a3b1-5bcfce829f3a) ### Contribution Checklist <!-- Before submitting this PR, please make sure: --> - [x] The code builds clean without any errors or warnings - [x] The PR follows the [SK Contribution Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md) and the [pre-submission formatting script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts) raises no violations - [x] All unit tests pass, and I have added new tests where possible - [x] I didn't break anyone 😄
1 parent 4ec03be commit 4205cd3

File tree

68 files changed

+3557
-85
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

68 files changed

+3557
-85
lines changed

.github/workflows/check-coverage.ps1

+54
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
param (
2+
[string]$JsonReportPath,
3+
[double]$CoverageThreshold
4+
)
5+
6+
$jsonContent = Get-Content $JsonReportPath -Raw | ConvertFrom-Json
7+
$coverageBelowThreshold = $false
8+
9+
function Get-FormattedValue($number) {
10+
$formattedNumber = "{0:N1}" -f $number
11+
$icon = if ($number -ge $CoverageThreshold) { '' } else { '' }
12+
13+
return "$formattedNumber% $icon"
14+
}
15+
16+
$lineCoverage = $jsonContent.summary.linecoverage
17+
$branchCoverage = $jsonContent.summary.branchcoverage
18+
19+
if ($lineCoverage -lt $CoverageThreshold -or $branchCoverage -lt $CoverageThreshold) {
20+
$coverageBelowThreshold = $true
21+
}
22+
23+
$totalTableData = [PSCustomObject]@{
24+
'Metric' = 'Total Coverage'
25+
'Line Coverage' = Get-FormattedValue $lineCoverage
26+
'Branch Coverage' = Get-FormattedValue $branchCoverage
27+
}
28+
29+
$totalTableData | Format-Table -AutoSize
30+
31+
$assemblyTableData = @()
32+
33+
foreach ($assembly in $jsonContent.coverage.assemblies) {
34+
$assemblyName = $assembly.name
35+
$assemblyLineCoverage = $assembly.coverage
36+
$assemblyBranchCoverage = $assembly.branchcoverage
37+
38+
if ($assemblyLineCoverage -lt $CoverageThreshold -or $assemblyBranchCoverage -lt $CoverageThreshold) {
39+
$coverageBelowThreshold = $true
40+
}
41+
42+
$assemblyTableData += [PSCustomObject]@{
43+
'Assembly Name' = $assemblyName
44+
'Line' = Get-FormattedValue $assemblyLineCoverage
45+
'Branch' = Get-FormattedValue $assemblyBranchCoverage
46+
}
47+
}
48+
49+
$assemblyTableData | Format-Table -AutoSize
50+
51+
if ($coverageBelowThreshold) {
52+
Write-Host "Code coverage is lower than defined threshold: $CoverageThreshold. Stopping the task."
53+
exit 1
54+
}

.github/workflows/dotnet-build-and-test.yml

+18-1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ on:
1212
merge_group:
1313
branches: ["main"]
1414

15+
env:
16+
COVERAGE_THRESHOLD: 80
17+
1518
concurrency:
1619
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
1720
cancel-in-progress: true
@@ -82,7 +85,7 @@ jobs:
8285
run: |
8386
export UT_PROJECTS=$(find ./dotnet -type f -name "*.UnitTests.csproj" | grep -v -E "(Planners.Core.UnitTests.csproj|Experimental.Orchestration.Flow.UnitTests.csproj|Experimental.Assistants.UnitTests.csproj)" | tr '\n' ' ')
8487
for project in $UT_PROJECTS; do
85-
dotnet test -c ${{ matrix.configuration }} $project --no-build -v Normal --logger trx
88+
dotnet test -c ${{ matrix.configuration }} $project --no-build -v Normal --logger trx --collect:"XPlat Code Coverage" --results-directory:"TestResults/Coverage/"
8689
done
8790
8891
- name: Run Integration Tests
@@ -109,6 +112,20 @@ jobs:
109112
Bing__ApiKey: ${{ secrets.BING__APIKEY }}
110113
OpenAI__ApiKey: ${{ secrets.OPENAI__APIKEY }}
111114

115+
# Generate test reports and check coverage
116+
- name: Generate test reports
117+
uses: danielpalme/[email protected]
118+
with:
119+
reports: "./TestResults/Coverage/**/coverage.cobertura.xml"
120+
targetdir: "./TestResults/Reports"
121+
reporttypes: "JsonSummary"
122+
# Report for production packages only
123+
assemblyfilters: "+Microsoft.SemanticKernel.Abstractions;+Microsoft.SemanticKernel.Core;+Microsoft.SemanticKernel.PromptTemplates.Handlebars;+Microsoft.SemanticKernel.Connectors.OpenAI;+Microsoft.SemanticKernel.Yaml;"
124+
125+
- name: Check coverage
126+
shell: pwsh
127+
run: .github/workflows/check-coverage.ps1 -JsonReportPath "TestResults/Reports/Summary.json" -CoverageThreshold $env:COVERAGE_THRESHOLD
128+
112129
# This final job is required to satisfy the merge queue. It must only run (or succeed) if no tests failed
113130
dotnet-build-and-test-check:
114131
if: always()

dotnet/SK-dotnet.sln

+6
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,11 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "System", "System", "{3CDE10
118118
src\InternalUtilities\src\System\EnvExtensions.cs = src\InternalUtilities\src\System\EnvExtensions.cs
119119
EndProjectSection
120120
EndProject
121+
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Type", "Type", "{E85EA4D0-BB7E-4DFD-882F-A76EB8C0B8FF}"
122+
ProjectSection(SolutionItems) = preProject
123+
src\InternalUtilities\src\Type\TypeExtensions.cs = src\InternalUtilities\src\Type\TypeExtensions.cs
124+
EndProjectSection
125+
EndProject
121126
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Plugins.Core", "src\Plugins\Plugins.Core\Plugins.Core.csproj", "{0D0C4DAD-E6BC-4504-AE3A-EEA4E35920C1}"
122127
EndProject
123128
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "NCalcPlugins", "samples\NCalcPlugins\NCalcPlugins.csproj", "{E6EDAB8F-3406-4DBF-9AAB-DF40DC2CA0FA}"
@@ -500,6 +505,7 @@ Global
500505
{B00AD427-0047-4850-BEF9-BA8237EA9D8B} = {958AD708-F048-4FAF-94ED-D2F2B92748B9}
501506
{1C19D805-3573-4477-BF07-40180FCDE1BD} = {958AD708-F048-4FAF-94ED-D2F2B92748B9}
502507
{3CDE10B2-AE8F-4FC4-8D55-92D4AD32E144} = {958AD708-F048-4FAF-94ED-D2F2B92748B9}
508+
{E85EA4D0-BB7E-4DFD-882F-A76EB8C0B8FF} = {958AD708-F048-4FAF-94ED-D2F2B92748B9}
503509
{0D0C4DAD-E6BC-4504-AE3A-EEA4E35920C1} = {D6D598DF-C17C-46F4-B2B9-CDE82E2DE132}
504510
{E6EDAB8F-3406-4DBF-9AAB-DF40DC2CA0FA} = {FA3720F1-C99A-49B2-9577-A940257098BF}
505511
{C754950A-E16C-4F96-9CC7-9328E361B5AF} = {FA3720F1-C99A-49B2-9577-A940257098BF}

dotnet/code-coverage.ps1

+60
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
# This script is for local use to analyze code coverage in more detail using HTML report.
2+
3+
Param(
4+
[switch]$ProdPackagesOnly = $false
5+
)
6+
7+
# Generate a timestamp for the current date and time
8+
$timestamp = Get-Date -Format "yyyyMMdd-HHmmss"
9+
10+
# Define paths
11+
$scriptPath = Get-Item -Path $PSScriptRoot
12+
$coverageOutputPath = Join-Path $scriptPath "TestResults\Coverage\$timestamp"
13+
$reportOutputPath = Join-Path $scriptPath "TestResults\Reports\$timestamp"
14+
15+
# Create output directories
16+
New-Item -ItemType Directory -Force -Path $coverageOutputPath
17+
New-Item -ItemType Directory -Force -Path $reportOutputPath
18+
19+
# Find tests for projects ending with 'UnitTests.csproj'
20+
$testProjects = Get-ChildItem $scriptPath -Filter "*UnitTests.csproj" -Recurse
21+
22+
foreach ($project in $testProjects) {
23+
$testProjectPath = $project.FullName
24+
Write-Host "Running tests for project: $($testProjectPath)"
25+
26+
# Run tests
27+
dotnet test $testProjectPath `
28+
--collect:"XPlat Code Coverage" `
29+
--results-directory:$coverageOutputPath `
30+
31+
}
32+
33+
# Install required tools
34+
& dotnet tool install -g coverlet.console
35+
& dotnet tool install -g dotnet-reportgenerator-globaltool
36+
37+
# Generate HTML report
38+
if ($ProdPackagesOnly) {
39+
$assemblies = @(
40+
"+Microsoft.SemanticKernel.Abstractions",
41+
"+Microsoft.SemanticKernel.Core",
42+
"+Microsoft.SemanticKernel.PromptTemplates.Handlebars",
43+
"+Microsoft.SemanticKernel.Connectors.OpenAI",
44+
"+Microsoft.SemanticKernel.Yaml"
45+
)
46+
47+
$assemblyFilters = $assemblies -join ";"
48+
49+
# Generate report for production assemblies only
50+
& reportgenerator -reports:"$coverageOutputPath/**/coverage.cobertura.xml" -targetdir:$reportOutputPath -reporttypes:Html -assemblyfilters:$assemblyFilters
51+
}
52+
else {
53+
& reportgenerator -reports:"$coverageOutputPath/**/coverage.cobertura.xml" -targetdir:$reportOutputPath -reporttypes:Html
54+
}
55+
56+
Write-Host "Code coverage report generated at: $reportOutputPath"
57+
58+
# Open report
59+
$reportIndexHtml = Join-Path $reportOutputPath "index.html"
60+
Invoke-Item -Path $reportIndexHtml

dotnet/src/Connectors/Connectors.OpenAI/AzureSdk/AzureOpenAIClientCore.cs

-13
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
using System;
44
using System.Net.Http;
5-
using System.Runtime.CompilerServices;
65
using Azure;
76
using Azure.AI.OpenAI;
87
using Azure.Core;
@@ -98,16 +97,4 @@ internal AzureOpenAIClientCore(
9897

9998
this.AddAttribute(DeploymentNameKey, deploymentName);
10099
}
101-
102-
/// <summary>
103-
/// Logs Azure OpenAI action details.
104-
/// </summary>
105-
/// <param name="callerMemberName">Caller member name. Populated automatically by runtime.</param>
106-
internal void LogActionDetails([CallerMemberName] string? callerMemberName = default)
107-
{
108-
if (this.Logger.IsEnabled(LogLevel.Information))
109-
{
110-
this.Logger.LogInformation("Action: {Action}. Azure OpenAI Deployment Name: {DeploymentName}.", callerMemberName, this.DeploymentOrModelName);
111-
}
112-
}
113100
}

dotnet/src/Connectors/Connectors.OpenAI/AzureSdk/AzureOpenAIWithDataChatMessageContent.cs

+2-1
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@ internal AzureOpenAIWithDataChatMessageContent(ChatWithDataChoice chatChoice, st
3030
: base(default, string.Empty, modelId, chatChoice, System.Text.Encoding.UTF8, CreateMetadataDictionary(metadata))
3131
{
3232
// An assistant message content must be present, otherwise the chat is not valid.
33-
var chatMessage = chatChoice.Messages.First(m => string.Equals(m.Role, AuthorRole.Assistant.Label, StringComparison.OrdinalIgnoreCase));
33+
var chatMessage = chatChoice.Messages.FirstOrDefault(m => string.Equals(m.Role, AuthorRole.Assistant.Label, StringComparison.OrdinalIgnoreCase)) ??
34+
throw new ArgumentException("Chat is not valid. Chat message does not contain any messages with 'assistant' role.");
3435

3536
this.Content = chatMessage.Content;
3637
this.Role = new AuthorRole(chatMessage.Role);

dotnet/src/Connectors/Connectors.OpenAI/TextEmbedding/AzureOpenAITextEmbeddingGenerationService.cs

-1
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,6 @@ public Task<IList<ReadOnlyMemory<float>>> GenerateEmbeddingsAsync(
9393
Kernel? kernel = null,
9494
CancellationToken cancellationToken = default)
9595
{
96-
this._core.LogActionDetails();
9796
return this._core.GetEmbeddingsAsync(data, kernel, cancellationToken);
9897
}
9998
}

dotnet/src/Connectors/Connectors.OpenAI/TextToImage/OpenAITextToImageService.cs

+1-5
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
using System;
44
using System.Collections.Generic;
5-
using System.Diagnostics;
65
using System.Diagnostics.CodeAnalysis;
76
using System.Net.Http;
87
using System.Text.Json;
@@ -87,10 +86,7 @@ private async Task<string> GenerateImageAsync(
8786
string format, Func<TextToImageResponse.Image, string> extractResponse,
8887
CancellationToken cancellationToken)
8988
{
90-
Debug.Assert(width == height);
91-
Debug.Assert(width is 256 or 512 or 1024);
92-
Debug.Assert(format is "url" or "b64_json");
93-
Debug.Assert(extractResponse is not null);
89+
Verify.NotNull(extractResponse);
9490

9591
var requestBody = JsonSerializer.Serialize(new TextToImageRequest
9692
{

dotnet/src/Connectors/Connectors.UnitTests/Connectors.UnitTests.csproj

+7-10
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,17 @@
2727
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
2828
<PrivateAssets>all</PrivateAssets>
2929
</PackageReference>
30+
<PackageReference Include="coverlet.collector">
31+
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
32+
<PrivateAssets>all</PrivateAssets>
33+
</PackageReference>
3034
<PackageReference Include="System.Numerics.Tensors" />
3135
<PackageReference Include="System.Text.Json" />
3236
<PackageReference Include="Microsoft.Extensions.DependencyInjection" />
3337
</ItemGroup>
3438

3539
<ItemGroup>
36-
<Compile Include="$(RepoRoot)/dotnet/src/InternalUtilities/test/AssertExtensions.cs"
37-
Link="%(RecursiveDir)%(Filename)%(Extension)" />
40+
<Compile Include="$(RepoRoot)/dotnet/src/InternalUtilities/test/AssertExtensions.cs" Link="%(RecursiveDir)%(Filename)%(Extension)" />
3841
</ItemGroup>
3942

4043
<ItemGroup>
@@ -55,16 +58,10 @@
5558
</ItemGroup>
5659

5760
<ItemGroup>
58-
<None Update="HuggingFace\TestData\completion_test_response.json">
59-
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
60-
</None>
61-
<None Update="HuggingFace\TestData\embeddings_test_response.json">
62-
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
63-
</None>
64-
<None Update="OpenAI\TestData\image_result_test_response.json">
61+
<None Update="HuggingFace\TestData\*">
6562
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
6663
</None>
67-
<None Update="OpenAI\TestData\image_generation_test_response.json">
64+
<None Update="OpenAI\TestData\*">
6865
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
6966
</None>
7067
</ItemGroup>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
// Copyright (c) Microsoft. All rights reserved.
2+
3+
using System;
4+
using System.Collections.Generic;
5+
using System.Net.Http;
6+
using System.Net.Http.Headers;
7+
using System.Threading;
8+
using System.Threading.Tasks;
9+
10+
namespace SemanticKernel.Connectors.UnitTests;
11+
12+
internal sealed class MultipleHttpMessageHandlerStub : DelegatingHandler
13+
{
14+
private int _callIteration = 0;
15+
16+
public List<HttpRequestHeaders?> RequestHeaders { get; private set; }
17+
18+
public List<HttpContentHeaders?> ContentHeaders { get; private set; }
19+
20+
public List<byte[]?> RequestContents { get; private set; }
21+
22+
public List<Uri?> RequestUris { get; private set; }
23+
24+
public List<HttpMethod?> Methods { get; private set; }
25+
26+
public List<HttpResponseMessage> ResponsesToReturn { get; set; }
27+
28+
public MultipleHttpMessageHandlerStub()
29+
{
30+
this.RequestHeaders = [];
31+
this.ContentHeaders = [];
32+
this.RequestContents = [];
33+
this.RequestUris = [];
34+
this.Methods = [];
35+
this.ResponsesToReturn = [];
36+
}
37+
38+
protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
39+
{
40+
this._callIteration++;
41+
42+
this.Methods.Add(request.Method);
43+
this.RequestUris.Add(request.RequestUri);
44+
this.RequestHeaders.Add(request.Headers);
45+
this.ContentHeaders.Add(request.Content?.Headers);
46+
47+
var content = request.Content == null ? null : await request.Content.ReadAsByteArrayAsync(cancellationToken);
48+
49+
this.RequestContents.Add(content);
50+
51+
return await Task.FromResult(this.ResponsesToReturn[this._callIteration - 1]);
52+
}
53+
}

0 commit comments

Comments
 (0)