Skip to content

Add .NET 9.0 to app and tests; set roll-forward; update Rider config #20

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

Merged
yesseruser merged 7 commits into from
Oct 12, 2025
Merged

Add .NET 9.0 to app and tests; set roll-forward; update Rider config #20

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f7cd660
Add .NET 9.0 to target frameworks and allow major version roll-forward
yesseruser Apr 22, 2025
67119f3
Auto-generated project settings
yesseruser Apr 22, 2025
88cf4fc
Merge branch 'main' into maintenance
yesseruser Oct 12, 2025
98e65ce
Do not use appdata directory for tests
yesseruser Oct 12, 2025
cdd1537
Cleanup proper folder
yesseruser Oct 12, 2025
5f8b5f0
Restore AppData directory cleanup
yesseruser Oct 12, 2025
07f2956
Wrap cleanup method with try block
yesseruser Oct 12, 2025
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
2 changes: 2 additions & 0 deletions .idea/.idea.yTools/.idea/projectSettingsUpdater.xml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion yTools.ConsoleTryout/yTools.ConsoleApp.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
<ImplicitUsings>enable</ImplicitUsings>
<Nullable>enable</Nullable>
<ReleaseVersion>0.2.2</ReleaseVersion>
<TargetFrameworks>net7.0;net8.0</TargetFrameworks>
<TargetFrameworks>net7.0;net8.0;net9.0</TargetFrameworks>
<RollForward>LatestMajor</RollForward>
</PropertyGroup>

<ItemGroup>
Expand Down
49 changes: 22 additions & 27 deletions yTools.Tests/SerializationTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,29 @@
namespace yTools.Tests
{
[TestClass]
public class SerializationTests
public class SerializationTests
{
private readonly char _sep = General.PathSeparator;

[TestCleanup]
public void Cleanup()
{
const string testDirectory = "yTools";

var fullDir = $"{Environment.GetFolderPath(
Environment.SpecialFolder.LocalApplicationData
)}{_sep}{testDirectory}";
if (Directory.Exists(fullDir))
Directory.Delete(fullDir, true);
try
{
const string testDirectory = "yTools";
var fullDir = $"{Environment.GetFolderPath(
Environment.SpecialFolder.LocalApplicationData
)}{_sep}{testDirectory}";
if (Directory.Exists(fullDir))
Directory.Delete(fullDir, true);

if (Directory.Exists("./test-serialization"))
Directory.Delete("./test-serialization", true);
}
catch
{
Console.WriteLine("Something went wrong when cleaning up.");
}
Comment on lines +11 to +26

@coderabbitai coderabbitai Bot Oct 12, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Implement retry logic for locked file handles.

The try-catch wrapper silences cleanup failures but doesn't address the locked file handle issue flagged in past reviews. Pipeline failures indicate Directory.Delete fails with "Directory not empty" errors, likely due to file handles remaining open after tests complete.

Consider implementing retry logic with backoff as suggested in previous reviews:

 [TestCleanup]
 public void Cleanup()
 {
-  try
-  {
-      const string testDirectory = "yTools";
-      var fullDir = $"{Environment.GetFolderPath(
-          Environment.SpecialFolder.LocalApplicationData
-      )}{_sep}{testDirectory}";
-      if (Directory.Exists(fullDir))
-          Directory.Delete(fullDir, true);
+    const string testDirectory = "yTools";
+    var fullDir = $"{Environment.GetFolderPath(
+        Environment.SpecialFolder.LocalApplicationData
+    )}{_sep}{testDirectory}";
+    
+    TryDeleteDirectory(fullDir);
+    TryDeleteDirectory("./test-serialization");
+}
+
+private static void TryDeleteDirectory(string path, int maxAttempts = 3)
+{
+    if (!Directory.Exists(path)) return;
+    
+    for (int attempt = 0; attempt < maxAttempts; attempt++)
+    {
+        try
+        {
+            Directory.Delete(path, true);
+            return;
+        }
+        catch (IOException) when (attempt < maxAttempts - 1)
+        {
+            Thread.Sleep(100);
+        }
+        catch (UnauthorizedAccessException) when (attempt < maxAttempts - 1)
+        {
+            Thread.Sleep(100);
+        }
+    }
+    
+    // Log failure after all retries
+    Console.WriteLine($"Failed to delete directory: {path}");
+}

Alternatively, ensure serializers in CheckSerializationDirCreated are properly disposed using using statements if they implement IDisposable.

Based on past review comments.

🏁 Script executed:

#!/bin/bash
# Locate the test file
TEST_FILE=$(find . -type f -name 'SerializationTests.cs')
echo "Found file: $TEST_FILE"
# Show lines around the TestCleanup method
rg -nC3 '\[TestCleanup\]' "$TEST_FILE"
# Show the full Cleanup method
sed -n '1,200p' "$TEST_FILE"

Length of output: 6160

🏁 Script executed:

#!/bin/bash
# Find JsonSerializer definition
rg -nP 'class\s+JsonSerializer' -C3 .

# Find XmlSerializer definition
rg -nP 'class\s+XmlSerializer' -C3 .

Length of output: 799

Replace broad try-catch in Cleanup with retry logic for Directory.Delete
Implement a helper like TryDeleteDirectory(path, maxAttempts) that retries on IOException/UnauthorizedAccessException with a short backoff, and call it for both cleanup paths instead of silencing failures:

private static void TryDeleteDirectory(string path, int maxAttempts = 3)
{
    if (!Directory.Exists(path)) return;
    for (int i = 1; i <= maxAttempts; i++)
    {
        try { Directory.Delete(path, true); return; }
        catch (IOException) when (i < maxAttempts) { Thread.Sleep(100); }
        catch (UnauthorizedAccessException) when (i < maxAttempts) { Thread.Sleep(100); }
    }
    Console.WriteLine($"Failed to delete directory after retries: {path}");
}

Call TryDeleteDirectory(fullDir) and TryDeleteDirectory("./test-serialization") in Cleanup.

🤖 Prompt for AI Agents 
In yTools.Tests/SerializationTests.cs around lines 11 to 26, replace the broad
try-catch that swallows all exceptions during directory cleanup with a retrying
helper: add a private static TryDeleteDirectory(string path, int maxAttempts =
3) that returns early if the path doesn't exist and attempts
Directory.Delete(path, true) up to maxAttempts, catching IOException and
UnauthorizedAccessException and retrying after a short Thread.Sleep backoff when
i < maxAttempts, and logging a failure message if all attempts fail; then call
TryDeleteDirectory(fullDir) and TryDeleteDirectory("./test-serialization") from
the Cleanup block instead of the existing try-catch.

}

[TestMethod]
Expand All @@ -24,14 +33,7 @@ public void CheckObjectsJsonSerialized()
TestSerializationObject testObject2 = new("Good Morning.", -8485, -6498.948);

JsonSerializer serializer = new();

var directory = $"yTools{_sep}Tests{_sep}Serialization";
serializer.SetSerializationDirectoryInLocalAppData(directory);

Assert.IsTrue(Directory.Exists(
$"{Environment.GetFolderPath(
Environment.SpecialFolder.LocalApplicationData
)}{_sep}{directory}"));
serializer.SetSerializationDirectory("./test-serialization");
Comment thread
coderabbitai[bot] marked this conversation as resolved.

bool serialized = serializer.SerializeInDefault("testObject1.json", testObject1, out Exception? exception, out _);
if (!serialized && exception != null) throw exception;
Expand All @@ -56,14 +58,7 @@ public void CheckObjectsXmlSerialized()
TestSerializationObject testObject2 = new("Good Morning.", -8485, -6498.948);

XmlSerializer serializer = new();

var directory = $"yTools{_sep}Tests{_sep}Serialization";
serializer.SetSerializationDirectoryInLocalAppData(directory);

Assert.IsTrue(Directory.Exists(
$"{Environment.GetFolderPath(
Environment.SpecialFolder.LocalApplicationData
)}{_sep}{directory}"));
serializer.SetSerializationDirectory("./test-serialization");

bool serialized = serializer.SerializeInDefault("testObject1.xml", testObject1, out Exception? exception, out _);
if (!serialized && exception != null) throw exception;
Expand Down Expand Up @@ -92,7 +87,7 @@ public void CheckSerializationDirCreated()

jsonSerializer.Serialize(testObj, "json.json", $@"{baseDir}{_sep}json", out _, out _);
Assert.IsTrue(Directory.Exists($"{baseDir}{_sep}json"));
Comment on lines 88 to 89

@coderabbitai coderabbitai Bot Oct 12, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Check serialization results to diagnose the pipeline failure.

Lines 81 and 84 discard the serialization return value and exception with out _, so if serialization fails, the test won't detect it until the assertion at line 82—which is currently failing in CI. Other tests in this file (e.g., CheckObjectsJsonSerialized) properly check both the return value and exception.

Apply this diff to detect serialization failures:

-    jsonSerializer.Serialize(testObj, "json.json", $@"{baseDir}{_sep}json", out _, out _);
+    var jsonSuccess = jsonSerializer.Serialize(testObj, "json.json", $@"{baseDir}{_sep}json", out var jsonEx, out _);
+    if (!jsonSuccess && jsonEx != null) throw jsonEx;
     Assert.IsTrue(Directory.Exists($"{baseDir}{_sep}json"));

-    xmlSerializer.Serialize(testObj, "xml.xml", $@"{baseDir}{_sep}xml", out _, out _);
+    var xmlSuccess = xmlSerializer.Serialize(testObj, "xml.xml", $@"{baseDir}{_sep}xml", out var xmlEx, out _);
+    if (!xmlSuccess && xmlEx != null) throw xmlEx;
     Assert.IsTrue(Directory.Exists($"{baseDir}{_sep}xml"));

This change will surface the root cause of the failed assertion and help resolve the pipeline failure.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
jsonSerializer.Serialize(testObj, "json.json", $@"{baseDir}{_sep}json", out _, out _);
Assert.IsTrue(Directory.Exists($"{baseDir}{_sep}json"));
var jsonSuccess = jsonSerializer.Serialize(
testObj,
"json.json",
$@"{baseDir}{_sep}json",
out var jsonEx,
out _
);
if (!jsonSuccess && jsonEx != null) throw jsonEx;
Assert.IsTrue(Directory.Exists($"{baseDir}{_sep}json"));
var xmlSuccess = xmlSerializer.Serialize(
testObj,
"xml.xml",
$@"{baseDir}{_sep}xml",
out var xmlEx,
out _
);
if (!xmlSuccess && xmlEx != null) throw xmlEx;
Assert.IsTrue(Directory.Exists($"{baseDir}{_sep}xml"));
🧰 Tools
🪛 GitHub Actions: .NET

[error] 82-82: Assert.IsTrue failed in CheckSerializationDirCreated().

🤖 Prompt for AI Agents 
In yTools.Tests/SerializationTests.cs around lines 81-82, the call to
jsonSerializer.Serialize discards its return value and exception (uses out _),
so serialization failures are hidden until the Directory.Exists assertion;
capture the method outputs instead and assert they indicate success: store the
bool return and the exception out parameter into variables,
Assert.IsTrue(returnValue) and Assert.IsNull(exception) (then keep the directory
existence check). This will surface serialization errors immediately and help
diagnose the CI failure.


xmlSerializer.Serialize(testObj, "xml.xml", $@"{baseDir}{_sep}xml", out _, out _);
Assert.IsTrue(Directory.Exists($"{baseDir}{_sep}xml"));
}
Expand Down Expand Up @@ -131,4 +126,4 @@ public void CheckSerializeException()
Console.WriteLine($"Type of XML exception: {ex.GetType()}");
}
}
}
}
3 changes: 2 additions & 1 deletion yTools.Tests/yTools.Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
<IsPackable>false</IsPackable>
<IsTestProject>true</IsTestProject>
<ReleaseVersion>0.2.2</ReleaseVersion>
<TargetFrameworks>net7.0;net8.0</TargetFrameworks>
<TargetFrameworks>net7.0;net8.0;net9.0</TargetFrameworks>
<RollForward>LatestMajor</RollForward>
</PropertyGroup>

<ItemGroup>
Expand Down
Loading