[Lldb-commits] [PATCH] D74579: [don't review]Creating environment variable test for lldbd

[walter erquinigo via Phabricator via lldb-commits]

wallace requested changes to this revision.
wallace added inline comments.
This revision now requires changes to proceed.


================
Comment at: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/TestVSCode_completions.py:26
     @skipIfWindows
-    @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
+    #@skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
     def test_completions(self):
----------------
don't forget to remove the #


================
Comment at: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/environmentVariables/TestVSCode_environmentVariables.py:19-24
+    def verify_completions(self, actual_list, expected_list, not_expected_list=[]):
+        for expected_item in expected_list:
+            self.assertTrue(expected_item in actual_list)
+
+        for not_expected_item in not_expected_list:
+            self.assertFalse(not_expected_item in actual_list)
----------------
remove this


================
Comment at: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/environmentVariables/TestVSCode_environmentVariables.py:38-43
+        found = False
+        for line in lines:
+            if line.startswith('PATH='):
+                found = True
+                self.assertTrue(path_env_variable == line, "PATH environment variable not the same")
+        self.assertTrue(found, 'PATH environment variable not found')
----------------
you can write different tests:
- inheritEnvironment = false -> no PATH
- inheritEnvironment = true
 --  send an additional env in the config -> it should override the inherited
 -- PATH should be correct 


================
Comment at: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py:906
                                       stopCommands=options.stopCmds,
                                       exitCommands=options.exitCmds)
 
----------------
inheritEnv should be passed here


================
Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:1400-1412
   auto envs = GetStrings(arguments, "env");
-  if (!envs.empty())
+
+  // if arguments has launchWithDebuggerEnvironment
+  // then append current environment to g_vsc.launch_info
+  if (launchWithDebuggerEnvironment) {
+  // if (true) {
+    char** env_var_pointer = environ;
----------------
the env sent by the user in the launch.json config should have more priority than the inherited environment. For example, if env = ["X=A"] and the inherited env is ["X=Z", "Y=V"], then the resulting environment should be ["X=A", "Y=V"].
You should also write a test for that


================
Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:1405
+  if (launchWithDebuggerEnvironment) {
+  // if (true) {
+    char** env_var_pointer = environ;
----------------
remove this


================
Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:2250-2251
 void request_stepIn(const llvm::json::Object &request) {
+
+  return;
   llvm::json::Object response;
----------------
lol


================
Comment at: lldb/tools/lldb-vscode/package.json:89
+								"type": "boolean",
+								"description": "Inherit the VSCode Environment Variables",
+								"default": false
----------------
Inherit the debugger environment when launching a process. Only works for binaries launched directly by LLDB.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74579/new/

https://reviews.llvm.org/D74579