DYN-9702: Fix flaky PackageLoaderTests broken by shipped definitions folder#17222
Conversation
The shipped definitions folder (added in DYN-9702) now loads Curve_Validate.dyf as a non-package custom node at model startup, leaving PackageInfo null in CustomNodeManager.NodeInfos. The LINQ predicates in LoadingCustomNodeFromPackageSetsNodeInfoPackageInfoCorrectly and PlacingCustomNodeInstanceFromPackageRetainsCorrectPackageInfoState called PackageInfo.Equals on every entry, throwing a NullReferenceException on the non-package node and failing both tests deterministically. Add a null guard so non-package nodes are skipped before the Equals comparison. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![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-9702
There was a problem hiding this comment.
Pull request overview
This PR fixes deterministic flakiness in PackageLoaderTests caused by shipped (non-package) custom node definitions being loaded at model startup. Specifically, it prevents NullReferenceException when tests iterate CustomNodeManager.NodeInfos and encounter nodes whose PackageInfo is null.
Changes:
- Added a null guard around
PackageInfo.Equals(packageInfo)inLoadingCustomNodeFromPackageSetsNodeInfoPackageInfoCorrectly. - Added the same null guard in
PlacingCustomNodeInstanceFromPackageRetainsCorrectPackageInfoState(both initial query and post-placement re-query).
|
jasonstratton
left a comment
jasonstratton
left a comment
There was a problem hiding this comment.
Perfect! Thank you!
Approved and will merge. I'll make a couple minor changes to the PR description too.
|
I'll wait for the SelfServe to complete before merging |
|
Tidied the description on a maintainer's request: dropped the cross-reference to #17221 (being closed as a duplicate of this PR), removed the 'fails randomly' note, and pointed the Jira link at autodesk.atlassian.net. The fix itself is untouched. |
Purpose
DYN-9702 (#17201) added a shipped
definitions/folder (containingCurve_Validate.dyf) and changedPathManager.DefinitionDirectoriesto include the common definitions directory. As a result,Curve_Validate.dyfis now loaded at model startup as a non-package custom node, leavingPackageInfo == nullinCustomNodeManager.NodeInfos.Two tests iterate all of
NodeInfoswith an unguardedx.Value.PackageInfo.Equals(packageInfo)predicate, which throws aNullReferenceExceptionon the non-package node:LoadingCustomNodeFromPackageSetsNodeInfoPackageInfoCorrectlyPlacingCustomNodeInstanceFromPackageRetainsCorrectPackageInfoStateThe DYN-9702 change made the failure deterministic in CI whenever the definitions folder is present. This PR null-guards the comparison so non-package nodes are skipped before
Equalsis called.Declarations
Check these if you believe they are true
Release Notes
N/A -- test-only fix; no user-facing or behavioral change.
Reviewers
(FILL ME IN) Reviewer 1
FYIs
(FILL ME IN, Optional)
🤖 Generated with Claude Code