Repository navigation
fix(tests): parse appsettings with comments in the CORS configuration tests - #1448
Merged
iammukeshm merged 2 commits intoNov 3, 2026
Conversation
… tests The ASP.NET configuration loader accepts comments and trailing commas in appsettings files, but CorsConfigurationTests reads them with a strict JsonDocument.Parse. The first project that documents a setting inline (a common thing to do in a scaffolded app) gets a JsonReaderException from a test that has nothing to do with the comment. Parse with the same leniency. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
iammukeshm
approved these changes
Nov 3, 2026
iammukeshm
left a comment
iammukeshm
left a comment
Member
There was a problem hiding this comment.
Makes sense. The test should be no stricter than the config loader it stands in for. Thanks!
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.
CorsConfigurationTests(from #1391) readsappsettings.json/appsettings.Production.jsonwith a strictJsonDocument.Parse. The ASP.NET configuration loader accepts comments and trailing commas in those files, so the test is stricter than the runtime.It doesn't show up here because the template's appsettings has no comments. It does show up as soon as a scaffolded project documents a setting inline, which is how many teams annotate config. The CORS tests then fail with an error that has nothing to do with CORS:
The fix parses the files with
JsonCommentHandling.SkipandAllowTrailingCommas, the same leniency as the config loader. The test only changes how it parses the files; its assertions are unchanged.Verification
The run used
-p:NuGetAudit=false, becausemaincurrently fails restore on the crypto advisories that #1447 fixes. With a comment added toappsettings.json, the old parser throws and the new one passes.🤖 Generated with Claude Code