chore(tests): Extract test utils to a new test project for reusability#28
Merged
danielpindur merged 3 commits intomasterfrom Jan 20, 2026
Merged
chore(tests): Extract test utils to a new test project for reusability#28danielpindur merged 3 commits intomasterfrom
danielpindur merged 3 commits intomasterfrom
Conversation
📝 WalkthroughWalkthroughReorganizes tests: removes Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test File
participant Helper as GeneratorTestHelper
participant Roslyn as CSharpCompilation
participant Generator as DangoGenerator
participant Driver as GeneratorDriver
Test->>Helper: RunGenerator(source)
Helper->>Roslyn: CreateCompilation(source, additionalSources)
Helper->>Driver: Create GeneratorDriver with Generator
Driver->>Generator: Run against Compilation
Driver-->>Helper: GeneratorDriverRunResult
Helper-->>Test: Return run result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
danielpindur
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@test/Dango.Tests.Utils/MappingRegistrarBuilder.cs`:
- Around line 12-22: The generated registrar emits the file-scoped namespace
before the using directives and omits System.Collections.Generic causing compile
errors; update the string construction in MappingRegistrarBuilder so the using
directives (including "using System.Collections.Generic;" and "using
Dango.Abstractions;") are written first, then append the file-scoped
namespaceDeclaration (if any), and keep the registrar class generation using
registrarClassName implementing IDangoMapperRegistrar and the
Register(IDangoMapperRegistry registry) method unchanged so Dictionary<>
references resolve.
In `@test/Dango.Unit.Tests/DangoGeneratorTests.Configuration.cs`:
- Line 151: The test contains a malformed closing sequence `}<` inside the
embedded C# source string in DangoGeneratorTests.Configuration; locate the
embedded test source (the string literal in the failing test) and replace the
stray `}<` with a proper closing brace `}` so the embedded C# is valid; ensure
the string now ends with the correct `}` and run the tests to confirm the
generator receives well-formed input.
🧹 Nitpick comments (3)
test/Dango.Tests.Utils/EnumDefinitionBuilder.cs (2)
5-21: Add basic input guards for clearer failures.
Line 5:enumNameandvalueCountare unchecked. A null/whitespace name or negative count will surface as less clear runtime errors later. Consider explicit validation for readability and debuggability.♻️ Proposed refactor
public static string BuildEnumDefinition(string enumName, int valueCount, string? namespaceName = null) { + if (string.IsNullOrWhiteSpace(enumName)) + { + throw new ArgumentException("Enum name cannot be empty.", nameof(enumName)); + } + if (valueCount < 0) + { + throw new ArgumentOutOfRangeException(nameof(valueCount), "Value count cannot be negative."); + } + var values = Enumerable.Range(0, valueCount) .Select(i => $"Value{i}") .ToList();
23-35: Guard against null/empty value lists for deterministic output.
Line 23:valuescan be null or empty, which can lead to confusing failures or empty enum bodies. Consider validating and materializing the sequence once.♻️ Proposed refactor
public static string BuildEnumDefinitionWithValues(string enumName, IEnumerable<string> values, string? namespaceName = null) { + if (string.IsNullOrWhiteSpace(enumName)) + { + throw new ArgumentException("Enum name cannot be empty.", nameof(enumName)); + } + if (values is null) + { + throw new ArgumentNullException(nameof(values)); + } + + var valueList = values.ToList(); + if (valueList.Count == 0) + { + throw new ArgumentException("Enum must contain at least one value.", nameof(values)); + } + var namespaceDeclaration = string.IsNullOrEmpty(namespaceName) ? "" : $"namespace {namespaceName};\n\n"; - var enumValues = string.Join(",\n ", values); + var enumValues = string.Join(",\n ", valueList);test/Dango.Tests.Utils/GeneratorTestHelper.cs (1)
16-22: Prefer deterministic reference assemblies over AppDomain-loaded ones.Line 16-21 can vary across test runners/load order, which makes compilation inputs non-deterministic. Consider using the trusted platform assemblies list (TPA) to build a stable reference set.
♻️ Suggested refactor (stable references)
- var references = AppDomain - .CurrentDomain.GetAssemblies() - .Where(a => !a.IsDynamic && !string.IsNullOrWhiteSpace(a.Location)) - .Select(a => MetadataReference.CreateFromFile(a.Location)) - .Cast<MetadataReference>() - .ToList(); + var tpa = ((string?)AppContext.GetData("TRUSTED_PLATFORM_ASSEMBLIES"))? + .Split(Path.PathSeparator) + .Where(p => !string.IsNullOrWhiteSpace(p)) + .Distinct() + .Select(MetadataReference.CreateFromFile) + .Cast<MetadataReference>() + .ToList() + ?? new List<MetadataReference>(); + + var references = tpa;
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@test/Dango.Tests.Utils/EnumDefinitionBuilder.cs`:
- Around line 16-31: The method BuildEnumDefinitionWithValues should guard its
values parameter to avoid a null reference when iterating; at the start of
BuildEnumDefinitionWithValues add a null-check for the values parameter and
throw ArgumentNullException(nameof(values)) if null so callers fail fast and
avoid NREs during the foreach.
🧹 Nitpick comments (1)
test/Dango.Tests.Utils/EnumDefinitionBuilder.cs (1)
7-14: Guard against negativevalueCountfor clearer failures.
Enumerable.Rangewill throw ifvalueCount < 0, but the exception isn’t very descriptive. An explicit guard improves error clarity for callers.🔧 Suggested change
public static string BuildEnumDefinition(string enumName, int valueCount, string? namespaceName = null) { + if (valueCount < 0) throw new ArgumentOutOfRangeException(nameof(valueCount)); var values = Enumerable.Range(0, valueCount) .Select(i => $"Value{i}") .ToList(); return BuildEnumDefinitionWithValues(enumName, values, namespaceName); }
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary by CodeRabbit
Tests
Chores
Note: No user-facing changes.
✏️ Tip: You can customize this high-level summary in your review settings.