DYN-7392 Forge Units reference #15512
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-7392
UI Smoke TestsTest: success. 11 passed, 0 failed. |
| <!--Skip copying the runtime dlls because we only need one (ForgeUnitsManaged.dll) which we copy in a task bellow.--> | ||
| <ExcludeAssets>runtime;build</ExcludeAssets> | ||
| </PackageReference> | ||
| <PackageReference Include="ForgeUnits.Schemas" Version="1.0.1" GeneratePathProperty="true" /> | ||
| <PackageReference Include="ForgeUnits.Schemas" Version="1.0.4" GeneratePathProperty="true" /> |
There was a problem hiding this comment.
so does this work in Revit?
There was a problem hiding this comment.
The compatibility discussion is here:
https://autodesk.slack.com/archives/CA4SXCD4L/p1730755756302839?thread_ts=1730726649.001799&cid=CA4SXCD4L
| @@ -199,7 +199,7 @@ public void ForgeUnitDropdownsLoadWithMalformedData() | |||
| //we've changed both index and name, so a new item is selected. | |||
| Assert.AreNotEqual(DynamoUnits.Quantity.ByTypeID("autodesk.unit.quantity:force-1.0.2"), node3.CachedValue.Data); | |||
| Assert.AreEqual(DynamoUnits.Symbol.ByTypeID("autodesk.unit.symbol:mm-1.0.1"), node4.CachedValue.Data); | |||
| Assert.AreEqual(DynamoUnits.Unit.ByTypeID("autodesk.unit.unit:millimeters-1.0.1"), node5.CachedValue.Data); | |||
| Assert.AreEqual(DynamoUnits.Unit.ByTypeID("autodesk.unit.unit:microarcseconds-1.0.1"), node5.CachedValue.Data); | |||
There was a problem hiding this comment.
new unit changes went in so the order changed
https://git.autodesk.com/forge/HFDMSchemas/pull/148/files#diff-d5a66c435e446495cd9f736484e9a5bbcffb1e9fb15e7542418fadc479f2ad19
There was a problem hiding this comment.
It seems to me this test is trying to assert exactly what you saying is NOT happening.
IE when new schema data is introduced the nodes should not just return new values... that will break graphs.
There was a problem hiding this comment.
@QilongTang @saintentropy do I have that right? I kind of remember this is a general problem with dropdowns on the Revit side, though I thought we had semi fixed it with string matching as well as index matching at some point?
I am remembering incorrectly?
There was a problem hiding this comment.
Right, to me it looks like opening a legacy graph the node is returning a different value, if the old value is still there from the list, then this could be a bug worth fixing. I would suggest checking the serialized index and node value and compare with the new graph opening
There was a problem hiding this comment.
The node in question
"SelectedIndex": 221,
"SelectedString": "Millimeters AAA",
"NodeType": "ExtensionNode",
"Id": "2cc49d25d3b447b281a81efd6173c6cf",
```
It has an intentional malformed selection "Millimeters AAA" - that does not exist.
So in turn it defaults to the selected index (221) which now is different (points to microarcsecond), because of the changes that went in
There was a problem hiding this comment.
I don't want to hold this up further, but I am trying to understand the following:
- Is this failure an existing issue with all dropdowns or just these unit nodes?
- What was this test intending to test? Does it still test that? Is it valuable?
There was a problem hiding this comment.
- I think this scenario could affect all dropdowns
- It looks to me like this test wants to cover the stability of unit drop downs. When the serialized unit selection exists at runtime and when it does not exist anymore. I think there is still value in it
If a unit value is renamed, the selectedIndex could still point to the new (renamed) unit.
This test is prone to failures if new units are added or removed.
|
why the test changes? |
new units were introduced so some of the hardcoded values in the tests changed |
Co-authored-by: pinzart90 <[email protected]>
Purpose
Per https://jira.autodesk.com/browse/DYN-7392, update dependency originally brought in as part of #12126
Updates ForgeUnits to 5.3.2 as REvit
Updates Forge.Schema to 1.0.4 (latest on public nuget), but it still does not match the latest Revit
Updates Analtics to 4.2.1.6685 which brings in the same version of ADP that Revit uses
Declarations
Check these if you believe they are true
*.resxfilesRelease Notes
Update Forge Units reference
Reviewers
(FILL ME IN) Reviewer 1 (If possible, assign the Reviewer for the PR)
(FILL ME IN, optional) Any additional notes to reviewers or testers.
FYIs
(FILL ME IN, Optional) Names of anyone else you wish to be notified of