Add test coverage for prerendering closed generic components#18
Conversation
Co-authored-by: javiercn <6995051+javiercn@users.noreply.github.com>
Code Review by Qodo
1. Block-scoped namespace used
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+10
to
+13
| namespace Microsoft.AspNetCore.Components.Server.Circuits | ||
| { | ||
| public class ServerComponentDeserializerTest | ||
| { |
There was a problem hiding this comment.
1. Block-scoped namespace used 📘 Rule violation ✓ Correctness
• The file uses a traditional block-scoped namespace declaration instead of a file-scoped namespace. • This violates the required modern C# convention and can cause inconsistent formatting across the repo. • Converting to file-scoped namespace restores consistency and reduces indentation.
Agent prompt
## Issue description
`ServerComponentDeserializerTest.cs` uses a block-scoped namespace, but the repo requires file-scoped namespaces.
## Issue Context
Compliance requires `namespace X;` syntax (no braces) for C# files.
## Fix Focus Areas
- src/Components/Server/test/Circuits/ServerComponentDeserializerTest.cs[10-25]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| var markers = SerializeMarkers(CreateMarkers(typeof(GenericTestComponent<int>))); | ||
| var serverComponentDeserializer = CreateServerComponentDeserializer(); | ||
|
|
||
| // Act & assert |
There was a problem hiding this comment.
2. Nonstandard act & assert 📘 Rule violation ✓ Correctness
• Several new tests use the combined AAA delimiter comment // Act & assert, which does not match the required standard comment format. • This inconsistency reduces test readability and makes it harder to quickly scan test phases across the codebase.
Agent prompt
## Issue description
New tests use `// Act & assert` instead of the standardized AAA delimiter format.
## Issue Context
The repository requires consistent AAA comments (including consistent capitalization when using the combined `Act & Assert` form).
## Fix Focus Areas
- src/Components/Server/test/Circuits/ServerComponentDeserializerTest.cs[79-145]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+722
to
+817
| public async Task CanRender_ClosedGenericComponent() | ||
| { | ||
| // Arrange | ||
| var httpContext = GetHttpContext(); | ||
| var writer = new StringWriter(); | ||
|
|
||
| // Act | ||
| var parameters = ParameterView.FromDictionary(new Dictionary<string, object> { { "Value", 42 } }); | ||
| var result = await renderer.PrerenderComponentAsync(httpContext, typeof(GenericComponent<int>), null, parameters); | ||
| await renderer.Dispatcher.InvokeAsync(() => result.WriteTo(writer, HtmlEncoder.Default)); | ||
| var content = writer.ToString(); | ||
|
|
||
| // Assert | ||
| Assert.Equal("<p>Generic value: 42</p>", content); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task CanRender_ClosedGenericComponent_ServerMode() | ||
| { | ||
| // Arrange | ||
| var httpContext = GetHttpContext(); | ||
| var protector = _dataprotectorProvider.CreateProtector(ServerComponentSerializationSettings.DataProtectionProviderPurpose) | ||
| .ToTimeLimitedDataProtector(); | ||
|
|
||
| // Act | ||
| var parameters = ParameterView.FromDictionary(new Dictionary<string, object> { { "Value", "TestString" } }); | ||
| var result = await renderer.PrerenderComponentAsync(httpContext, typeof(GenericComponent<string>), new InteractiveServerRenderMode(false), parameters); | ||
| var content = await renderer.Dispatcher.InvokeAsync(() => HtmlContentToString(result)); | ||
| var match = Regex.Match(content, ComponentPattern); | ||
|
|
||
| // Assert | ||
| Assert.True(match.Success); | ||
| var marker = JsonSerializer.Deserialize<ComponentMarker>(match.Groups[1].Value, ServerComponentSerializationSettings.JsonSerializationOptions); | ||
| Assert.Equal(0, marker.Sequence); | ||
| Assert.Null(marker.PrerenderId); | ||
| Assert.NotNull(marker.Descriptor); | ||
| Assert.Equal("server", marker.Type); | ||
|
|
||
| var unprotectedServerComponent = protector.Unprotect(marker.Descriptor); | ||
| var serverComponent = JsonSerializer.Deserialize<ServerComponent>(unprotectedServerComponent, ServerComponentSerializationSettings.JsonSerializationOptions); | ||
| Assert.Equal(0, serverComponent.Sequence); | ||
| Assert.Equal(typeof(GenericComponent<string>).Assembly.GetName().Name, serverComponent.AssemblyName); | ||
| Assert.Equal(typeof(GenericComponent<string>).FullName, serverComponent.TypeName); | ||
| Assert.NotEqual(Guid.Empty, serverComponent.InvocationId); | ||
|
|
||
| var parameterDefinition = Assert.Single(serverComponent.ParameterDefinitions); | ||
| Assert.Equal("Value", parameterDefinition.Name); | ||
| Assert.Equal("System.String", parameterDefinition.TypeName); | ||
| Assert.Equal("System.Private.CoreLib", parameterDefinition.Assembly); | ||
|
|
||
| var value = Assert.Single(serverComponent.ParameterValues); | ||
| var rawValue = Assert.IsType<JsonElement>(value); | ||
| Assert.Equal("TestString", rawValue.GetString()); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task CanPrerender_ClosedGenericComponent_ServerMode() | ||
| { | ||
| // Arrange | ||
| var httpContext = GetHttpContext(); | ||
| var protector = _dataprotectorProvider.CreateProtector(ServerComponentSerializationSettings.DataProtectionProviderPurpose) | ||
| .ToTimeLimitedDataProtector(); | ||
|
|
||
| // Act | ||
| var parameters = ParameterView.FromDictionary(new Dictionary<string, object> { { "Value", 123 } }); | ||
| var result = await renderer.PrerenderComponentAsync(httpContext, typeof(GenericComponent<int>), RenderMode.InteractiveServer, parameters); | ||
| var content = await renderer.Dispatcher.InvokeAsync(() => HtmlContentToString(result)); | ||
| 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.Equal(0, preambleMarker.Sequence); | ||
| Assert.NotNull(preambleMarker.PrerenderId); | ||
| Assert.NotNull(preambleMarker.Descriptor); | ||
| Assert.Equal("server", preambleMarker.Type); | ||
|
|
||
| var unprotectedServerComponent = protector.Unprotect(preambleMarker.Descriptor); | ||
| var serverComponent = JsonSerializer.Deserialize<ServerComponent>(unprotectedServerComponent, ServerComponentSerializationSettings.JsonSerializationOptions); | ||
| Assert.NotEqual(default, serverComponent); | ||
| Assert.Equal(0, serverComponent.Sequence); | ||
| Assert.Equal(typeof(GenericComponent<int>).Assembly.GetName().Name, serverComponent.AssemblyName); | ||
| Assert.Equal(typeof(GenericComponent<int>).FullName, serverComponent.TypeName); | ||
| Assert.NotEqual(Guid.Empty, serverComponent.InvocationId); | ||
|
|
||
| var prerenderedContent = match.Groups["content"].Value; | ||
| Assert.Equal("<p>Generic value: 123</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() |
There was a problem hiding this comment.
3. Async tests missing async 📘 Rule violation ✓ Correctness
• Newly added xUnit tests return Task but their method names do not end with the required Async suffix. • This violates the async naming convention rule and can make asynchronous behavior less discoverable when scanning test code.
Agent prompt
## Issue description
New xUnit tests return `Task` but are missing the `Async` suffix required by the async naming convention.
## Issue Context
These are `[Fact]` methods and can be renamed without changing behavior.
## Fix Focus Areas
- src/Components/Endpoints/test/EndpointHtmlRendererTest.cs[721-846]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+825
to
+838
| 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); |
There was a problem hiding this comment.
4. Wasm marker type mismatch 🐞 Bug ✓ Correctness
• The WebAssembly prerender test renders a closed generic component using GenericComponent<int> but asserts the marker’s Assembly/TypeName for GenericComponent<string>. • This will fail deterministically (or validate the wrong thing) and undermines the intended coverage for closed generic type serialization.
Agent prompt
### Issue description
The client-mode prerender test renders `GenericComponent<int>` but asserts marker metadata for `GenericComponent<string>`, causing incorrect validation and likely test failure.
### Issue Context
This test is specifically intended to validate correct handling of *closed generic* components in WebAssembly prerender mode.
### Fix Focus Areas
- src/Components/Endpoints/test/EndpointHtmlRendererTest.cs[823-839]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+138
to
+145
| 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.
5. Wrong sequence assertion 🐞 Bug ✓ Correctness
• The new "multiple markers" generic test asserts that both descriptors have Sequence==0. • The serializer increments sequence via invocationId.Next() on every SerializeInvocation call, so within one invocation the second marker should have Sequence==1.
Agent prompt
### Issue description
The new test `CanParseMultipleMarkersForClosedGenericComponents` asserts `Sequence == 0` for the second descriptor, but sequence numbers are generated by `invocationId.Next()` per serialized marker.
### Issue Context
`CreateMarkers(params Type[] types)` serializes multiple markers using the same invocation sequence instance; sequence should increment across markers.
### Fix Focus Areas
- src/Components/Server/test/Circuits/ServerComponentDeserializerTest.cs[127-145]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+1
to
+5
| @typeparam TValue | ||
|
|
||
| <p>Generic value: @(Value?.ToString() ?? "(null)")</p> | ||
| @code { | ||
| [Parameter] public TValue Value { get; set; } |
There was a problem hiding this comment.
6. Invalid generic null-conditional 🐞 Bug ✓ Correctness
• The new Razor component uses Value?.ToString() where Value is an unconstrained generic type parameter (TValue). • In C#, the null-conditional operator requires a reference type or Nullable<T> receiver; this pattern is likely to fail Razor compilation or be fragile across compiler/tooling changes.
Agent prompt
### Issue description
`Value?.ToString()` is used on an unconstrained generic type parameter, which can be invalid/fragile and may break Razor compilation.
### Issue Context
The component is used as `GenericComponent<int>` in the new endpoint renderer tests, so the generated code must be valid for value-type instantiations.
### Fix Focus Areas
- src/Components/Endpoints/test/TestComponents/GenericComponent.razor[1-6]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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 qodo-benchmark#86