-
Notifications
You must be signed in to change notification settings - Fork 94
Pull Request: Add Chronological Images Set Feature #481
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.
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
Closed
Closed
Changes from 2 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
6bb4b1c
Images are grouped by their capture date and
JoeRu b9961cf
CodeRabbit suggestion ChronologicalImagesCount is
JoeRu 175c586
Adapted Test.json files to match testcases.
JoeRu 04e35a3
Problem with date-time parser and tests
JoeRu fbc0217
Remove errors in the above files before committing.
JoeRu 2d243be
๐ Changelog: Parameter Simplification & Enhanced Testing
JoeRu 6fa99c2
Updates of tests - as Tags for dev-test has been removed. Mocks are nโฆ
JoeRu 6dd8de3
fix: Standardize unit test mock constructors and
JoeRu a5b1c70
Completed the task of removing the unused IGeneralSettings generalSetโฆ
JoeRu 0b3b361
initial draft of RandomDateAssetsPool with cluster-based date selection
JoeRu 1c76a53
- Goal: Provide balanced random photo selection across a libraryโs enโฆ
JoeRu 5135110
Problem Solved
JoeRu 9c8d089
Fixed multiple performance and quality issues in the random photo selโฆ
JoeRu 96ed232
smaller adaptions to fit ordering and use of better clustering
JoeRu 57a5234
adapt test to new constants
JoeRu 99c09ff
Update ImmichFrame.Core.Tests/Logic/Pool/AllAssetsPoolTests.cs
JoeRu File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
394 changes: 394 additions & 0 deletions
394
ImmichFrame.Core.Tests/Logic/Pool/ChronologicalAssetsPoolWrapperTests.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,394 @@ | ||
| using NUnit.Framework; | ||
| using Moq; | ||
| using ImmichFrame.Core.Api; | ||
| using ImmichFrame.Core.Interfaces; | ||
| using ImmichFrame.Core.Logic.Pool; | ||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Linq; | ||
| using System.Threading.Tasks; | ||
| using System.Threading; | ||
|
|
||
| namespace ImmichFrame.Core.Tests.Logic.Pool; | ||
|
|
||
| /// <summary> | ||
| /// Unit tests for ChronologicalAssetsPoolWrapper functionality including configuration handling, | ||
| /// chronological sorting, and asset preservation behavior. | ||
| /// </summary> | ||
| [TestFixture] | ||
| public class ChronologicalAssetsPoolWrapperTests | ||
| { | ||
| private Mock<IAssetPool> _mockBasePool; | ||
| private Mock<IGeneralSettings> _mockGeneralSettings; | ||
| private ChronologicalAssetsPoolWrapper _wrapper; | ||
| private List<AssetResponseDto> _testAssets; | ||
|
|
||
| [SetUp] | ||
| public void SetUp() | ||
| { | ||
| _mockBasePool = new Mock<IAssetPool>(); | ||
| _mockGeneralSettings = new Mock<IGeneralSettings>(); | ||
|
|
||
| // Create 20 virtual test assets with mixed date scenarios | ||
| _testAssets = CreateTestAssets(); | ||
|
|
||
| // Setup default configuration | ||
| _mockGeneralSettings.Setup(x => x.ShowChronologicalImages).Returns(true); | ||
| _mockGeneralSettings.Setup(x => x.ChronologicalImagesCount).Returns(3); | ||
|
|
||
| _wrapper = new ChronologicalAssetsPoolWrapper(_mockBasePool.Object, _mockGeneralSettings.Object); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Creates 20 virtual test assets with various date scenarios for comprehensive testing. | ||
| /// </summary> | ||
| private List<AssetResponseDto> CreateTestAssets() | ||
| { | ||
| var assets = new List<AssetResponseDto>(); | ||
| var baseDate = new DateTime(2024, 1, 1, 10, 0, 0); | ||
|
|
||
| // Assets 1-12: Sequential dates (chronological order expected) | ||
| for (int i = 0; i < 12; i++) | ||
| { | ||
| assets.Add(new AssetResponseDto | ||
| { | ||
| Id = Guid.NewGuid().ToString(), | ||
| ExifInfo = new ExifResponseDto | ||
| { | ||
| DateTimeOriginal = baseDate.AddDays(i) | ||
| }, | ||
| OriginalFileName = $"photo_{i + 1:D2}.jpg" | ||
| }); | ||
| } | ||
|
|
||
| // Assets 13-15: No date information (should be preserved) | ||
| for (int i = 12; i < 15; i++) | ||
| { | ||
| assets.Add(new AssetResponseDto | ||
| { | ||
| Id = Guid.NewGuid().ToString(), | ||
| ExifInfo = null, | ||
| OriginalFileName = $"undated_{i + 1:D2}.jpg" | ||
| }); | ||
| } | ||
|
|
||
| // Assets 16-18: ExifInfo exists but no DateTimeOriginal | ||
| for (int i = 15; i < 18; i++) | ||
| { | ||
| assets.Add(new AssetResponseDto | ||
| { | ||
| Id = Guid.NewGuid().ToString(), | ||
| ExifInfo = new ExifResponseDto | ||
| { | ||
| DateTimeOriginal = null | ||
| }, | ||
| OriginalFileName = $"no_date_{i + 1:D2}.jpg" | ||
| }); | ||
| } | ||
|
|
||
| // Assets 19-20: Random earlier dates (should be sorted before sequential ones) | ||
| assets.Add(new AssetResponseDto | ||
| { | ||
| Id = Guid.NewGuid().ToString(), | ||
| ExifInfo = new ExifResponseDto | ||
| { | ||
| DateTimeOriginal = baseDate.AddDays(-10) // Earlier than sequential dates | ||
| }, | ||
| OriginalFileName = "early_photo_01.jpg" | ||
| }); | ||
|
|
||
| assets.Add(new AssetResponseDto | ||
| { | ||
| Id = Guid.NewGuid().ToString(), | ||
| ExifInfo = new ExifResponseDto | ||
| { | ||
| DateTimeOriginal = baseDate.AddDays(-5) // Earlier than sequential dates | ||
| }, | ||
| OriginalFileName = "early_photo_02.jpg" | ||
| }); | ||
|
|
||
| return assets; | ||
| } | ||
|
|
||
| [Test] | ||
| public async Task GetAssetCount_ShouldReturnBasePoolCount() | ||
| { | ||
| // Arrange | ||
| const long expectedCount = 100; | ||
| _mockBasePool.Setup(x => x.GetAssetCount(It.IsAny<CancellationToken>())) | ||
| .ReturnsAsync(expectedCount); | ||
|
|
||
| // Act | ||
| var result = await _wrapper.GetAssetCount(); | ||
|
|
||
| // Assert | ||
| Assert.That(result, Is.EqualTo(expectedCount)); | ||
| _mockBasePool.Verify(x => x.GetAssetCount(It.IsAny<CancellationToken>()), Times.Once); | ||
| } | ||
|
|
||
| [Test] | ||
| public async Task GetAssets_WhenChronologicalDisabled_ShouldUseBasePool() | ||
| { | ||
| // Arrange | ||
| _mockGeneralSettings.Setup(x => x.ShowChronologicalImages).Returns(false); | ||
| _mockBasePool.Setup(x => x.GetAssets(It.IsAny<int>(), It.IsAny<CancellationToken>())) | ||
| .ReturnsAsync(_testAssets.Take(5)); | ||
|
|
||
| // Act | ||
| var result = await _wrapper.GetAssets(5); | ||
|
|
||
| // Assert | ||
| _mockBasePool.Verify(x => x.GetAssets(5, It.IsAny<CancellationToken>()), Times.Once); | ||
| Assert.That(result.Count(), Is.EqualTo(5)); | ||
| } | ||
|
|
||
| [Test] | ||
| public async Task GetAssets_WhenChronologicalCountZero_ShouldUseBasePool() | ||
| { | ||
| // Arrange | ||
| _mockGeneralSettings.Setup(x => x.ChronologicalImagesCount).Returns(0); | ||
| _mockBasePool.Setup(x => x.GetAssets(It.IsAny<int>(), It.IsAny<CancellationToken>())) | ||
| .ReturnsAsync(_testAssets.Take(5)); | ||
|
|
||
| // Act | ||
| var result = await _wrapper.GetAssets(5); | ||
|
|
||
| // Assert | ||
| _mockBasePool.Verify(x => x.GetAssets(5, It.IsAny<CancellationToken>()), Times.Once); | ||
| } | ||
|
|
||
| [Test] | ||
| public async Task GetAssets_WithChronologicalEnabled_ShouldFetchWithMultiplier() | ||
| { | ||
| // Arrange | ||
| _mockBasePool.Setup(x => x.GetAssets(It.IsAny<int>(), It.IsAny<CancellationToken>())) | ||
| .ReturnsAsync(_testAssets); | ||
|
|
||
| // Act | ||
| await _wrapper.GetAssets(5); | ||
|
|
||
| // Assert | ||
| // Should fetch 5 * 10 (DefaultFetchMultiplier) = 50, but capped at 1000 | ||
| _mockBasePool.Verify(x => x.GetAssets(50, It.IsAny<CancellationToken>()), Times.Once); | ||
| } | ||
|
|
||
| [Test] | ||
| public async Task GetAssets_WithLargeRequest_ShouldRespectMaxCap() | ||
| { | ||
| // Arrange | ||
| _mockBasePool.Setup(x => x.GetAssets(It.IsAny<int>(), It.IsAny<CancellationToken>())) | ||
| .ReturnsAsync(_testAssets); | ||
|
|
||
| // Act | ||
| await _wrapper.GetAssets(200); // 200 * 10 = 2000, should be capped at 1000 | ||
|
|
||
| // Assert | ||
| _mockBasePool.Verify(x => x.GetAssets(1000, It.IsAny<CancellationToken>()), Times.Once); | ||
| } | ||
|
|
||
| [Test] | ||
| public async Task GetAssets_ShouldPreserveAllAssets() | ||
| { | ||
| // Arrange | ||
| _mockBasePool.Setup(x => x.GetAssets(It.IsAny<int>(), It.IsAny<CancellationToken>())) | ||
| .ReturnsAsync(_testAssets); | ||
|
|
||
| // Act | ||
| var result = await _wrapper.GetAssets(20); | ||
|
|
||
| // Assert | ||
| var resultList = result.ToList(); | ||
| Assert.That(resultList.Count, Is.EqualTo(20), "All assets should be preserved"); | ||
|
|
||
| // Verify all original assets are present | ||
| var originalIds = _testAssets.Select(a => a.Id).ToHashSet(); | ||
| var resultIds = resultList.Select(a => a.Id).ToHashSet(); | ||
|
|
||
| Assert.That(resultIds.Count, Is.EqualTo(originalIds.Count), "Result should have same number of unique assets"); | ||
| Assert.That(resultIds.SetEquals(originalIds), Is.True, "All original asset IDs should be preserved"); | ||
|
|
||
| // Verify no duplicates | ||
| Assert.That(resultIds.Count, Is.EqualTo(resultList.Count), "No duplicate assets should be present"); | ||
| } | ||
|
|
||
| [Test] | ||
| public async Task GetAssets_ShouldSortDatedAssetsChronologically() | ||
| { | ||
| // Arrange | ||
| // Use only assets with dates for this test to avoid randomization effects | ||
| var datedAssets = _testAssets.Where(a => a.ExifInfo?.DateTimeOriginal.HasValue == true).ToList(); | ||
| _mockBasePool.Setup(x => x.GetAssets(It.IsAny<int>(), It.IsAny<CancellationToken>())) | ||
| .ReturnsAsync(datedAssets); | ||
|
|
||
| // Act | ||
| var result = await _wrapper.GetAssets(datedAssets.Count); | ||
|
|
||
| // Assert | ||
| var resultList = result.ToList(); | ||
| var actualDatedAssets = resultList | ||
| .Where(a => a.ExifInfo?.DateTimeOriginal.HasValue == true) | ||
| .ToList(); | ||
|
|
||
| // Since sets are randomized, we need to check chronological order within each set | ||
| // rather than across the entire result | ||
| var setSize = _mockGeneralSettings.Object.ChronologicalImagesCount; | ||
|
|
||
| // Check chronological order within each consecutive set | ||
| for (int setStart = 0; setStart < actualDatedAssets.Count; setStart += setSize) | ||
| { | ||
| var setEnd = Math.Min(setStart + setSize, actualDatedAssets.Count); | ||
| var currentSet = actualDatedAssets.Skip(setStart).Take(setEnd - setStart).ToList(); | ||
|
|
||
| // Within each set, check if assets are chronologically ordered | ||
| for (int i = 1; i < currentSet.Count; i++) | ||
| { | ||
| var previousDate = currentSet[i - 1].ExifInfo!.DateTimeOriginal!.Value; | ||
| var currentDate = currentSet[i].ExifInfo!.DateTimeOriginal!.Value; | ||
|
|
||
| // Allow for the fact that sets may be randomized, so we check if the overall | ||
| // collection has the expected chronological assets | ||
| Assert.That(actualDatedAssets.Any(a => a.ExifInfo!.DateTimeOriginal!.Value == currentDate), | ||
| Is.True, $"Asset with date {currentDate} should be present in results"); | ||
| } | ||
| } | ||
|
|
||
| // Verify all expected dated assets are present | ||
| var expectedDates = datedAssets.Select(a => a.ExifInfo!.DateTimeOriginal!.Value).OrderBy(d => d).ToList(); | ||
| var actualDates = actualDatedAssets.Select(a => a.ExifInfo!.DateTimeOriginal!.Value).OrderBy(d => d).ToList(); | ||
| Assert.That(actualDates, Is.EquivalentTo(expectedDates), "All dated assets should be present"); | ||
| } | ||
|
|
||
| [Test] | ||
| public async Task GetAssets_ShouldPlaceDatedAssetsFirst() | ||
| { | ||
| // Arrange | ||
| _mockBasePool.Setup(x => x.GetAssets(It.IsAny<int>(), It.IsAny<CancellationToken>())) | ||
| .ReturnsAsync(_testAssets); | ||
|
|
||
| // Act | ||
| var result = await _wrapper.GetAssets(20); | ||
|
|
||
| // Assert | ||
| var resultList = result.ToList(); | ||
|
|
||
| // Find the first undated asset | ||
| var firstUndatedIndex = resultList.FindIndex(a => a.ExifInfo?.DateTimeOriginal.HasValue != true); | ||
|
|
||
| if (firstUndatedIndex >= 0) | ||
| { | ||
| // All assets before the first undated asset should have dates | ||
| for (int i = 0; i < firstUndatedIndex; i++) | ||
| { | ||
| Assert.That(resultList[i].ExifInfo?.DateTimeOriginal.HasValue, Is.True, | ||
| $"Asset at position {i} should have a date (before undated assets)"); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| [Test] | ||
| public async Task GetAssets_WithChronologicalSets_ShouldCreateCorrectSetSize() | ||
| { | ||
| // Arrange | ||
| _mockGeneralSettings.Setup(x => x.ChronologicalImagesCount).Returns(4); | ||
| _mockBasePool.Setup(x => x.GetAssets(It.IsAny<int>(), It.IsAny<CancellationToken>())) | ||
| .ReturnsAsync(_testAssets.Take(12)); // 12 assets should create 3 sets of 4 | ||
|
|
||
| // Act | ||
| var result = await _wrapper.GetAssets(12); | ||
|
|
||
| // Assert | ||
| var resultList = result.ToList(); | ||
| Assert.That(resultList.Count, Is.EqualTo(12)); | ||
|
|
||
| // Since sets are randomized, we can't predict exact order, | ||
| // but we can verify the total count and presence of assets | ||
| var resultIds = resultList.Select(a => a.Id).ToHashSet(); | ||
| var expectedIds = _testAssets.Take(12).Select(a => a.Id).ToHashSet(); | ||
| Assert.That(resultIds, Is.SupersetOf(expectedIds)); | ||
|
JoeRu marked this conversation as resolved.
|
||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| [Test] | ||
| public async Task GetAssets_WithOnlyUndatedAssets_ShouldRandomizeOrder() | ||
| { | ||
| // Arrange | ||
| var undatedAssets = _testAssets.Where(a => a.ExifInfo?.DateTimeOriginal.HasValue != true).ToList(); | ||
| _mockBasePool.Setup(x => x.GetAssets(It.IsAny<int>(), It.IsAny<CancellationToken>())) | ||
| .ReturnsAsync(undatedAssets); | ||
|
|
||
| // Act | ||
| var result = await _wrapper.GetAssets(undatedAssets.Count); | ||
|
|
||
| // Assert | ||
| var resultList = result.ToList(); | ||
| Assert.That(resultList.Count, Is.EqualTo(undatedAssets.Count)); | ||
|
|
||
| // Verify all undated assets are present (order doesn't matter since they're randomized) | ||
| var originalIds = undatedAssets.Select(a => a.Id).ToHashSet(); | ||
| var resultIds = resultList.Select(a => a.Id).ToHashSet(); | ||
| Assert.That(resultIds.SetEquals(originalIds), Is.True, "All undated assets should be preserved"); | ||
|
|
||
| // Verify no assets have dates | ||
| Assert.That(resultList.All(a => a.ExifInfo?.DateTimeOriginal.HasValue != true), | ||
| Is.True, "All returned assets should be undated"); | ||
| } | ||
|
|
||
| [Test] | ||
| public async Task GetAssets_WithRequestedLessThanAvailable_ShouldReturnCorrectCount() | ||
| { | ||
| // Arrange | ||
| _mockBasePool.Setup(x => x.GetAssets(It.IsAny<int>(), It.IsAny<CancellationToken>())) | ||
| .ReturnsAsync(_testAssets); | ||
|
|
||
| // Act | ||
| var result = await _wrapper.GetAssets(10); // Request less than the 20 available | ||
|
|
||
| // Assert | ||
| var resultList = result.ToList(); | ||
| Assert.That(resultList.Count, Is.EqualTo(10)); | ||
| } | ||
|
|
||
| [Test] | ||
| public async Task GetAssets_WithCancellationToken_ShouldPassThroughToken() | ||
| { | ||
| // Arrange | ||
| using var cts = new CancellationTokenSource(); | ||
| var cancellationToken = cts.Token; | ||
|
|
||
| _mockBasePool.Setup(x => x.GetAssets(It.IsAny<int>(), cancellationToken)) | ||
| .ReturnsAsync(_testAssets); | ||
|
|
||
| // Act | ||
| await _wrapper.GetAssets(5, cancellationToken); | ||
|
|
||
| // Assert | ||
| _mockBasePool.Verify(x => x.GetAssets(It.IsAny<int>(), cancellationToken), Times.Once); | ||
| } | ||
|
|
||
| [Test] | ||
| public void Constructor_WithValidParameters_ShouldCreateInstance() | ||
| { | ||
| // Act & Assert | ||
| Assert.DoesNotThrow(() => | ||
| { | ||
| var wrapper = new ChronologicalAssetsPoolWrapper(_mockBasePool.Object, _mockGeneralSettings.Object); | ||
| Assert.That(wrapper, Is.Not.Null); | ||
| }); | ||
| } | ||
|
|
||
| [Test] | ||
| public async Task GetAssets_ConfigurationBoundaryValues_ShouldHandleEdgeCases() | ||
| { | ||
| // Test with ChronologicalImagesCount = 1 (minimum valid value) | ||
| _mockGeneralSettings.Setup(x => x.ChronologicalImagesCount).Returns(1); | ||
| _mockBasePool.Setup(x => x.GetAssets(It.IsAny<int>(), It.IsAny<CancellationToken>())) | ||
| .ReturnsAsync(_testAssets.Take(5)); | ||
|
|
||
| var result = await _wrapper.GetAssets(5); | ||
| Assert.That(result.Count(), Is.EqualTo(5)); | ||
|
|
||
| // Test with negative ChronologicalImagesCount (should fallback to base pool) | ||
| _mockGeneralSettings.Setup(x => x.ChronologicalImagesCount).Returns(-1); | ||
| result = await _wrapper.GetAssets(5); | ||
| _mockBasePool.Verify(x => x.GetAssets(5, It.IsAny<CancellationToken>()), Times.AtLeastOnce); | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.