Add test coverage for prerendering closed generic components #30
Conversation
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
Co-authored-by: javiercn <6995051+javiercn@users.noreply.github.com>
WalkthroughTest coverage is expanded for closed generic Blazor components across rendering and deserialization scenarios. A new generic test component Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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
Comment |
![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 `@src/Components/Endpoints/test/EndpointHtmlRendererTest.cs`:
- Around line 816-846: The assertions in
CanPrerender_ClosedGenericComponent_ClientMode are checking the wrong generic
type: change the checks that reference GenericComponent<string> to
GenericComponent<int> so they match the rendered component (rendered via
renderer.PrerenderComponentAsync(typeof(GenericComponent<int>))). Specifically
update the two assertions that compare preambleMarker.Assembly and
preambleMarker.TypeName to use
typeof(GenericComponent<int>).Assembly.GetName().Name and
typeof(GenericComponent<int>).FullName respectively, leaving the rest of the
test (parameter creation, content/assertions, and epilogue comparison)
unchanged.
In `@src/Components/Server/test/Circuits/ServerComponentDeserializerTest.cs`:
- Around line 127-145: The test
CanParseMultipleMarkersForClosedGenericComponents asserts the wrong Sequence for
the second descriptor; when deserializing multiple markers via
TryDeserializeComponentDescriptorCollection the second entry should have
Sequence == 1. Update the assertion on secondDescriptor.Sequence in the test to
expect 1 (matching the existing CanParseMultipleMarkers behavior) so the test
verifies the correct sequence ordering for the descriptors collection.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/Components/Endpoints/test/EndpointHtmlRendererTest.cssrc/Components/Endpoints/test/TestComponents/GenericComponent.razorsrc/Components/Server/test/Circuits/ServerComponentDeserializerTest.cs
🔇 Additional comments (4)
src/Components/Endpoints/test/TestComponents/GenericComponent.razor (1)
1-6: LGTM!This is a well-structured generic test component. The null-coalescing pattern correctly handles both reference types (where
Valuecan be null) and value types (whereToString()always returns a value).src/Components/Server/test/Circuits/ServerComponentDeserializerTest.cs (2)
79-125: LGTM!The new tests for closed generic component deserialization follow established patterns and provide good coverage for:
- Basic closed generic types (
GenericTestComponent<int>,GenericTestComponent<string>)- Closed generic types with parameters
The use of
Convert.ToInt64withCultureInfo.InvariantCulturecorrectly handles the JSON deserialization behavior where numeric types may vary.
589-597: LGTM!The
GenericTestComponent<T>follows the same pattern as the existing test components (TestComponent,DynamicallyAddedComponent) and appropriately supports testing closed generic component deserialization scenarios.src/Components/Endpoints/test/EndpointHtmlRendererTest.cs (1)
721-814: LGTM!These three test methods provide comprehensive coverage for closed generic components:
CanRender_ClosedGenericComponent: Static rendering with value type parameterCanRender_ClosedGenericComponent_ServerMode: Server mode with string parameter and descriptor verificationCanPrerender_ClosedGenericComponent_ServerMode: Server prerendering with marker and content verificationThe tests follow established patterns and correctly verify component type names, parameter definitions, and rendered content.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Comment on lines
+816
to
+846
| [Fact] | ||
| public async Task CanPrerender_ClosedGenericComponent_ClientMode() | ||
| { | ||
| // Arrange | ||
| var httpContext = GetHttpContext(); | ||
| var writer = new StringWriter(); | ||
|
|
||
| // Act | ||
| var parameters = ParameterView.FromDictionary(new Dictionary<string, object> { { "Value", 456 } }); | ||
| var result = await renderer.PrerenderComponentAsync(httpContext, typeof(GenericComponent<int>), RenderMode.InteractiveWebAssembly, parameters); | ||
| await renderer.Dispatcher.InvokeAsync(() => result.WriteTo(writer, HtmlEncoder.Default)); | ||
| var content = writer.ToString(); | ||
| content = AssertAndStripWebAssemblyOptions(content); | ||
| var match = Regex.Match(content, PrerenderedComponentPattern, RegexOptions.Multiline); | ||
|
|
||
| // Assert | ||
| Assert.True(match.Success); | ||
| var preamble = match.Groups["preamble"].Value; | ||
| var preambleMarker = JsonSerializer.Deserialize<ComponentMarker>(preamble, ServerComponentSerializationSettings.JsonSerializationOptions); | ||
| Assert.NotNull(preambleMarker.PrerenderId); | ||
| Assert.Equal("webassembly", preambleMarker.Type); | ||
| Assert.Equal(typeof(GenericComponent<string>).Assembly.GetName().Name, preambleMarker.Assembly); | ||
| Assert.Equal(typeof(GenericComponent<string>).FullName, preambleMarker.TypeName); | ||
|
|
||
| var prerenderedContent = match.Groups["content"].Value; | ||
| Assert.Equal("<p>Generic value: 456</p>", prerenderedContent); | ||
|
|
||
| var epilogue = match.Groups["epilogue"].Value; | ||
| var epilogueMarker = JsonSerializer.Deserialize<ComponentMarker>(epilogue, ServerComponentSerializationSettings.JsonSerializationOptions); | ||
| Assert.Equal(preambleMarker.PrerenderId, epilogueMarker.PrerenderId); | ||
| } |
There was a problem hiding this comment.
Type mismatch in assertions: Using GenericComponent<string> instead of GenericComponent<int>.
The test renders GenericComponent<int> with Value = 456 (line 824-825), but the assertions at lines 837-838 incorrectly check against GenericComponent<string>. This appears to be a copy-paste error from the previous test.
🐛 Proposed fix
var preambleMarker = JsonSerializer.Deserialize<ComponentMarker>(preamble, ServerComponentSerializationSettings.JsonSerializationOptions);
Assert.NotNull(preambleMarker.PrerenderId);
Assert.Equal("webassembly", preambleMarker.Type);
- Assert.Equal(typeof(GenericComponent<string>).Assembly.GetName().Name, preambleMarker.Assembly);
- Assert.Equal(typeof(GenericComponent<string>).FullName, preambleMarker.TypeName);
+ Assert.Equal(typeof(GenericComponent<int>).Assembly.GetName().Name, preambleMarker.Assembly);
+ Assert.Equal(typeof(GenericComponent<int>).FullName, preambleMarker.TypeName);📝 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
| [Fact] | |
| public async Task CanPrerender_ClosedGenericComponent_ClientMode() | |
| { | |
| // Arrange | |
| var httpContext = GetHttpContext(); | |
| var writer = new StringWriter(); | |
| // Act | |
| var parameters = ParameterView.FromDictionary(new Dictionary<string, object> { { "Value", 456 } }); | |
| var result = await renderer.PrerenderComponentAsync(httpContext, typeof(GenericComponent<int>), RenderMode.InteractiveWebAssembly, parameters); | |
| await renderer.Dispatcher.InvokeAsync(() => result.WriteTo(writer, HtmlEncoder.Default)); | |
| var content = writer.ToString(); | |
| content = AssertAndStripWebAssemblyOptions(content); | |
| var match = Regex.Match(content, PrerenderedComponentPattern, RegexOptions.Multiline); | |
| // Assert | |
| Assert.True(match.Success); | |
| var preamble = match.Groups["preamble"].Value; | |
| var preambleMarker = JsonSerializer.Deserialize<ComponentMarker>(preamble, ServerComponentSerializationSettings.JsonSerializationOptions); | |
| Assert.NotNull(preambleMarker.PrerenderId); | |
| Assert.Equal("webassembly", preambleMarker.Type); | |
| Assert.Equal(typeof(GenericComponent<string>).Assembly.GetName().Name, preambleMarker.Assembly); | |
| Assert.Equal(typeof(GenericComponent<string>).FullName, preambleMarker.TypeName); | |
| var prerenderedContent = match.Groups["content"].Value; | |
| Assert.Equal("<p>Generic value: 456</p>", prerenderedContent); | |
| var epilogue = match.Groups["epilogue"].Value; | |
| var epilogueMarker = JsonSerializer.Deserialize<ComponentMarker>(epilogue, ServerComponentSerializationSettings.JsonSerializationOptions); | |
| Assert.Equal(preambleMarker.PrerenderId, epilogueMarker.PrerenderId); | |
| } | |
| [Fact] | |
| public async Task CanPrerender_ClosedGenericComponent_ClientMode() | |
| { | |
| // Arrange | |
| var httpContext = GetHttpContext(); | |
| var writer = new StringWriter(); | |
| // Act | |
| var parameters = ParameterView.FromDictionary(new Dictionary<string, object> { { "Value", 456 } }); | |
| var result = await renderer.PrerenderComponentAsync(httpContext, typeof(GenericComponent<int>), RenderMode.InteractiveWebAssembly, parameters); | |
| await renderer.Dispatcher.InvokeAsync(() => result.WriteTo(writer, HtmlEncoder.Default)); | |
| var content = writer.ToString(); | |
| content = AssertAndStripWebAssemblyOptions(content); | |
| var match = Regex.Match(content, PrerenderedComponentPattern, RegexOptions.Multiline); | |
| // Assert | |
| Assert.True(match.Success); | |
| var preamble = match.Groups["preamble"].Value; | |
| var preambleMarker = JsonSerializer.Deserialize<ComponentMarker>(preamble, ServerComponentSerializationSettings.JsonSerializationOptions); | |
| Assert.NotNull(preambleMarker.PrerenderId); | |
| Assert.Equal("webassembly", preambleMarker.Type); | |
| Assert.Equal(typeof(GenericComponent<int>).Assembly.GetName().Name, preambleMarker.Assembly); | |
| Assert.Equal(typeof(GenericComponent<int>).FullName, preambleMarker.TypeName); | |
| var prerenderedContent = match.Groups["content"].Value; | |
| Assert.Equal("<p>Generic value: 456</p>", prerenderedContent); | |
| var epilogue = match.Groups["epilogue"].Value; | |
| var epilogueMarker = JsonSerializer.Deserialize<ComponentMarker>(epilogue, ServerComponentSerializationSettings.JsonSerializationOptions); | |
| Assert.Equal(preambleMarker.PrerenderId, epilogueMarker.PrerenderId); | |
| } |
🤖 Prompt for AI Agents
In `@src/Components/Endpoints/test/EndpointHtmlRendererTest.cs` around lines 816 -
846, The assertions in CanPrerender_ClosedGenericComponent_ClientMode are
checking the wrong generic type: change the checks that reference
GenericComponent<string> to GenericComponent<int> so they match the rendered
component (rendered via
renderer.PrerenderComponentAsync(typeof(GenericComponent<int>))). Specifically
update the two assertions that compare preambleMarker.Assembly and
preambleMarker.TypeName to use
typeof(GenericComponent<int>).Assembly.GetName().Name and
typeof(GenericComponent<int>).FullName respectively, leaving the rest of the
test (parameter creation, content/assertions, and epilogue comparison)
unchanged.
Comment on lines
+127
to
+145
| [Fact] | ||
| public void CanParseMultipleMarkersForClosedGenericComponents() | ||
| { | ||
| // Arrange | ||
| var markers = SerializeMarkers(CreateMarkers(typeof(GenericTestComponent<int>), typeof(GenericTestComponent<string>))); | ||
| var serverComponentDeserializer = CreateServerComponentDeserializer(); | ||
|
|
||
| // Act & assert | ||
| Assert.True(serverComponentDeserializer.TryDeserializeComponentDescriptorCollection(markers, out var descriptors)); | ||
| Assert.Equal(2, descriptors.Count); | ||
|
|
||
| var firstDescriptor = descriptors[0]; | ||
| Assert.Equal(typeof(GenericTestComponent<int>).FullName, firstDescriptor.ComponentType.FullName); | ||
| Assert.Equal(0, firstDescriptor.Sequence); | ||
|
|
||
| var secondDescriptor = descriptors[1]; | ||
| Assert.Equal(typeof(GenericTestComponent<string>).FullName, secondDescriptor.ComponentType.FullName); | ||
| Assert.Equal(0, secondDescriptor.Sequence); | ||
| } |
There was a problem hiding this comment.
Incorrect sequence assertion for the second descriptor.
Looking at the existing test CanParseMultipleMarkers (lines 147-165), the second descriptor should have Sequence = 1, not 0. The current assertion at line 144 appears to be incorrect and will cause the test to fail or is testing incorrect behavior.
🐛 Proposed fix
var secondDescriptor = descriptors[1];
Assert.Equal(typeof(GenericTestComponent<string>).FullName, secondDescriptor.ComponentType.FullName);
- Assert.Equal(0, secondDescriptor.Sequence);
+ Assert.Equal(1, secondDescriptor.Sequence);📝 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
| [Fact] | |
| public void CanParseMultipleMarkersForClosedGenericComponents() | |
| { | |
| // Arrange | |
| var markers = SerializeMarkers(CreateMarkers(typeof(GenericTestComponent<int>), typeof(GenericTestComponent<string>))); | |
| var serverComponentDeserializer = CreateServerComponentDeserializer(); | |
| // Act & assert | |
| Assert.True(serverComponentDeserializer.TryDeserializeComponentDescriptorCollection(markers, out var descriptors)); | |
| Assert.Equal(2, descriptors.Count); | |
| var firstDescriptor = descriptors[0]; | |
| Assert.Equal(typeof(GenericTestComponent<int>).FullName, firstDescriptor.ComponentType.FullName); | |
| Assert.Equal(0, firstDescriptor.Sequence); | |
| var secondDescriptor = descriptors[1]; | |
| Assert.Equal(typeof(GenericTestComponent<string>).FullName, secondDescriptor.ComponentType.FullName); | |
| Assert.Equal(0, secondDescriptor.Sequence); | |
| } | |
| [Fact] | |
| public void CanParseMultipleMarkersForClosedGenericComponents() | |
| { | |
| // Arrange | |
| var markers = SerializeMarkers(CreateMarkers(typeof(GenericTestComponent<int>), typeof(GenericTestComponent<string>))); | |
| var serverComponentDeserializer = CreateServerComponentDeserializer(); | |
| // Act & assert | |
| Assert.True(serverComponentDeserializer.TryDeserializeComponentDescriptorCollection(markers, out var descriptors)); | |
| Assert.Equal(2, descriptors.Count); | |
| var firstDescriptor = descriptors[0]; | |
| Assert.Equal(typeof(GenericTestComponent<int>).FullName, firstDescriptor.ComponentType.FullName); | |
| Assert.Equal(0, firstDescriptor.Sequence); | |
| var secondDescriptor = descriptors[1]; | |
| Assert.Equal(typeof(GenericTestComponent<string>).FullName, secondDescriptor.ComponentType.FullName); | |
| Assert.Equal(1, secondDescriptor.Sequence); | |
| } |
🤖 Prompt for AI Agents
In `@src/Components/Server/test/Circuits/ServerComponentDeserializerTest.cs`
around lines 127 - 145, The test
CanParseMultipleMarkersForClosedGenericComponents asserts the wrong Sequence for
the second descriptor; when deserializing multiple markers via
TryDeserializeComponentDescriptorCollection the second entry should have
Sequence == 1. Update the assertion on secondDescriptor.Sequence in the test to
expect 1 (matching the existing CanParseMultipleMarkers behavior) so the test
verifies the correct sequence ordering for the descriptors collection.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Benchmark PR from agentic-review-benchmarks#5
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.