Test Neural Interface Manager#53
Conversation
fernthao
force-pushed
the
test/NeuralInterfaceManager
branch
from
e75769d to
ccca206
Compare
lhmcgann
left a comment
lhmcgann
left a comment
There was a problem hiding this comment.
Great start! Check code coverage, also formatting. And if you could add doc-comments in the source class as well (class overall, properties, methods), that would be amazing
| Assert.NotNull(result); | ||
|
|
||
| // That Id is stored in the out parameter | ||
| Assert.True(NIManager.IsValidResourceId(globalInterfaceId)); |
There was a problem hiding this comment.
Id of the neural interface itself should also be set
| var result = NIManager.CreateAndRegisterNeuralInterface(interfaceType, interfaceSpecificParams, out int globalInterfaceId); | ||
|
|
||
| // Assert | ||
| Assert.NotNull(result); |
There was a problem hiding this comment.
If there isn't an available Id for the interface, the method will return an empty set rather than null. In this case though, to result should specifically be the correct list of contact IDs assigned to that neural interface. You can add another parameter (or more!) to your test methods and InlineData values to take in expected values
| Assert.True(NIManager.IsValidResourceId(globalInterfaceId)); | ||
|
|
||
| // That the neural interface object created can later be looked up via the GetNeuralInterface method | ||
| Assert.True(NIManager.TryGetNeuralInterface(globalInterfaceId, out _)); |
There was a problem hiding this comment.
Use the out parameter and make sure the resource that's found is the same object as the one you gave the manager. You can use Assert.Same to check that across repeated calls to the TryGetNeuralInterface() method, the output object is the same one
|
|
||
| // Assert | ||
| Assert.NotEmpty(contactIds); | ||
| Assert.All(contactIds, id => Assert.True(NIManager.IsValidContactId(id))); |
There was a problem hiding this comment.
These are good checks! Similar to comment above, can also pass in the explicit expected set of contact ids and make sure that matches too
| /// <param name="interfaceType">The type of the hardware interface, for example: ContactGroup, GelPad</param> | ||
| /// <param name="interfaceSpecificParams">Parameters specific to that interface</param> | ||
| [Theory] | ||
| [InlineData(typeof(ContactGroup), new object[]{1})] |
There was a problem hiding this comment.
To make passing the same data into all of these methods easier, you can use the MemberData struct instead and store all the options in a single struct which you then list before each method with the [MemberData(...)] tag
There was a problem hiding this comment.
Another data case to add to this MemberData item is a ContactGroup with a little more than one contact, and one with a lot more than one contact (test the integer limits!)
| // Assert | ||
| Assert.Equal(numContacts, contactIds.Count); | ||
| } | ||
|
|
There was a problem hiding this comment.
Also may want to consider some test cases that add more than one interface to the manager at once! And test adding an interface with a single contact first followed by a multi-contact second interface, or vice versa
3 participants
No description provided.