-
Notifications
You must be signed in to change notification settings - Fork 668
DYN-9537: Fix unit tests for cross version schemas support #16571
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-9537
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes unit test stability issues in the Forge Units tests by addressing cross-version schema compatibility problems. The tests were failing inconsistently across different environments because they expected a specific version format but different systems return different schema versions depending on whether ASC (Autodesk Shared Components) is installed.
- Updated version assertion logic to accept both
-2.0.0and-1.0.1version formats as valid - Removed fragile schema index dependency assertion in
ForgeUnitDropdownsLoadWithMalformedDatatest - Added explanatory comments documenting the cross-version compatibility handling
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| test/DynamoCoreTests/UnitsOfMeasureTests.cs | Updated version assertions to handle multiple valid schema versions with improved error messaging |
| test/DynamoCoreWpf3Tests/UnitsUITests.cs | Removed brittle assertion that depended on specific schema ordering |
| // Depending on the test system, the latest known version could be either -2.0.0 or -1.0.1 | ||
| var expectedVersions = new[] { $"{milimeters}-2.0.0", $"{milimeters}-1.0.1" }; | ||
| Assert.That(expectedVersions, Contains.Item(unitType.TypeId), | ||
| $"Expected TypeId to be one of the known versions, but got: {unitType.TypeId}"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depending on whether ASC schemas are available on the test system, this may be either 1.0.1 or 2.0.0. The schema fallback mechanism correctly selects the latest available schema when encountering an unknown future version, but the specific version depends on the current system configuration.
| // Depending on the test system, the latest known version could be either -2.0.0 or -1.0.1 | ||
| var expectedVersions = new[] { $"{milimeters}-2.0.0", $"{milimeters}-1.0.1" }; | ||
| Assert.That(expectedVersions, Contains.Item(unitType.TypeId), | ||
| $"Expected TypeId to be one of the known versions, but got: {unitType.TypeId}"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depending on whether ASC schemas are available on the test system, this may be either 1.0.1 or 2.0.0. The schema fallback mechanism correctly selects the latest available schema when encountering an unknown past version, but the specific version depends on the current system configuration.
| Assert.AreNotEqual("autodesk.unit.quantity:force", GetTypeName<DynamoUnits.Quantity>(node3.CachedValue.Data)); | ||
| Assert.AreEqual("autodesk.unit.quantity:flowPerVolume", GetTypeName<DynamoUnits.Quantity>(node3.CachedValue.Data)); | ||
| Assert.AreEqual("autodesk.unit.symbol:mm", GetTypeName<DynamoUnits.Symbol>(node4.CachedValue.Data)); | ||
| Assert.AreEqual("autodesk.unit.unit:maxwells", GetTypeName<DynamoUnits.Unit>(node5.CachedValue.Data)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The maxwells schema was selected because the dropdown node used index 221 in the malformed DYN node JSON data (since "Millimeters AAA" is not a valid unit schema name). This index maps to different units (maxwells or microarcseconds) depending on whether ASC schemas are present. This test condition was removed as it doesn't provide meaningful validation of the dropdown selection system's stability, which is already functioning correctly.
reddyashish
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
|
Merging this as to unblock daily builds. |
Purpose 🎯
This PR fixes unit test stability issues in the Forge Units tests by handling cross-version schema compatibility. The tests were failing inconsistently across different test systems because they expected a specific version format (
-2.0.0) but some systems return a different format (-1.0.1). This variation occurs depending on whether ASC (Autodesk Shared Components) is installed or the system falls back to bundled schemas.Changes: These changes only affect test assertions and don't modify any production code. The tests now accept both
-2.0.0and-1.0.1version formats as valid fallback versions. TheForgeUnitDropdownsLoadWithMalformedDatatest was updated to avoid checking fragile schema index dependencies that vary between ASC and bundled schema environments.Fixed Tests:
CanCreateForgeUnitType_FromFutureTypeStringCanCreateForgeUnitType_FromPastTypeStringForgeUnitDropdownsLoadWithMalformedDataContext: The alternate schema loading mechanism was introduced in PR #16529, which enhanced DynamoUnits with auto schema discovery from ASC installations.
Declarations ✅
Check these if you believe they are true
Release Notes 📝
Test Stability: Fixed unit tests for Forge unit type creation to handle cross-version schema compatibility, improving test reliability across different environments.
Reviewers 👥
(FILL ME IN) Reviewer 1 (If possible, assign the Reviewer for the PR)
FYIs 📢
(FILL ME IN, Optional) Names of anyone else you wish to be notified of