Improve OData and validation test coverage#324
Open
rgregg-msft wants to merge 13 commits intomasterfrom
Open
Conversation
…resource generation Adds 30 tests covering ODataParser.DeserializeEntityFramework (standard types, ags: custom attributes, inheritance merging), BuildJsonExample (primitives, collections, nested complex types, Edm.Stream exclusion), GenerateResourcesFromSchemas (namespace validation, alias namespace, multi-schema), and serialize/deserialize round-trip preservation of all ags: attributes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds 42 tests covering the surface area that directly gates the ODataLib migration decision: ODataNavigationTests (27 tests): - EntityType.NavigateByUriComponent: collection nav property, scalar nav property, unknown component, case-mismatch error, base type fallback, property on self - ComplexType.NavigateByUriComponent: property match, unknown component, case-mismatch error, base type fallback - EntityContainer.NavigateByUriComponent: EntitySet returns ODataCollection, Singleton returns EntityType, unknown returns null, key throws - ODataCollection.NavigateByUriComponent: Guid key, long key, type cast segment, unknown segment, NavigateByEntityTypeKey ODataExtensionMethodTests (15 tests): - NamespaceOnly: qualified, single-segment, collection, no-namespace throws - TypeOnly: qualified, collection, unqualified passthrough - HasNamespace: qualified true, unqualified false - IsCollection: collection true, Edm false, qualified false - ElementName: collection unwraps, non-collection passthrough, Edm collection - LookupNavigableType: EntityType, ComplexType, Edm primitive, unknown throws - ResourceWithIdentifier: EntityType, ODataSimpleType, Collection, unknown throws Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ActionOrFunctionTests (21 tests): - CanSubstituteFor: identical functions, null other, different name/IsBound/ param count/return type, both-null return type, one-null return type, Action vs Function type mismatch - Parameter matching: name not found, IsNullable mismatch, Unicode mismatch, type mismatch with no inheritance - Contravariance: this=base/other=derived returns true, reversed returns false, multi-level (grandparent to grandchild) returns true - Deserialization: Function with parameters and ReturnType, IsComposable flag, Action without ReturnType EnumTypeTests (10 tests): - Deserialization: name and members, UnderlyingType, IsFlags, IsFlags defaults false, UnderlyingType null when absent, member without value, multiple enums, TypeIdentifier equals Name - Navigation boundaries: NavigateByUriComponent and NavigateByEntityTypeKey both throw NotImplementedException Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fixes five gaps identified in adversarial review: - BuildJsonExample: assert actual values (not just JTokenTypes) to catch stale static-cache hits; documents the shared-state risk in a comment - GenerateResourcesFromSchemas: add test asserting enum types are intentionally excluded from resource output - EntityType collection nav test: chain through NavigateByEntityTypeKey to verify the element type is actually resolvable in the edmx, not just that the identifier string is correct - CanSubstituteFor: add two tests for null parameter type (this.Type=null and other.Type=null), exercising the null guard at ActionOrFunctionBase.cs:68-69 - Function deserialization: assert parameter types in addition to names Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
visitedProperties and generatedExamples were static fields on ODataParser,
making them process-level singletons that accumulated state across calls.
This caused two problems:
1. A second call to BuildJsonExample on the same type returned stale cached
property values rather than regenerating them.
2. Tests were order-dependent and could interfere if they used the same
namespace+typename+property key.
The visited set and cache are now allocated fresh on each call to
BuildJsonExample and threaded down through BuildDictionaryExample,
ExampleOfType, and ObjectExampleForType as parameters. The recursion guard
still functions correctly for circular type references within a single call.
Adds two regression tests:
- CalledTwiceOnSameType_SecondCallGeneratesFreshResult: would have failed
against the old code if the second call lost the property entirely
- CircularTypeReference_DoesNotStackOverflow: verifies the recursion guard
still works correctly after the refactor
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tring utilities - TransformationHelperTests: 19 tests covering ApplyTransformation (simple copy, null skip, alternative handler intercept/pass-through, GraphPropertyName rename) and ApplyTransformationToCollection (exact match, wildcard, Remove, version filtering, Add with/without version match) - SchemaValidationTests: 6 tests covering ValidateSchemaTypes for empty schema, primitives, known entity/complex type references, unknown type, cross-schema resolution, and collection types - StringSuggestionsAndXmlParseHelperTests: 16 tests covering Damerau-Levenshtein fuzzy matching (exact, one-char diff, transposition, threshold) and XmlParseHelper element name validation 196 tests total, all passing. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add coverlet.runsettings: configures Cobertura output format and enforces a 20% minimum line coverage threshold (Total). Build fails if coverage drops below this. Threshold set conservatively below the current ~24% baseline; raise it incrementally as coverage improves. - Update dotnet.yml: pass --settings coverlet.runsettings to the coverage test step so the threshold is checked on every run - Add ReportGenerator step: generates MarkdownSummaryGithub + HTML report from the Cobertura XML - Post coverage summary to $GITHUB_STEP_SUMMARY so it appears in the Actions job summary on every PR and push (no extra permissions required) - Upload HTML coverage report as a build artifact for detailed drill-down Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add pull-requests: write permission to the build job - Add marocchino/sticky-pull-request-comment step that posts the ReportGenerator markdown summary directly on the PR, updating in place on each push rather than creating duplicate comments - Comment is only posted on pull_request events (not push to master) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rgregg-msft
force-pushed
the
improve-odata-test-coverage
branch
from
326e5ab to
a55deb8
Compare
SummarySummary
Coverageapidoc - 7%
ApiDoctor.DocumentationGeneration - 72.9%
ApiDoctor.Publishing - 0.7%
ApiDoctor.Validation - 43%
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes ODataParser.BuildJsonExample example-generation state leaking across calls by removing static caches and introducing per-invocation recursion tracking, and it adds a broad set of NUnit unit tests covering OData parsing, navigation, transformations, and validation behaviors.
Changes:
- Replace static example-generation caches in
ODataParserwith per-callvisited/cachestate passed through recursion. - Add comprehensive unit tests for OData parsing/serialization, navigation, schema validation, transformations, extension methods, enums, and actions/functions.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| ApiDoctor.Validation/OData/ODataParser.cs | Removes static state from JSON example generation; threads recursion/caching state through helper methods. |
| ApiDoctor.Validation.UnitTests/TransformationHelperTests.cs | Adds tests for transformation application and collection modification behaviors. |
| ApiDoctor.Validation.UnitTests/StringSuggestionsAndXmlParseHelperTests.cs | Adds tests for string suggestion scoring and XML parse helper extension methods. |
| ApiDoctor.Validation.UnitTests/SchemaValidationTests.cs | Adds tests validating schema type resolution behavior and console reporting for unknown types. |
| ApiDoctor.Validation.UnitTests/ODataParserTests.cs | Adds tests for EDMX parsing, inheritance merging, JSON example generation, and round-trip serialization. |
| ApiDoctor.Validation.UnitTests/ODataNavigationTests.cs | Adds tests for URI navigation across entity/container/collection/complex type scenarios. |
| ApiDoctor.Validation.UnitTests/ODataExtensionMethodTests.cs | Adds tests for type string extension helpers and EntityFramework type lookup helpers. |
| ApiDoctor.Validation.UnitTests/EnumTypeTests.cs | Adds tests for enum parsing and navigability boundaries. |
| ApiDoctor.Validation.UnitTests/ActionOrFunctionTests.cs | Adds tests for substitution rules and function/action deserialization. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ApiDoctor.Validation.UnitTests/StringSuggestionsAndXmlParseHelperTests.cs
Show resolved
Hide resolved
ApiDoctor.Validation.UnitTests/StringSuggestionsAndXmlParseHelperTests.cs
Show resolved
Hide resolved
Without this, coverlet instruments the test project itself, which always shows 100% (the runner executes every test method) and skews the report. The Exclude filter restricts coverage to production assemblies only. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Using Include is more precise than Exclude: only first-party assemblies (ApiDoctor.*, apidoc, ObjectGraphMergeUtility) are measured. This drops MarkdownDeep (third-party OSS), all NuGet dependencies, and test projects from the report without having to enumerate them explicitly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- ODataParser: thread metadataValidationConfigs through ExampleOfType and ObjectExampleForType so nested @odata.type values respect ValidateNamespace and AliasNamespace settings consistently at all nesting levels - ODataParserTests: update two comments that still referenced the removed static fields (visitedProperties, generatedExamples); reword to describe the current per-call invariant being tested - StringSuggestionsAndXmlParseHelperTests: add explicit `using ApiDoctor.Validation` for StringSuggestions; rename test to SuggestStringFromCollection_EmptyCollection_ReturnsMaxScore to match what is actually asserted Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
[ApiDoctor.*] in the Include pattern also matches ApiDoctor.Validation.UnitTests. Add an Exclude for *.UnitTests and *.Tests so test assemblies are filtered out even when they satisfy the Include glob. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Covers the primary user-facing CLI option classes: - CommandLineOptionsVerbTests (12): assert all verb constants match their expected CLI string values — catches accidental renames that would silently break the CLI - BaseOptionsTests (5): PageParameterDict URL-encoded query string parsing (null, empty, single, multiple params, case-insensitive keys); base HasRequiredProperties always returns true - PrintOptionsTests (6): HasRequiredProperties validation — returns false when no print flags are set, true for each individual flag and for combinations - FixDocsOptionsTests (8): Matches property parsing — null/empty/whitespace → empty set; comma and semicolon separators; whitespace trimming; case-insensitive comparison - PublishOptionsTests (9): FilesToPublish splitting (null, single, multi, round-trip setter); HasRequiredProperties for Markdown (always OK), Mustache (requires template), Swagger2 (requires title/description/version, reports all missing fields) - GeneratePermissionFilesOptionsTests (4): HasRequiredProperties — bootstrapping mode skips file requirement; non-bootstrapping requires non-whitespace file - PublishMetadataOptionsTests (4): GetOptions() mapping — all fields copied correctly; Namespaces split on comma and semicolon; null Namespaces → null Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.
Summary
ODataParser.BuildJsonExamplewherevisitedPropertiesandgeneratedExampleswere process-level singletons, causing stale results across callsApiDoctor.Validation.ODatasubstantially improved (EntityType 88.8%, EntityContainer 92.5%, ODataCollection 94.7%, Schema 95.2%)New test files
ODataParserTests.csBuildJsonExample, resource generation, cache regressionODataNavigationTests.csNavigateByUriComponenton EntityType, ComplexType, EntityContainer, ODataCollectionODataExtensionMethodTests.csNamespaceOnly,TypeOnly,HasNamespace,IsCollection,ResourceWithIdentifierActionOrFunctionTests.csCanSubstituteForcontravariance, parameter matching, deserializationEnumTypeTests.csIsFlags,NavigateByUriComponent/NavigateByEntityTypeKeyTransformationHelperTests.csApplyTransformation,ApplyTransformationToCollection(wildcard, Remove, version filtering, Add)SchemaValidationTests.csValidateSchemaTypesfor valid/invalid type referencesStringSuggestionsAndXmlParseHelperTests.csXmlParseHelperelement name validationTest plan
dotnet test)🤖 Generated with Claude Code