Adding Caller for generating unit test #356
Conversation
IsakNaslundBh
requested review from
al-fisher,
adecler,
FraserGreenroyd and
LMarkowski
|
This is timely @IsakNaslundBh as @JohannaOlin1 and I are preparing some Unit Tests for some Environment Engine work. We've started looking into this PR today, will continue to do so early next week and provide detailed feedback prior to merging 😄 |
BHoM_UI/Components/UI/UnitTest.cs
Outdated
| public override object Run(List<object> inputs) | ||
| { | ||
| if (m_CompiledFunc != null) | ||
| { | ||
| object returnValue = null; | ||
| MethodInfo method = m_OriginalItem as MethodInfo; | ||
| try | ||
| { | ||
| returnValue = m_CompiledFunc(inputs.ToArray()); | ||
| } | ||
| catch (InvalidCastException e) | ||
| { | ||
| MethodInfo originalMethod = m_OriginalItem as MethodInfo; | ||
| if (originalMethod != null && originalMethod.IsGenericMethod) | ||
| { | ||
| // Try to update the generic method to fit the input types | ||
| method = Engine.Reflection.Compute.MakeGenericFromInputs(originalMethod, inputs.Select(x => x.GetType()).ToList()); | ||
| m_CompiledFunc = method.ToFunc(); | ||
| returnValue = m_CompiledFunc(inputs.ToArray()); | ||
| } | ||
| else | ||
| throw e; | ||
| } | ||
|
|
||
| return new UnitTest() { Method = method, Data = new List<TestData>() { new TestData(inputs, SeparateOutputs(returnValue)) } }; | ||
| } | ||
| else if (InputParams.Count <= 0) | ||
| { | ||
| BH.Engine.Reflection.Compute.RecordWarning("This is a magic component. Right click on it and <Select a method>"); | ||
| return null; | ||
| } | ||
| else | ||
| { | ||
| BH.Engine.Reflection.Compute.RecordError("The component is not linked to a method."); | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
As requested previously, call the base method instead of just copy pasting the whole thing. This creates duplicates in the code that make it difficult to maintain.
Here's an example of what you could do instead:
public override object Run(List<object> inputs)
{
object returnValue = base.Run(inputs);
if (m_CompiledFunc != null)
return new UnitTest() { Method = m_OriginalItem as MethodInfo, Data = new List<TestData>() { new TestData(inputs, SeparateOutputs(returnValue)) } };
else
return null;
}There was a problem hiding this comment.
Agreed simpler. That case does not capture the case with generic methods though, but maybe that is something to be resolved at the run side of the unit test rather than here.
Will update.
|
@BHoMBot check code copyright |
|
@IsakNaslundBh to confirm, |
|
@BHoMBot check installer |
|
@IsakNaslundBh to confirm, |
|
@BHoMBot check copyright |
|
@IsakNaslundBh to confirm, |
There was a problem hiding this comment.
Icon looks good to me @JohannaOlin1 - see screenshot of it rendered on my monitor below.
@IsakNaslundBh see compliance fix needed - hopefully an easy commit? 😉
There was a problem hiding this comment.
@IsakNaslundBh noticed you left so took liberty of committing fix to allow @adecler a final review and merge if he's happy 😄 but this LGTM in testing with the other PR!
|
@BHoMBot check installer |
|
@FraserGreenroyd to confirm, |
|
@BHoMBot check copyright compliance |
|
@FraserGreenroyd to confirm, |
|
The icon colour was not correct (notice colour coding for each section of icon) so fixed it myself.
|
|
/azp run BHoM_UI.CheckCore |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@BHoMBot check copyright |
|
@adecler to confirm, |
|
@BHoMBot check installer |
|
@adecler to confirm, |
|
Like the Icon! Thanks @JohannaOlin1 and @adecler ! Tested this quickly again, and works fine for me! Can not approve the latest tweaks, being the author, but I am happy! |
Issues addressed by this PR
Closes #355
Adding a complete new component that can be used to generate a unit test.
The component lets you pick the method you want, and then transforms into giving the inputs corresponding to that method. Instead of just running the method, though, the component returns a UnitTest object with the method from the component as well as the linked input-output data.
The component is currently returning one UnitTest object per run of the method, which then you should be able to convert into one simply after BHoM/Test_Toolkit#316 has been closed out.
The menu for the method will need some work, but that is quite deep into the code and used by a lot of other things (not something that is simply overrideable as far as I can see @adecler ), so all methods that are not create methods can be found under "NonBHoMObjects".
Have not made an icon yet (and do not have time right now) but wanted to get this pushed for initial feedback and review.
Test files
Changelog
Additional comments
Requires BHoM/Grasshopper_UI#590 to be run in GH. I have not added support in any other UI right now (although I think it should work in them as well with similar addition)