Refactor and enhance test code for clarity and consistency by bechtd · Pull Request #13 · jdubar/RoyApp

bechtd · 2025-11-09T07:39:48Z

Refactor and enhance test code for clarity and consistency

Refactored exception assertions for better readability.
Updated test data structures to use arrays instead of lists, as test data is static.
Improved test assertions with precise validation and detailed messages.
Adjusted test namespaces to align with namespace of class tested.
Separated Act & Assert sections for adherence to the AAA pattern.
Added global using for FakeItEasy to simplify imports.

- Refactored exception assertions for better readability. - Updated test data structures to use arrays instead of lists, as test data is static. - Improved test assertions with precise validation and detailed messages. - Adjusted test namespaces to align with namespace of class tested. - Separated Act & Assert sections for adherence to the AAA pattern. - Added global using for `FakeItEasy` to simplify imports.

Refactor and enhance test code for clarity and consistency

bechtd

Explanation of suggestions for changes for the Test files.

Overall, the tests look good.

bechtd · 2025-11-09T07:40:19Z

RoyAppMaui.Tests/Converters/TimeSpanConverterTests.cs

 using CsvHelper.Configuration;
 using CsvHelper.TypeConversion;

-using FakeItEasy;


moved to Usings.cs as global

bechtd · 2025-11-09T07:43:55Z

RoyAppMaui.Tests/Converters/TimeSpanConverterTests.cs

-        // Act & Assert
-        Assert.Throws<TypeConverterException>(() => converter.ConvertFromString("notatime", _readerRow, _memberMapData));
+        // Act
+        object action() => converter.ConvertFromString("notatime", _readerRow, _memberMapData);
+
+        // Assert
+        var exception = Assert.Throws<TypeConverterException>(action);
+        Assert.Contains("Invalid TimeSpan format", exception.Message);


Exceptions assertions are awkward, but they can be separated from the Act portion by creating a local function, which I typically call action (Act => Action). This effectively treats your function you want to test as a variable, but doesn't invoke it yet.

The Assert.Throws does the invocation.

In addition, Assert.Throws return the exception itself, which you can further test, usually for the error message contents, which is usually a good idea to see this.

bechtd · 2025-11-09T07:46:21Z

RoyAppMaui.Tests/Extensions/SleepExtensionsTests.cs

-            new List<Sleep>
-            {
+            [
                new() { Bedtime = new TimeSpan(2, 0, 0) },
                new() { Bedtime = new TimeSpan(3, 0, 0) }
-            },
+            ],


because this is static, instead of dynamic data, we can declare this data as an array.

It's important to keep in mind that you only need a List if you are going to use its Add (or rarely, its Remove).
Arrays are a set size in memory. In the test, we have no plans to grow nor shrink the contents, so it's more efficient to use arrays.

bechtd · 2025-11-09T07:48:00Z

RoyAppMaui.Tests/Extensions/StringExtensionsTests.cs

+    [InlineData(null, "Input string is null or empty.")]
+    [InlineData("", "Input string is null or empty.")]
+    [InlineData("   ", "Input string is null or empty.")]
+    [InlineData("notatime", "String 'notatime' was not recognized as a valid DateTime.")]


this is a perfect example of why I like to see the Exception message, because occasionally you'll get variation.

bechtd · 2025-11-09T07:51:54Z

RoyAppMaui.Tests/Models/SleepTests.cs

-namespace RoyAppMaui.Tests.Models;
+namespace RoyAppMaui.Models.Tests;


you were mostly good about this in the test classes but missed this in a few of them. It's understandable because, by default Visual Studio puts Tests in the middle. However, to take advantage of the inheritance on the using, we move the .Tests to the end. I like to do it to ensure I'm targeting the correct class.

bechtd · 2025-11-09T07:53:19Z

RoyAppMaui.Tests/Models/SleepTests.cs

-        // Act & Assert
-        Assert.Equal(22.5m, sleep.BedtimeAsDecimal);
+        // Act
+        decimal actual = sleep.BedtimeAsDecimal;
+
+        // Assert
+        Assert.Equal(22.5m, actual);


I know it's extra lines and this is nitpicky, but I always strive to keep my Act and Assert separate. I want it to be very obvious the method (or in this case Property) being tested.

bechtd · 2025-11-09T07:57:01Z

RoyAppMaui.Tests/Services/DataServiceTests.cs

-        Assert.Contains("Id,Bedtime,Bedtime (as decimal),Waketime,Waketime (as decimal)", actual);
-        Assert.Contains("Bedtime Average: 0", actual);
-        Assert.Contains("Waketime Average: 0", actual);
+        Assert.Equal("""
+            Id,Bedtime,Bedtime (as decimal),Waketime,Waketime (as decimal),Duration
+            Bedtime Average: 0
+            Waketime Average: 0
+            Duration Average: 0
+
+            """, actual);


I used to do Contains a lot as well, but once raw string came out, as long as the string isn't excessively long (100s of lines), I prefer to see all of it.

With Contains, I have found that changes will happen, over time, as you make improvements, and you may not catch, right away, that you've broken your output, because Contains only verifies at a slice.

This, above, now verifies the whole thing.

Interesting 🤔

bechtd · 2025-11-09T07:59:17Z

RoyAppMaui.Tests/Services/FileServiceTests.cs

-        var actual = result.Value.ToList();
+        var actual = result.Value;


Value was already a List so you didn't need to cast it.
FluentResults uses generics so these types of castings aren't needed.

Refactored the calculation logic in `SleepExtensions` to use LINQ methods (`Sum` and `Count`) for improved readability and efficiency. Replaced the manual `foreach` loop with a more concise implementation. Added a new test case in `SleepExtensionsTests` to validate the updated logic. The test includes multiple `Waketime` values and expects the result to be `4.08m` when using the `WaketimeAsDecimal` selector.

- Simplified the implementation of the `GetAverage` method in the `SleepExtensions` class by replacing explicit null and empty checks with a single condition using `sleeps is null || !sleeps.Any()`. - Replaced manual average calculation using `Sum` and `Count` with the `Average` method for improved readability. - Retained rounding logic to ensure results are rounded to 2 decimal places.

jdubar

Thanks Dave! Appreciate the review!

jdubar · 2025-11-09T21:29:24Z

RoyAppMaui.Tests/Services/DataServiceTests.cs

-        Assert.Contains("Id,Bedtime,Bedtime (as decimal),Waketime,Waketime (as decimal)", actual);
-        Assert.Contains("Bedtime Average: 0", actual);
-        Assert.Contains("Waketime Average: 0", actual);
+        Assert.Equal("""
+            Id,Bedtime,Bedtime (as decimal),Waketime,Waketime (as decimal),Duration
+            Bedtime Average: 0
+            Waketime Average: 0
+            Duration Average: 0
+
+            """, actual);


Interesting 🤔

codecov-commenter · 2025-11-09T22:12:40Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines	Coverage Δ
RoyAppMaui/Extensions/SleepExtensions.cs	`100.00% <100.00%> (ø)`

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

bechtd · 2025-11-09T22:42:39Z

RoyAppMaui/Extensions/SleepExtensions.cs

-    {
-        if (sleeps is null)
-        {
-            return 0m;
-        }
-
-        var sum = 0m;
-        var count = 0;
-        foreach (var sleep in sleeps)
-        {
-            sum += selector(sleep);
-            count++;
-        }
-
-        return count == 0
+        => sleeps is null || !sleeps.Any()
            ? 0m
-            : decimal.Round(sum / count, 2);
-    }
+            : decimal.Round(sleeps.Average(selector), 2);


sorry about this change.

this was caused by the difficulty with using a fork.
I meant for this to be in a follow-up PR ... BUT ... here it is.

I added an extra test (wherein the average would gone beyond 2 decimal places), but this should still be equivalent to what you had, BUT use the LINQ (functional programming) library to skip "doing it manual," and getting more comfortable with the Functional programming ways of thinking:

Map (Select in C#)

Filter (Where in C#)

Fold (AKA Reduce, Aggregate in C# ... though Count, Sum, and Average or subsets/specialists of Aggregate)

Count is aggregating, over a foreach, increase a count by 1

Sum is aggregate, over a foreach, adding on the item at the index to the current value

Average is aggregate, over a foreach, wherein it is doing both Sum and Count, and then dividing

under the surface, this is doing, arguably, the same exact thing you are doing ... but these functional methods are tried and true, reliable. Once you get used to them, you write less code, and arguably have less to test ... less chance of a silly bug (loops being a common dev "dropping the ball" situation).

AND, every major language has these (Map, Filter, Fold ... usually by those exact words), so once you get used to the concept, you can pick up other languages faster

for example, I recall the first time being shown JavaScript's "lodash" library, and it suddenly hitting me "oh, this is like LINQ!" and then the greater realization hit me "RIGHT! because this is functional programming!" And suddenly I went from confused by it, to "I love this!," and I started finding and fixing existing code in our codebase that wasn't using lodash, but should have.

it's because so much of our work tends to revolve around lists (arrays). And we are doing some form of transformation on the list. Projecting it onto something else (Map), or shrinking the size of the result set (Filter), or converting the "vector" (the list/array/dimension) down to a single "scalar" value (Fold/Reduce).

Once you get used to it, there's an elegance/simplicity to it.

Love this - I really appreciate the details.

As soon as we started discussing your suggestion, I totally got it. I love seeing "the long version" of code be transformed into effectively shorthand and work EXACTLY THE SAME. So awesome.

bechtd and others added 2 commits November 9, 2025 02:33

Merge pull request #1 from bechtd/Cleanup/Tests/first-pass-on-tests

3e69eb3

Refactor and enhance test code for clarity and consistency

bechtd commented Nov 9, 2025

View reviewed changes

bechtd added 2 commits November 9, 2025 03:27

jdubar approved these changes Nov 9, 2025

View reviewed changes

bechtd commented Nov 9, 2025

View reviewed changes

jdubar assigned bechtd Nov 10, 2025

jdubar added refactor cleanup and improve code coverage added or modified coverage tests labels Nov 10, 2025

jdubar merged commit 751c67b into jdubar:master Nov 10, 2025
4 checks passed

		namespace RoyAppMaui.Tests.Models;
		namespace RoyAppMaui.Models.Tests;

		var actual = result.Value.ToList();
		var actual = result.Value;

Conversation

bechtd commented Nov 9, 2025

Uh oh!

bechtd left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jdubar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Nov 9, 2025

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants