microsoft / vscode-makefile-tools

MAKE integration in Visual Studio Code
Other
194 stars 58 forks source link

Dev/andris/makefile tools/fixes #491

Closed andreeis closed 1 year ago

andreeis commented 1 year ago

This PR addresses the blocking bug https://github.com/microsoft/vscode-makefile-tools/issues/489, along with a few minor issues I noticed around commands in the palette.

Currently all PRs are blocked because while running tests we cannot trigger a quickPick and wait for user selection. This is as a hang for the tests infrastructure. So far, we've been avoiding opening quickPicks in test mode but now with several code changes and a combination of factors (related to variable expansion and reading settings more times to propagate updates plus something else we did not identify, because this did not reproduce when the feature was merged in) we got a new such situation.

One test is setting launch configurations, which usually writes into settings.json but we avoided so far writing updates into the settings.json of the tests to keep cleanup simple. Since now (after variable expansion) we read settings more times than before (because some values depend on each other and we have a 2 pass evaluation) we detect a launch configuration that is not present in settings.json, where usually for production code we would (correctly) unset the launch configuration. We could either not unset the launch config in such scenario for tests... or we could start writing into settings.json and ensure proper cleanup and deterministic outcomes in case tests stop in the middle. Both approaches end up having some ugly hacks being needed but I chose the latest because it appeared to have the fewest cons.

Additionally, writing into settings.json triggers some settings updates events which lead to read settings again. This caused lots of extra logging about reading settings. I updated some baselines to account for that but where it was possible to identify a logging to be redundant we skipped over writing the message.

I'll write more comments now on the PR directly near the lines that need some extra explanations.