Improve error reporting when missing remote configurations

[omarArm]

Context:

When using the extension for target debugging. If a user forgets to set the port number for the gdb connection, connection is definitely going to fail. However, the reported error by the extension was not clear enough about what is the cause of the error.

Previous behaviour:

The previous reported error message used to be a complete log of the debug-console, which obviously was not user friendly.

Current suggested behaviour:

An error message will now pop up with the value

Port number not specified, cannot connect

I also updated some launch related tests to accommodate the change.

[jonahgraham]

LGTM - please ensure that this is squashed before or as part of the merge and the resulting commit message is a good summary, especially as some of the intermediary commits have merge markers (>>>>) in them

[jonahgraham]

PS There are failing tests that should help guide you on getting the special cases resolved. Let me or @jreineckearm know if you need any additional input

[omarArm]

@jonahgraham Hi Jonah,

I am finally able to pass all of the PR tests, sorry for the insane amount of commits, but I had to update all existing test scripts so that they can follow the new infrastructure of my actual change

[jonahgraham]

@omarArm Sorry that there seems to be some misalignment here on the design and implementation. I hope we can come to a resolution.

I think one thing missing is a better problem statement, something that I (and others) can do to reproduce the bug that you are fixing here.

If you want to discuss further, such as have a zoom call about this, please drop me an email jonah (at) kichwacoders.com and we can perhaps discuss faster - perhaps when @jreineckearm is also available.

[jonahgraham]

@omarArm and I had a good call about this and we identified a likely good solution involving a timeout if the port is not specified and the regexp isn't matched. I look forward to reviewing an updated PR from @omarArm in the coming days.

[omarArm]

Hi @jonahgraham ,

I added the change and tested it manually. However, I don't know how can I come up with a test for the timeout routine, given that the current test framework will always generate a port to be caught by the default regex. Any ideas on that?

[jonahgraham]

Quick answer to test question (I haven't looked at code yet)

Make the timeout configurable as waiting 10 seconds in a test is too long. Then, in a new test, make the regex something that won't match anything so that gdb server starts properly, but the port number is never detected.

HTH

[jonahgraham]

This change looks correct, but I haven't tried to run it yet. When you have some new passing tests let me know and I can look again.

[jreineckearm]

One mild concern is that this change leaves behind running GDB servers. When it ran indefinitely before, a user was forced to click the stop button which terminated the server.

This isn't the only scenario that can lead to this and I am planning to enhance the process management as part of #361.
I guess the main question is whether we can live with this until the process management rework happened.

[jreineckearm]

Changes LGTM, but we probably should wait for a couple of days with merging. Hopefully getting something in for #361 quickly. Which should address the leftover GDB server processes for this scenario, too.

[jonahgraham]

@jreineckearm I'll leave you to merge when it is convenient for you

[jreineckearm]

About to merge #392 . Changes there should in theory help with the leftover processes I could see with this PR. Hence merging this PR here now.

[jreineckearm]

Can confirm that the gdbserver gets terminated if used in combination with #392 .