Implement Godot-in-the-loop test suite and fix debugger errors

[MichaelXt]

Fixes for get variables issues

1. Same reference but different variable override fix, which resulted in variables being lost
   ** Now different GodotVariable instances are used for different variables with same reference
   ** const replacement = {value: rawObject, sub_values: sub_values } as GodotVariable;
2. 'Signal' type handling crash and string representation fix
   ** value.sub_values()?.map
3. Empty scopes return fix
   ** if (this.ongoing_inspections.length === 0 && stackVars.remaining == 0)
4. Various splice from findIndex fixes (where findIndex can return -1)
5. Added variables tests
   ** updated vscode types to version 1.96 to use onDidChangeActiveStackItem for breakpoint hit detection in tests
   1 & 3 should fix Race condition in godot4\debug_session::get_variable() results in adding watch and showing variables stops working. #779

[MichaelXt]

Looks like godot is not installed or not aliased on the test machines. I can disable the tests requiring godot to unblock the pipeline. This way at least local testing will still be possible.

[DaelonSuzuka]

Yeah, until now there haven't been any tests that involved godot. You could figure out how to set that up, wink wink.

[DaelonSuzuka]

At a glance this looks fantastic, thank you for the work setting this up!

I'm going to try and dig into this later today.

[MichaelXt]

This PR is ready.
Regarding tests: The DAP implementation is flaky, hence there are bunch of async sleeps in tests to workaround current DAP limitations.
There are several more issues discovered related to variables and watch window, which need to be addressed in a separate PR.

[MichaelXt]

@DaelonSuzuka, I've re-wrote the variables code to make it more reliable and closer mapped to DAP and godot concepts. Would you like to review it as a separate PR or part of this PR?

[DaelonSuzuka]

Hey @MichaelXt, sorry this has taken me so long to review. My productive hours have nosedived lately and I'm not sure why.

I just got your tests to run locally, so that looks good to me.

I have two minor complaints:

When I first did npm test I got this:

'"godot"' is not recognized as an internal or external command,
operable program or batch file.
'"p:\_godot\godot-vscode-plugin\test_projects\test-dap-project-godot4\godot"' is not recognized as an internal or external command,
operable program or batch file.

My Godot 4 binary was named godot4.exe, and that's what editorPath.godot4 in my extension settings is set to. Can you make the test check that I don't have a godot.exe on my path and explicitly fail? (eventually I hope we can expand this test suite to run against multiple godot binaries, so it's fine to expect a specific naming convention, but silently passing isn't okay)

Then, once I made godot.exe binary, that error went away but there was no additional output in the test terminal. I managed to fix that by running godot --import test_projects/test-dap-project-godot4/project.godot --headless, which I grabbed from ci.yml, and after I did the initial import the tests ran and all passed.

Can you make sure this is also a failing condition?

Thanks again for the contribution, this is a huge upgrade!

[MichaelXt]

@DaelonSuzuka, thanks for taking a look and reviewing!
The test code is not setting any workspace settings, including it doesn't set the godotTools.editorPath.godot4, hence the vscode-test uses following: Global settings, User Settings, Workspace Settings in order to find the godotTools.editorPath.godot4 executable path, default plugin settings.
Those are the current defaults for the plugin:

//file: package.json
		"godotTools.editorPath.godot3": {
			"type": "string",
			"default": "godot3",
			"description": "Path to the Godot 3 editor executable"
		},
		"godotTools.editorPath.godot4": {
			"type": "string",
			"default": "godot",
			"description": "Path to the Godot 4 editor executable"
		},

I don't think we should hardcode the test to use godot4 executable, ignoring all the settings. But if you think that we should add the .vscode/settings.json with hardcoded value to godot4 we could do that. lmk.

Working on executing ${godotTools.editorPath.godot4} --import --headless now.

[DaelonSuzuka]

I don't think we should hardcode the test to use godot4 executable, ignoring all the settings. But if you think that we should add the .vscode/settings.json with hardcoded value to godot4 we could do that. lmk.

Can you make the test runner set godotTools.editorPath.godotX? In the future I want to be able to run tests against, say, Godot 4.3 and 4.4, to test before and after a breaking change.

The most important thing is just making sure it actually fails if the binary is missing, though.

[DaelonSuzuka]

@MichaelXt If it's problematic to run the import command every time, you can just have it check if the import has happened, and have an error telling the user to run npm run import-test-projects or something

[MichaelXt]

set godotTools.editorPath.godotX

Could you please elaborate a bit. do you suggest to call:

const config = vscode.workspace.getConfiguration("godotTools");
config.update("editorPath.godot4", "godot4", vscode.ConfigurationTarget.Workspace);

on suiteSetup? That effectively is the same as adding test_projects/test-dap-project-godot4/.vscode/settings.json with

{
  "godotTools.editorPath.godot4": "godot4"
}

[DaelonSuzuka]

Well I'm still not very familiar with the test runner, so I was trying not to speak out of my depth about the implementation.

I think the first one is what I was describing. The purpose would be to run the same test project using two different Godot versions, so I believe I'd want to set the value of editorPath.godot4 from the js test code, before launching the test environment.

Am I better off making additional test projects? test_projects/test-debug-protocol-godot4.1, test_projects/test-debug-protocol-godot4.2, etc? That would avoid potential issues with re-importing between different version....

This probably isn't worth your time right now, so if you just make sure the test fails if the binary is missing (for whatever reason) then we can call this PR done.

[MichaelXt]

Agree, it is better to have separate project per godot version:

1. The common godot folder is not cross-version compatible
2. The new functionality will require new code in the godot project

Yes, with the addition of godot --import, the project will fail if godot executable is not found (as defined by config).

[DaelonSuzuka]

Thanks for indulging my rambling; this looks good to me.