Skip to content

Conversation

@benglin
Copy link
Contributor

@benglin benglin commented Oct 3, 2025

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.0 and -1.0.1 version formats as valid fallback versions. The ForgeUnitDropdownsLoadWithMalformedData test was updated to avoid checking fragile schema index dependencies that vary between ASC and bundled schema environments.

Fixed Tests:

  • CanCreateForgeUnitType_FromFutureTypeString
  • CanCreateForgeUnitType_FromPastTypeString
  • ForgeUnitDropdownsLoadWithMalformedData

Context: 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

Copilot AI review requested due to automatic review settings October 3, 2025 16:15
@github-actions github-actions bot changed the title Fix unit tests for cross version schemas support DYN-9537: Fix unit tests for cross version schemas support Oct 3, 2025
Copy link

@github-actions github-actions bot left a 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

Copy link
Contributor

Copilot AI left a 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.0 and -1.0.1 version formats as valid
  • Removed fragile schema index dependency assertion in ForgeUnitDropdownsLoadWithMalformedData test
  • 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}");
Copy link
Contributor Author

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}");
Copy link
Contributor Author

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));
Copy link
Contributor Author

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.

Copy link
Contributor

@reddyashish reddyashish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@reddyashish
Copy link
Contributor

Merging this as to unblock daily builds.

@reddyashish reddyashish merged commit f1c7df2 into master Oct 3, 2025
27 of 28 checks passed
@reddyashish reddyashish deleted the DYN-9537 branch October 3, 2025 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants