Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
using System;
using System.Threading.Tasks;
using Xunit;
using BannerlordModEditor.Common.Services;

namespace BannerlordModEditor.Common.Tests.Services
{
public class SdkCheckerServiceTests
{
private readonly SdkCheckerService _service;

public SdkCheckerServiceTests()
{
_service = new SdkCheckerService();
}

[Fact]
public void GetDownloadUrl_ReturnsValidUrl()
{
var url = _service.GetDownloadUrl();
Assert.NotNull(url);
Assert.Contains("dotnet", url);
}

[Fact]
public async Task CheckSdkAsync_ReturnsValidInfo()
{
var info = await _service.CheckSdkAsync();

Assert.NotNull(info);
Assert.NotNull(info.RecommendedVersion);
Assert.NotNull(info.DownloadUrl);
}
Comment on lines +26 to +33
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

These tests execute the real dotnet CLI on the machine running the tests. That makes them environment-dependent and potentially flaky in CI (e.g., missing SDK, different PATH, localized output). To make them true unit tests, introduce an abstraction for process execution (or inject a runner) and mock outputs to deterministically test parsing/version detection/path extraction; then add tests for --list-sdks parsing and GetSdkPath() behavior.

Copilot uses AI. Check for mistakes.

[Fact]
public void CheckSdk_ReturnsValidInfo()
{
var info = _service.CheckSdk();

Assert.NotNull(info);
Assert.NotNull(info.RecommendedVersion);
Assert.NotNull(info.DownloadUrl);
}

[Fact]
public void IsSdkInstalled_ReturnsBoolean()
{
var result = _service.IsSdkInstalled();
Assert.IsType<bool>(result);
}

[Fact]
public void GetSdkVersion_ReturnsStringOrNull()
{
var result = _service.GetSdkVersion();
Assert.True(result == null || !string.IsNullOrEmpty(result));
}

[Fact]
public void GetInstalledVersions_ReturnsList()
{
var versions = _service.GetInstalledVersions();
Assert.NotNull(versions);
}
}
}
202 changes: 202 additions & 0 deletions BannerlordModEditor.Common/Services/SdkCheckerService.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,202 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Runtime.InteropServices;
using System.Threading.Tasks;

namespace BannerlordModEditor.Common.Services
{
public class SdkInfo
{
public bool IsInstalled { get; set; }
public string? Version { get; set; }
public string? Path { get; set; }
public List<string> InstalledVersions { get; set; } = new();
public string? RecommendedVersion { get; set; }
public string? DownloadUrl { get; set; }
}

public interface ISdkCheckerService
{
Task<SdkInfo> CheckSdkAsync();
SdkInfo CheckSdk();
bool IsSdkInstalled();
string? GetSdkVersion();
List<string> GetInstalledVersions();
string GetDownloadUrl();
}

public class SdkCheckerService : ISdkCheckerService
{
private const string RecommendedSdkVersion = "9.0.100";
private const string DownloadUrlBase = "https://dotnet.microsoft.com/download/dotnet/9.0";

public async Task<SdkInfo> CheckSdkAsync()
{
return await Task.Run(() => CheckSdk());
Comment on lines +36 to +38
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

CheckSdkAsync wraps a synchronous, blocking implementation in Task.Run, which is generally discouraged for library/service code (it consumes a thread-pool thread and doesn’t make the underlying work non-blocking). Prefer either (a) making the underlying process execution truly async (e.g., async read + WaitForExitAsync) or (b) removing the async method and keeping only CheckSdk() if true async isn’t feasible.

Suggested change
public async Task<SdkInfo> CheckSdkAsync()
{
return await Task.Run(() => CheckSdk());
public Task<SdkInfo> CheckSdkAsync()
{
return Task.FromResult(CheckSdk());

Copilot uses AI. Check for mistakes.
}

public SdkInfo CheckSdk()
{
var info = new SdkInfo
{
IsInstalled = IsSdkInstalled(),
Version = GetSdkVersion(),
Path = GetSdkPath(),
InstalledVersions = GetInstalledVersions(),
RecommendedVersion = RecommendedSdkVersion,
DownloadUrl = GetDownloadUrl()
};

return info;
}

public bool IsSdkInstalled()
{
try
{
var process = new Process
{
StartInfo = new ProcessStartInfo
{
FileName = "dotnet",
Arguments = "--version",
RedirectStandardOutput = true,
RedirectStandardError = true,
UseShellExecute = false,
CreateNoWindow = true
}
};

process.Start();
var output = process.StandardOutput.ReadToEnd();
process.WaitForExit();

Comment on lines +74 to +76
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

This can deadlock/hang: RedirectStandardError = true but stderr is never drained. If dotnet writes enough to stderr, the child process can block, causing ReadToEnd()/WaitForExit() to hang indefinitely. Fix by either reading stderr as well (or not redirecting it), and consider using WaitForExitAsync plus async reads to avoid blocking.

Suggested change
var output = process.StandardOutput.ReadToEnd();
process.WaitForExit();
var stdoutTask = process.StandardOutput.ReadToEndAsync();
var stderrTask = process.StandardError.ReadToEndAsync();
process.WaitForExit();
Task.WaitAll(stdoutTask, stderrTask);
var output = stdoutTask.Result;

Copilot uses AI. Check for mistakes.
return process.ExitCode == 0 && !string.IsNullOrWhiteSpace(output);
Comment on lines +56 to +77
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

Process-start logic is duplicated across IsSdkInstalled, GetSdkVersion, GetInstalledVersions, and GetSdkPath, making it harder to consistently apply fixes (timeouts, stderr draining, disposal, etc.). Consider extracting a single helper (e.g., RunDotnet(args) returning exit code + stdout/stderr) and reuse it for all commands.

Suggested change
public bool IsSdkInstalled()
{
try
{
var process = new Process
{
StartInfo = new ProcessStartInfo
{
FileName = "dotnet",
Arguments = "--version",
RedirectStandardOutput = true,
RedirectStandardError = true,
UseShellExecute = false,
CreateNoWindow = true
}
};
process.Start();
var output = process.StandardOutput.ReadToEnd();
process.WaitForExit();
return process.ExitCode == 0 && !string.IsNullOrWhiteSpace(output);
private (int ExitCode, string StdOut, string StdErr) RunDotnet(string arguments)
{
var startInfo = new ProcessStartInfo
{
FileName = "dotnet",
Arguments = arguments,
RedirectStandardOutput = true,
RedirectStandardError = true,
UseShellExecute = false,
CreateNoWindow = true
};
using var process = new Process
{
StartInfo = startInfo
};
process.Start();
string stdout = process.StandardOutput.ReadToEnd();
string stderr = process.StandardError.ReadToEnd();
process.WaitForExit();
return (process.ExitCode, stdout, stderr);
}
public bool IsSdkInstalled()
{
try
{
var result = RunDotnet("--version");
return result.ExitCode == 0 && !string.IsNullOrWhiteSpace(result.StdOut);

Copilot uses AI. Check for mistakes.
}
catch
{
return false;
}
}

public string? GetSdkVersion()
{
try
{
var process = new Process
{
StartInfo = new ProcessStartInfo
{
FileName = "dotnet",
Arguments = "--version",
RedirectStandardOutput = true,
UseShellExecute = false,
CreateNoWindow = true
}
};

process.Start();
var output = process.StandardOutput.ReadToEnd().Trim();
process.WaitForExit();
Comment on lines +89 to +103
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

All process invocations use blocking ReadToEnd() + WaitForExit() without any timeout/cancellation. In environments where dotnet is slow/hung/misconfigured, this can block the calling thread (and CheckSdkAsync will still tie up a thread-pool thread). Consider adding a bounded timeout (and/or cancellation token support) and cleaning up the process if it exceeds the limit.

Copilot uses AI. Check for mistakes.

return process.ExitCode == 0 ? output : null;
}
catch
{
return null;
}
}

public List<string> GetInstalledVersions()
{
var versions = new List<string>();

try
{
var process = new Process
{
StartInfo = new ProcessStartInfo
{
FileName = "dotnet",
Arguments = "--list-sdks",
RedirectStandardOutput = true,
UseShellExecute = false,
CreateNoWindow = true
}
};

process.Start();
var output = process.StandardOutput.ReadToEnd();
process.WaitForExit();

if (process.ExitCode == 0)
{
foreach (var line in output.Split('\n', StringSplitOptions.RemoveEmptyEntries))
{
var parts = line.Trim().Split(' ');
if (parts.Length > 0)
{
versions.Add(parts[0]);
}
}
Comment on lines +137 to +144
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

Parsing uses Split(' ') without removing empty entries. If the --list-sdks output contains multiple spaces/tabs, parts[0] can be empty, producing empty version strings in versions. Use a split that removes empty entries (and preferably handles tabs) before taking the first token.

Copilot uses AI. Check for mistakes.
}
}
catch
{
}

return versions;
}

public string GetDownloadUrl()
{
return DownloadUrlBase;
}

private string? GetSdkPath()
{
try
{
var process = new Process
{
StartInfo = new ProcessStartInfo
{
FileName = "dotnet",
Arguments = "--list-sdks",
RedirectStandardOutput = true,
UseShellExecute = false,
CreateNoWindow = true
}
};

process.Start();
var output = process.StandardOutput.ReadToEnd();
process.WaitForExit();

if (process.ExitCode == 0)
{
foreach (var line in output.Split('\n', StringSplitOptions.RemoveEmptyEntries))
{
var bracketIndex = line.IndexOf('[');
if (bracketIndex > 0)
{
var path = line[(bracketIndex + 1)..].TrimEnd(']');
if (Directory.Exists(path))
{
return path;
}
}
}
}
}
catch
{
}

return null;
}
}
}
Loading