Add test coverage for prerendering closed generic components#5
Conversation
Co-authored-by: javiercn <6995051+javiercn@users.noreply.github.com>
📝 WalkthroughWalkthroughThis pull request adds comprehensive test coverage for rendering and deserializing closed generic components in Blazor. A new generic test component ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
Author
|
@coderabbitai review |
1 similar comment
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![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 expected assembly and type name checks that reference
GenericComponent<string> to instead reference GenericComponent<int> so they
match the rendered component (replace
typeof(GenericComponent<string>).Assembly.GetName().Name and
typeof(GenericComponent<string>).FullName with the corresponding
typeof(GenericComponent<int>) checks when asserting preambleMarker.Assembly and
preambleMarker.TypeName).
In `@src/Components/Server/test/Circuits/ServerComponentDeserializerTest.cs`:
- Around line 127-145: The test
CanParseMultipleMarkersForClosedGenericComponents has an incorrect assertion for
the second descriptor's sequence: change the assertion on
secondDescriptor.Sequence to expect 1 instead of 0 so it matches the
incrementing-sequence behavior (see CanParseMultipleMarkers and the descriptors
list created by
ServerComponentDeserializer.TryDeserializeComponentDescriptorCollection).
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: GenericComponent<string> should be GenericComponent<int>.
The test renders GenericComponent<int> (line 825) but asserts against GenericComponent<string> (lines 837-838). This will cause the test to fail because typeof(GenericComponent<int>).FullName differs from typeof(GenericComponent<string>).FullName.
🐛 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);🤖 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 expected assembly and type name
checks that reference GenericComponent<string> to instead reference
GenericComponent<int> so they match the rendered component (replace
typeof(GenericComponent<string>).Assembly.GetName().Name and
typeof(GenericComponent<string>).FullName with the corresponding
typeof(GenericComponent<int>) checks when asserting preambleMarker.Assembly and
preambleMarker.TypeName).
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 second descriptor.
Line 144 asserts Assert.Equal(0, secondDescriptor.Sequence), but when parsing multiple markers, each should have an incrementing sequence number. Comparing with CanParseMultipleMarkers (line 164), the second descriptor should have sequence 1.
🐛 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 has an incorrect assertion for
the second descriptor's sequence: change the assertion on
secondDescriptor.Sequence to expect 1 instead of 0 so it matches the
incrementing-sequence behavior (see CanParseMultipleMarkers and the descriptors
list created by
ServerComponentDeserializer.TryDeserializeComponentDescriptorCollection).
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
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