DYN-1914: Add tests for package dependency collection, serialization, and deserialization #9784
Conversation
scottmitchell
changed the title
|
@scottmitchell what needs to be discussed on the last test? |
|
@mjkkirschner I wanted to get some more info about when/if package removal happens within a session. When you uninstall a package, it looks like the package isn’t actually removed until you close Dynamo? If that’s true, then my original intention for the test (to remove a package within a session) doesn’t make sense. I’ll join standup tomorrow. |
|
That’s true for binary packages but I think custom node only packages can be removed.
… On Jun 10, 2019, at 12:57 AM, Scott Mitchell ***@***.***> wrote:
@mjkkirschner I wanted to get some more info about when/if package removal happens within a session. When you uninstall a package, it looks like the package isn’t actually removed until you close Dynamo? If that’s true, then my original intention for the test (to remove a package within a session) doesn’t make sense. I’ll join standup tomorrow.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
@scottmitchell does this need a review? |
|
@scottmitchell where is the code for your testPackage.dll? |
|
@mjkkirschner Yes, it's ready for review. I think we should get this initial package dependency work wrapped up, and I think this gets us good test coverage to start. ZTTestPackage is here: https://github.com/scottmitchell/ZTTestPackage |
|
@scottmitchell can you guarantee that this test code will never disappear and we'll always have access to it? I guess in the worst case we can decompile the dll - I think it's a bit cumbersome to have this in another repo - let's improve this in a followup. |
| // Add one node from "Dynamo Samples" package | ||
| var pi = GetPackageInfo("Dynamo Samples"); | ||
| var node = GetNodeInstance("Examples.NEWBasicExample.Create@double,double,double"); | ||
| CurrentDynamoModel.AddNodeToCurrentWorkspace(node, true); |
There was a problem hiding this comment.
@scottmitchell how is this test different from the above test? Did you mean to add a new workspace?
|
@scottmitchell please run this on the self serve - because you are adding a new package - it will likely break some other tests which count the number of packages loaded from the test/package folder. |
|
@mjkkirschner everything looks good on the self serve. I didn’t put this package with the other test packages because I didn’t want it to load by default. It’s in the new “packagedDependencyTests” folder. I can replace this package with a new package in the Dynamo repo if we want. I just needed a very simple package that wasn’t already included in the default test packages. Functionality of the package does not matter for these tests. |
| if (package.Equals(package1)) | ||
| { | ||
| // Package 1 should have two nodes | ||
| Assert.AreEqual(2, package.Nodes.Count); |
|
These tests LGTM. Oops, wrong button |
| base.GetLibrariesToPreload(libraries); | ||
| } | ||
|
|
||
| private PackageLoader GetPackageLoader() |
There was a problem hiding this comment.
This function already exists in PackageLoaderTests:
… and deserialization (#9784) * Return copy of PackageDependencyInfo instead of reference. * Add comments and TODOs * Override GetHashCode for PackageDependencyInfo * Subscribe to default home workspace * Add PackageDependencyTests * GetHashCode summary * Add PackageDependenciesUpdatedAfterNodesAdded test * Add PackageDependenciesUpdatedAfterNodeRemoved test * Add ZTTestPackage * Add PackageDependenciesUpdatedAfterPackageAndNodeAdded test * Add PackageDependenciesPreservedWhenPackagesNotLoaded test * Switch node in custom node test graph * Rework custom node tests to avoid errors
Purpose
This PR addresses JIRA issue DYN-1914: Add tests for package dependency collection, serialization, and deserialization. A new test class
PackageDependencyTestshas been added. This PR does the following:Add test coverage for package dependency
ZeroTouchPackageDependencyIsCollectedAndSerializedNodeModelPackageDependencyIsCollectedCustomNodePackageDependencyIsCollectedPackageDependencyIsCollectedForNewWorkspace-- fails without the fix contained in this PRPackageDependenciesUpdatedAfterNodeAddedPackageDependenciesUpdatedAfterNodeRemoved-- fails without the fix at DYN-1662 Follow Up: Return copy of PackageDependencyInfo instead of reference #9773PackageDependenciesUpdatedAfterPackageAndNodeAddedPackageDependenciesPreservedWhenPackagesNotLoadedPackageDependenciesPreservedAfterPackageRemovedAddress a bug found by @QilongTang where package dependency collection failed for some graphs
Override
GetHashCodeforPackageDependencyInfotypeDeclarations
Check these if you believe they are true
*.resxfilesReviewers
@mjkkirschner @QilongTang