Search Unity

Why is this test failing?

Discussion in 'Testing & Automation' started by sebas77, Jan 29, 2021.

  1. sebas77

    sebas77

    Joined:
    Nov 4, 2011
    Posts:
    1,387
    NewTestScriptSimplePasses (0.021s)
    ---
    Expected: not allocates GC memory
    But was: <NUnit.Framework.TestDelegate>
    ---
    at NewTestScript.NewTestScriptSimplePasses () [0x00013] in C:\Prgdir\UsingTest\New Unity Project\Assets\Tests\NewTestScript.cs:16

    My code is not supposed to allocate at all, but this test fails. It doesn't explicitly say that it fails because of allocations though, so before to continue the investigation I need to be sure about it.
     
  2. superpig

    superpig

    Drink more water! Unity Technologies

    Joined:
    Jan 16, 2011
    Posts:
    4,486
    Can you show the code of the test?
     
  3. sebas77

    sebas77

    Joined:
    Nov 4, 2011
    Posts:
    1,387
    sure but I would prefer you answering my question because I needed it to decide if to open a bug or not
     
  4. sebas77

    sebas77

    Joined:
    Nov 4, 2011
    Posts:
    1,387
    right I noticed I didn't ask the question I actually wanted to ask. The question is: what does But was: <NUnit.Framework.TestDelegate> mean?
     
    Last edited: Jan 29, 2021
  5. sebas77

    sebas77

    Joined:
    Nov 4, 2011
    Posts:
    1,387
  6. superpig

    superpig

    Drink more water! Unity Technologies

    Joined:
    Jan 16, 2011
    Posts:
    4,486
    Ahhh, right. What's happening there is that NUnit has a standard way of displaying constraint failures - it shows you the constraint (in this case, 'not allocates GC memory') and the 'value' that did not meet the constraint, in this case a delegate that was generated from the function you passed in. The fact that it's reporting that constraint as having failed means that the delegate you passed is indeed causing GC memory to be allocated when it is invoked.

    The message could definitely be improved... I'm a little puzzled because it looks like the code for AllocatingGCMemoryResult does override the method for generating the output, it should be producing a message like "The provided delegate made 3 GC allocations" or similar. We do have tests that are checking
    AssertionException.Message
    for a string along those lines, so it might be a bug in the way NUnit generates the final result for the console.
     
    sebas77 likes this.
  7. sebas77

    sebas77

    Joined:
    Nov 4, 2011
    Posts:
    1,387
    Ok thank you so much. Since the attached code must not allocate, I'll report it as a bug. Let me know if you have any thoughts on it
     
  8. superpig

    superpig

    Drink more water! Unity Technologies

    Joined:
    Jan 16, 2011
    Posts:
    4,486
    One thing to check - if you execute the delegate once first, and then do the assertion for it, do you still see it fail? I think I've seen issues in the past on Mono where the initial JIT of the code can cause GC allocations.
     
    sebas77 likes this.
  9. sebas77

    sebas77

    Joined:
    Nov 4, 2011
    Posts:
    1,387
    yes that worked! Of course I suggest to try to find a way to execute the Delegate before testing it because it doesn't make much sense
     
  10. superpig

    superpig

    Drink more water! Unity Technologies

    Joined:
    Jan 16, 2011
    Posts:
    4,486
    We can't guarantee that the delegate will not have side effects, so it's not safe for us to just execute it for you automatically.

    I think we might be able to force JIT of the delegate itself, which will help with some cases, but I don't think there is any way for us to guarantee that no JIT happens at all during the assertion. At the very least we should mention this in the docs though. Let me see what I can do about that...
     
    sebas77 likes this.
  11. sebas77

    sebas77

    Joined:
    Nov 4, 2011
    Posts:
    1,387
    Ok I understand it but that would mean that the test fails without a good reason to fail under the test point of view.
     
  12. sebas77

    sebas77

    Joined:
    Nov 4, 2011
    Posts:
    1,387
    BTW one thing that is important to note is that the using line showed in my code is what cause the allocation. If I comment it, the test won't fail.
     
  13. superpig

    superpig

    Drink more water! Unity Technologies

    Joined:
    Jan 16, 2011
    Posts:
    4,486
    I've opened a PR that will replace the 'NUnit.Framework.TestDelegate' with something more useful like '3 GC allocations', as well as improving the docs a bit. Hopefully this can ship in the next release of the test framework.

    Looking at the IL for your code, the using line is doing this:
    Code (csharp):
    1.  
    2.                 IL_0024: ldarg.0
    3.                 IL_0025: ldstr "Svelto.ECS - Entities Submission"
    4.                 IL_002a: newobj instance void PlatformProfiler::.ctor(string)
    5.                 IL_002f: stfld valuetype PlatformProfiler NewTestScript/'<UsingTest>d__1'::'<>7__wrap1'
    6.  
    The
    newobj
    is what is causing the allocation. I think it's because your
    UsingTest
    method is an iterator block; if I change it so that it is not, the IL changes to this:
    Code (csharp):
    1.  
    2.         IL_0000: ldloca.s 0
    3.         IL_0002: ldstr "Svelto.ECS - Entities Submission"
    4.         IL_0007: call instance void PlatformProfiler::.ctor(string)
    5.  
     
  14. sebas77

    sebas77

    Joined:
    Nov 4, 2011
    Posts:
    1,387
    that's a newobj of a struct, so it shouldn't cause any allocation. However I now wonder if it boxes, but it shoulnd't:

    The newobj instruction creates a new object or a new instance of a value type

    Value types are not usually created using newobj. They are usually allocated either as arguments or local variables, using newarr (for zero-based, one-dimensional arrays), or as fields of objects. Once allocated, they are initialized using Initobj. However, the newobj instruction can be used to create a new instance of a value type on the stack, that can then be passed as an argument, stored in a local, and so on.


    If you were right, the warm up trick wouldn't have worked btw
     
    Last edited: Jan 30, 2021
  15. superpig

    superpig

    Drink more water! Unity Technologies

    Joined:
    Jan 16, 2011
    Posts:
    4,486
    You're right - I thought perhaps it was an iterator block issue, but writing the same thing out explicitly results in the same IL.

    True; if the warmup trick works then it does imply what we should be looking for is a one-time operation, not something in the IL that would run every time.

    One thing you could try is to use
    Marshal.GetFunctionPointerForDelegate
    to force the delegate to be JITted, without actually executing it. If doing that eliminates the allocation from the assert, then I think we can conclude that the allocation is coming from the JIT process itself.
     
    sebas77 likes this.
unityunity