Test cases for dictionary link generation #7706
Conversation
|
|
||
| [Test] | ||
| [Category("UnitTests")] | ||
| public void GetAllFunctionDescriptorsTest() |
There was a problem hiding this comment.
@yeexinc the nodes can change over time so this test may not hold true when a new overload is added for any of these nodes for example. We usually test the library services against a dummy library in the test framework called FFITarget.dll. See other tests using this library.
|
Thanks for the review @aparajit-pratap , I've updated the test case for it. Please take a look. |
test/DynamoCoreTests/LibraryTests.cs
Outdated
| descriptors = libraryServices.GetAllFunctionDescriptors("Invalid node name"); | ||
| Assert.IsNull(descriptors); | ||
| // Get the function groups that are named FFITarget.TestOverloadConstructor.TestOverloadConstructor | ||
| var descriptors = libraryServices.GetFunctionGroups(libraryPath) |
There was a problem hiding this comment.
It looks like we are testing for both GetFunctionGroups as well as GetAllFunctionDescriptors in this unit test. Can't we simply test for the result from GetAllFunctionDescriptors against the constant number? That number should not change since they belong to FFITarget and even if they do, the test can be updated accordingly.
|
@aparajit-pratap - The test case has been updated accordingly. Thanks! |
|
Thanks @yeexinc |
test/DynamoCoreTests/LibraryTests.cs
Outdated
| @@ -31,6 +33,7 @@ public override void Setup() | |||
| libraryCore.ParsingMode = ParseMode.AllowNonAssignment; | |||
|
|
|||
| var pathResolver = new TestPathResolver(); | |||
| pathResolver.AddPreloadLibraryPath("ProtoGeometry.dll"); | |||
There was a problem hiding this comment.
@yeexinc please remove this line
Purpose
This PR adds test cases for functions used while generating the dictionary links on context menu help popups (#7572 and #7661).
Tests are done on the functions
GetAllFunctionDescriptors()andConstructDictionaryLinkFromLibrary().Declarations
Check these if you believe they are true
*.resxfilesReviewers
@aparajit-pratap
FYIs