MikeSchulze / gdUnit4Net

A Godot C# Unit Test Framework.
MIT License
30 stars 3 forks source link

GD-57: Await on C# executor throws error on non-existing signal `ExecutionCompleted` #57

Closed ngburke closed 1 month ago

ngburke commented 2 months ago

The used GdUnit4 version

4.2.1 (Pre Release/Master branch)

The used Godot version

v4.2.1.stable.mono.official [b09f793f5]

Operating System

macOS Ventura

Describe the bug

When running a long set of C# test suites, after ~50% of them complete and pass I hit the following error. Once this fires, the tests no longer run as the runner has crashed.

`E 0:00:11:0413 GdUnitRunner.gd:83 @ _process(): In Object of type 'RefCounted': Attempt to connect nonexistent signal '' to callable 'GDScriptFunctionState::_signal_callback'. <C++ Error> Condition "!signal_is_valid" is true. Returning: ERR_INVALID_PARAMETER <C++ Source> core/object/object.cpp:1344 @ connect()

GdUnitRunner.gd:83 @ _process() ` Some context showing where the error was triggered is in the screenshot below: Screenshot 2024-03-06 at 11 15 25 PM ### Steps to Reproduce Very difficult to nail this down to a smaller test case. I only saw this begin after accumulating test suites after some time. Attempting to disable certain test suites only makes the problem happen on the next test suite after the disabled one. ### Minimal reproduction project _No response_
ngburke commented 2 months ago

Looks like this is probably a duplicate of https://github.com/MikeSchulze/gdUnit4Mono/issues/42.

I'm using <PackageReference Include="gdUnit4.api" Version="4.2.1" />

For my situation it was after the 9th test suite was run 🤷🏻

MikeSchulze commented 2 months ago

This bug should already be fixed, yes is a duplicate to https://github.com/MikeSchulze/gdUnit4Mono/issues/42 But you are able to reproduce with gdUnit4.api Version="4.2.1" ?

ngburke commented 2 months ago

This bug should already be fixed, yes is a duplicate to MikeSchulze/gdUnit4Mono#42 But you are able to reproduce with gdUnit4.api Version="4.2.1" ?

Yes I can reproduce it with 4.2.1, here is my csproj file

<Project Sdk="Godot.NET.Sdk/4.2.1">
  <PropertyGroup>
    <TargetFramework>net7.0</TargetFramework>
    <EnableDynamicLoading>true</EnableDynamicLoading>
  </PropertyGroup>
  <ItemGroup>
    <!--Required for GdUnit4-->
    <PackageReference Include="gdUnit4.api" Version="4.2.1" />
  </ItemGroup>
</Project>

Perhaps the workaround could be some polling of some sort? Not super elegant, but it may get the job done?

MikeSchulze commented 2 months ago

The issue is related to a Godot bug I don't want to introduce a polling mechanism as a workaround.

ngburke commented 2 months ago

The issue is related to a Godot bug I don't want to introduce a polling mechanism as a workaround.

I came up with a hacky workaround on the GDScript side to stop looking at the signal and just wait some amount of time (for both the Godot editor runner and command line) so I can continue limping along for now 😄

RUN:
    # all test suites executed
    if _test_suites_to_process.is_empty():
        _state = STOP
    else:
        # process next test suite
        set_process(false)
        var test_suite :Node = _test_suites_to_process.pop_front()
        if _cs_executor != null and _cs_executor.IsExecutable(test_suite):
            _cs_executor.Execute(test_suite)
            var connections = _cs_executor.get_signal_connection_list("ExecutionCompleted")
            if connections.is_empty():
                _signals_lost = true
            if _signals_lost:
                await get_tree().create_timer(2).timeout
            else:
                await _cs_executor.ExecutionCompleted
        else:
            await _executor.execute(test_suite)
        set_process(true)

If you haven't tried already, perhaps you can see if the workaround mentioned in the Godot issue https://github.com/godotengine/godot/issues/84254 with the await Task.yield() prior to emitting the signal on the C# side will work with your minimal reproduction project from before (the 7 runs). Maybe that's a more robust workaround? You may have already tried this, if so please ignore this suggestion!

MikeSchulze commented 2 months ago

Thanks for your feedback, i will have a look to day to test the await Task.Yield(); workaround 👍

MikeSchulze commented 2 months ago

It looks like it could be fixed by https://github.com/godotengine/godot/issues/84254

MikeSchulze commented 1 month ago

I'm not able to reproduce with GdUnit4 v4.2.4 and GdUnit4Net v4.2.2

Godot Engine v4.2.1.stable.mono.official (c) 2007-present Juan Linietsky, Ariel Manzur & Godot Contributors.
  modules/gltf/register_types.cpp:63 - Der Blend-Dateiimport ist in den Projekteinstellungen aktiviert, aber es wurde kein Blender-Pfad in den Editoreinstellungen festgelegt. Es werden keine Blend-Dateien importiert.
--- Debug adapter server started ---
--- GDScript language server started on port 6005 ---
Loading GdUnit4 Plugin success
GdUnit4Net version '4.2.2.0' loaded.
GdUnit4: Test server successfully started checked port: 31002

cs project file

<Project Sdk="Godot.NET.Sdk/4.2.1">
  <PropertyGroup>
    <TargetFrameworks>net8.0</TargetFrameworks>
    <LangVersion>11.0</LangVersion>
    <!--Force nullable warnings, you can disable if you want-->
    <Nullable>enable</Nullable>
    <CopyLocalLockFileAssemblies>true</CopyLocalLockFileAssemblies>
    <!--Disable warning of invalid/incompatible GodotSharp version-->
    <NoWarn>NU1605</NoWarn>
  </PropertyGroup>
  <ItemGroup>
    <!--Required for GdUnit4-->
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.9.0" />
    <PackageReference Include="gdUnit4.api" Version="4.2.*-*" />
    <PackageReference Include="gdUnit4.test.adapter" Version="1.*" />
  </ItemGroup>
</Project>

Test template:

// GdUnit generated TestSuite
using Godot;
using GdUnit4;
using System;
using System.Threading.Tasks;

namespace GdUnit4
{
    using static Assertions;
    using static Utils;

    [TestSuite]
    public partial class ExampleTestSuite9
    {
        [TestCase]
        public async Task IsFooAsync()
        {
            await ISceneRunner.SyncPhysicsFrame;
            AssertThat("Foo").IsEqual("Foo");
        }

        [TestCase]
        public async Task IsBarAsync()
        {
            await ISceneRunner.SyncPhysicsFrame;
            AssertThat("Foo").IsEqual("Foo");
        }
    }
}

image