Clear all settings during a test's setUp

[tatyana-krasnukha]

Summary

Moved from D74903

[labath]

Clearing all settings is a reasonable operation, so I don't have any problem with it per se. I'm just wondering, given that this is not something that one would want to do very often (and it can have catastrophic results), whether this operation should not be more explicit. "settings clear --all" for instance ? Adding @jingham for the interface aspects.

It would also be nice to add a test case for the new command.

[tatyana-krasnukha]

Added --all property to settings clear + added a test

[tatyana-krasnukha]

Removed TestFixIts.py as those changes relate to the parent revision.

[labath]

lldb/source/Commands/CommandObjectSettings.cpp

1085–1088

What will happen if I pass both --all and an actual setting argument. I think that should be an error.

lldb/test/API/commands/settings/TestSettings.py

545–548 ↗

(On Diff #248404)

This dependence on setUpCommands looks weird. Any way we can get rid of that? Maybe if we check the exact four settings that you modified before clear --all and verifying they have the correct (hardcoded) value? (and maybe not use the term-width setting, since the default value of that is unpredictable)

[tatyana-krasnukha]

Addressed comments

[labath]

Awesome, thanks.

[teemperor]

Somehow this broke the macOS LLDB bot:

Failing Tests (2):
    lldb-api :: functionalities/inline-stepping/TestInlineStepping.py
    lldb-api :: lang/cpp/std-function-step-into-callable/TestStdFunctionStepIntoCallable.py

[teemperor]

I guess now all the shady hacks are coming out. The following settings change their value after calling settings clear after startup in the test:

target.process.thread.step-avoid-regexp (regex) -> from '^std::' to empty string
platform.module-cache-directory (file) -> from '"/Users/teemperor/.lldb/module_cache"' to empty string
script-lang (enum) -> from 'default' to 'python'

[tatyana-krasnukha]

Thank you for details, I'm looking at these failures, however, I'm not able to debug on the macOS.

[teemperor]

Sure, I can take care of it. It seems OptionValueRegex::Clear is always resetting to an empty regex. I guess the same goes for the other option values that are failing.

[teemperor]

Fixed in 352f16db87f583ec7f55f8028647b5fd8616111f

[tatyana-krasnukha]

platform.module-cache-directory should be fixed by rGfe74df01a909.

Regarding script-lang, its default value is eScriptLanguagePython in CoreProperties.td and I'm not sure that I should change it.

lldb/packages/Python/lldbsuite/test/lldbtest.py

class Base(unittest2.TestCase):
 
     def getSourcePath(self, name):
         """Return absolute path to a file in the test's source directory."""
         return os.path.join(self.getSourceDir(), name)
 
     @classmethod
     def setUpCommands(cls):
         commands = [
+            # First of all, clear all settings to have clean state of global properties.
+            "settings clear",
+
             # Disable Spotlight lookup. The testsuite creates
             # different binaries with the same UUID, because they only
             # differ in the debug info, which is not being hashed.
             "settings set symbols.enable-external-lookup false",
 
             # Disable fix-its by default so that incorrect expressions in tests don't
             # pass just because Clang thinks it has a fix-it.
             "settings set target.auto-apply-fixits false",

lldb/source/Commands/CommandObjectSettings.cpp

 
 // CommandObjectSettingsClear
 
 class CommandObjectSettingsClear : public CommandObjectParsed {
 public:
   CommandObjectSettingsClear(CommandInterpreter &interpreter)
       : CommandObjectParsed(
             interpreter, "settings clear",
-            "Clear a debugger setting array, dictionary, or string.", nullptr) {
+            "Clear a debugger setting array, dictionary, or string. "
+            "Clear all settings if no arguments were specified.", nullptr) {
     CommandArgumentEntry arg;
     CommandArgumentData var_name_arg;
 
     // Define the first (and only) variant of this arg.
     var_name_arg.arg_type = eArgTypeSettingVariableName;
     var_name_arg.arg_repetition = eArgRepeatPlain;
 
     // There is only one variant this argument could be; put it into the
if (request.GetCursorIndex() < 2)
           GetCommandInterpreter(), CommandCompletions::eSettingsNameCompletion,
           request, nullptr);
   }
 
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
     result.SetStatus(eReturnStatusSuccessFinishNoResult);
     const size_t argc = command.GetArgumentCount();
 
+    if (argc == 0)  {
+      GetDebugger().GetValueProperties()->Clear();
+      return result.Succeeded();
+    }
+
     if (argc != 1) {
-      result.AppendError("'settings clear' takes exactly one argument");
+      result.AppendError("'settings clear' takes at most one argument");
       result.SetStatus(eReturnStatusFailed);
       return false;
     }
 
     const char *var_name = command.GetArgumentAtIndex(0);
     if ((var_name == nullptr) || (var_name[0] == '\0')) {
       result.AppendError("'settings clear' command requires a valid variable "
                          "name; No value supplied");