From 00a9e8479c89149c7dd7ab3b7dca59445827d2a0 Mon Sep 17 00:00:00 2001 From: Kaleb Luedtke Date: Tue, 15 Feb 2022 20:21:19 -0600 Subject: [PATCH 1/5] Add Settings and Test Case --- src/AppInstallerCLITests/UserSettings.cpp | 76 +++++++++++++++++++ .../Public/winget/UserSettings.h | 12 +++ src/AppInstallerCommonCore/UserSettings.cpp | 32 ++++++++ 3 files changed, 120 insertions(+) diff --git a/src/AppInstallerCLITests/UserSettings.cpp b/src/AppInstallerCLITests/UserSettings.cpp index 02cb75c6bb..159ca65638 100644 --- a/src/AppInstallerCLITests/UserSettings.cpp +++ b/src/AppInstallerCLITests/UserSettings.cpp @@ -202,6 +202,82 @@ TEST_CASE("SettingProgressBar", "[settings]") } } +TEST_CASE("SettingLoggingLevelPreference", "[settings]") +{ + DeleteUserSettingsFiles(); + + SECTION("Default value") + { + UserSettingsTest userSettingTest; + + REQUIRE(userSettingTest.Get() == LoggingLevel::Info); + REQUIRE(userSettingTest.GetWarnings().size() == 0); + } + SECTION("Info") + { + std::string_view json = R"({ "logging": { "level": "info" } })"; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + REQUIRE(userSettingTest.Get() == LoggingLevel::Info); + REQUIRE(userSettingTest.GetWarnings().size() == 0); + } + SECTION("Verbose") + { + std::string_view json = R"({ "logging": { "level": "verbose" } })"; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + REQUIRE(userSettingTest.Get() == LoggingLevel::Verbose); + REQUIRE(userSettingTest.GetWarnings().size() == 0); + } + SECTION("Warning") + { + std::string_view json = R"({ "logging": { "level": "warning" } })"; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + REQUIRE(userSettingTest.Get() == LoggingLevel::Warning); + REQUIRE(userSettingTest.GetWarnings().size() == 0); + } + SECTION("Error") + { + std::string_view json = R"({ "logging": { "level": "error" } })"; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + REQUIRE(userSettingTest.Get() == LoggingLevel::Error); + REQUIRE(userSettingTest.GetWarnings().size() == 0); + } + SECTION("Critical") + { + std::string_view json = R"({ "logging": { "level": "critical" } })"; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + REQUIRE(userSettingTest.Get() == LoggingLevel::Crit); + REQUIRE(userSettingTest.GetWarnings().size() == 0); + } + SECTION("Bad value") + { + std::string_view json = R"({ "logging": { "level": "fake" } })"; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + REQUIRE(userSettingTest.Get() == LoggingLevel::Info); + REQUIRE(userSettingTest.GetWarnings().size() == 1); + } + SECTION("Bad value type") + { + std::string_view json = R"({ "logging": { "level": 5 } })"; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + REQUIRE(userSettingTest.Get() == LoggingLevel::Info); + REQUIRE(userSettingTest.GetWarnings().size() == 1); + } +} + TEST_CASE("SettingAutoUpdateIntervalInMinutes", "[settings]") { DeleteUserSettingsFiles(); diff --git a/src/AppInstallerCommonCore/Public/winget/UserSettings.h b/src/AppInstallerCommonCore/Public/winget/UserSettings.h index 7cf95b2ec1..8956565180 100644 --- a/src/AppInstallerCommonCore/Public/winget/UserSettings.h +++ b/src/AppInstallerCommonCore/Public/winget/UserSettings.h @@ -56,6 +56,16 @@ namespace AppInstaller::Settings DeliveryOptimization, }; + // The level to use when logging command output + enum class LoggingLevel + { + Verbose, + Info, + Warning, + Error, + Crit, + }; + // Enum of settings. // Must start at 0 to enable direct access to variant in UserSettings. @@ -80,6 +90,7 @@ namespace AppInstaller::Settings InstallArchitectureRequirement, InstallLocalePreference, InstallLocaleRequirement, + LoggingLevelPreference, EFDirectMSI, EnableSelfInitiatedMinidump, Max @@ -131,6 +142,7 @@ namespace AppInstaller::Settings SETTINGMAPPING_SPECIALIZATION(Setting::InstallLocaleRequirement, std::vector, std::vector, {}, ".installBehavior.requirements.locale"sv); SETTINGMAPPING_SPECIALIZATION(Setting::EFDirectMSI, bool, bool, false, ".experimentalFeatures.directMSI"sv); SETTINGMAPPING_SPECIALIZATION(Setting::EnableSelfInitiatedMinidump, bool, bool, false, ".debugging.enableSelfInitiatedMinidump"sv); + SETTINGMAPPING_SPECIALIZATION(Setting::LoggingLevelPreference, std::string, LoggingLevel, LoggingLevel::Info, ".logging.level"sv); // Used to deduce the SettingVariant type; making a variant that includes std::monostate and all SettingMapping types. template diff --git a/src/AppInstallerCommonCore/UserSettings.cpp b/src/AppInstallerCommonCore/UserSettings.cpp index 8ea3b34585..9853b46246 100644 --- a/src/AppInstallerCommonCore/UserSettings.cpp +++ b/src/AppInstallerCommonCore/UserSettings.cpp @@ -316,6 +316,38 @@ namespace AppInstaller::Settings { return std::chrono::seconds(value); } + + WINGET_VALIDATE_SIGNATURE(LoggingLevelPreference) + { + // logging preference possible values + static constexpr std::string_view s_logging_verbose = "verbose"; + static constexpr std::string_view s_logging_info = "info"; + static constexpr std::string_view s_logging_warning = "warning"; + static constexpr std::string_view s_logging_error = "error"; + static constexpr std::string_view s_logging_critical = "critical"; + + if (Utility::CaseInsensitiveEquals(value, s_logging_verbose)) + { + return LoggingLevel::Verbose; + } + else if (Utility::CaseInsensitiveEquals(value, s_logging_info)) + { + return LoggingLevel::Info; + } + else if (Utility::CaseInsensitiveEquals(value, s_logging_warning)) + { + return LoggingLevel::Warning; + } + else if (Utility::CaseInsensitiveEquals(value, s_logging_error)) + { + return LoggingLevel::Error; + } + else if (Utility::CaseInsensitiveEquals(value, s_logging_critical)) + { + return LoggingLevel::Crit; + } + return {}; + } } #ifndef AICLI_DISABLE_TEST_HOOKS From 179b900c1366fc808a94c0da8797c19b75f7ab9f Mon Sep 17 00:00:00 2001 From: Kaleb Luedtke Date: Tue, 15 Feb 2022 20:27:13 -0600 Subject: [PATCH 2/5] Update Settings Schema --- .../JSON/settings/settings.schema.0.2.json | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/schemas/JSON/settings/settings.schema.0.2.json b/schemas/JSON/settings/settings.schema.0.2.json index 2a9ab5bbd8..b839fae7db 100644 --- a/schemas/JSON/settings/settings.schema.0.2.json +++ b/schemas/JSON/settings/settings.schema.0.2.json @@ -31,6 +31,23 @@ } } }, + "Logging": { + "description": "Logging settings", + "type": "object", + "properties": { + "level": { + "description": "Preferred logging level", + "type": "string", + "enum": [ + "verbose", + "info", + "warning", + "error", + "critical" + ] + } + } + }, "InstallPrefReq": { "description": "Shared schema for preferences and requirements", "type": "object", @@ -150,6 +167,12 @@ }, "additionalItems": true }, + { + "properties": { + "logging": { "$ref": "#/definitions/Logging" } + }, + "additionalItems": true + }, { "properties": { "source": { "$ref": "#/definitions/Source" } From 2410f4ae8ac5a350ad124a509fec03b871d7d58f Mon Sep 17 00:00:00 2001 From: Kaleb Luedtke Date: Tue, 15 Feb 2022 21:27:58 -0600 Subject: [PATCH 3/5] Fix Type --- src/AppInstallerCLICore/Core.cpp | 2 +- src/AppInstallerCLITests/UserSettings.cpp | 18 ++++++++++-------- .../Public/winget/UserSettings.h | 16 +++------------- src/AppInstallerCommonCore/UserSettings.cpp | 11 ++++++----- 4 files changed, 20 insertions(+), 27 deletions(-) diff --git a/src/AppInstallerCLICore/Core.cpp b/src/AppInstallerCLICore/Core.cpp index 9baa0b3936..ca7a47d6a3 100644 --- a/src/AppInstallerCLICore/Core.cpp +++ b/src/AppInstallerCLICore/Core.cpp @@ -66,7 +66,7 @@ namespace AppInstaller::CLI // Enable all logging for this phase; we will update once we have the arguments Logging::Log().EnableChannel(Logging::Channel::All); - Logging::Log().SetLevel(Logging::Level::Info); + Logging::Log().SetLevel(Settings::User().Get()); Logging::AddFileLogger(); Logging::EnableWilFailureTelemetry(); diff --git a/src/AppInstallerCLITests/UserSettings.cpp b/src/AppInstallerCLITests/UserSettings.cpp index 159ca65638..dd3ed57d6a 100644 --- a/src/AppInstallerCLITests/UserSettings.cpp +++ b/src/AppInstallerCLITests/UserSettings.cpp @@ -5,6 +5,7 @@ #include "TestSettings.h" #include #include +#include "AppInstallerLogging.h" #include @@ -13,6 +14,7 @@ #include using namespace AppInstaller::Settings; +using namespace AppInstaller::Logging; using namespace AppInstaller::Runtime; using namespace TestCommon; using namespace std::string_literals; @@ -210,7 +212,7 @@ TEST_CASE("SettingLoggingLevelPreference", "[settings]") { UserSettingsTest userSettingTest; - REQUIRE(userSettingTest.Get() == LoggingLevel::Info); + REQUIRE(userSettingTest.Get() == Level::Info); REQUIRE(userSettingTest.GetWarnings().size() == 0); } SECTION("Info") @@ -219,7 +221,7 @@ TEST_CASE("SettingLoggingLevelPreference", "[settings]") SetSetting(Stream::PrimaryUserSettings, json); UserSettingsTest userSettingTest; - REQUIRE(userSettingTest.Get() == LoggingLevel::Info); + REQUIRE(userSettingTest.Get() == Level::Info); REQUIRE(userSettingTest.GetWarnings().size() == 0); } SECTION("Verbose") @@ -228,7 +230,7 @@ TEST_CASE("SettingLoggingLevelPreference", "[settings]") SetSetting(Stream::PrimaryUserSettings, json); UserSettingsTest userSettingTest; - REQUIRE(userSettingTest.Get() == LoggingLevel::Verbose); + REQUIRE(userSettingTest.Get() == Level::Verbose); REQUIRE(userSettingTest.GetWarnings().size() == 0); } SECTION("Warning") @@ -237,7 +239,7 @@ TEST_CASE("SettingLoggingLevelPreference", "[settings]") SetSetting(Stream::PrimaryUserSettings, json); UserSettingsTest userSettingTest; - REQUIRE(userSettingTest.Get() == LoggingLevel::Warning); + REQUIRE(userSettingTest.Get() == Level::Warning); REQUIRE(userSettingTest.GetWarnings().size() == 0); } SECTION("Error") @@ -246,7 +248,7 @@ TEST_CASE("SettingLoggingLevelPreference", "[settings]") SetSetting(Stream::PrimaryUserSettings, json); UserSettingsTest userSettingTest; - REQUIRE(userSettingTest.Get() == LoggingLevel::Error); + REQUIRE(userSettingTest.Get() == Level::Error); REQUIRE(userSettingTest.GetWarnings().size() == 0); } SECTION("Critical") @@ -255,7 +257,7 @@ TEST_CASE("SettingLoggingLevelPreference", "[settings]") SetSetting(Stream::PrimaryUserSettings, json); UserSettingsTest userSettingTest; - REQUIRE(userSettingTest.Get() == LoggingLevel::Crit); + REQUIRE(userSettingTest.Get() == Level::Crit); REQUIRE(userSettingTest.GetWarnings().size() == 0); } SECTION("Bad value") @@ -264,7 +266,7 @@ TEST_CASE("SettingLoggingLevelPreference", "[settings]") SetSetting(Stream::PrimaryUserSettings, json); UserSettingsTest userSettingTest; - REQUIRE(userSettingTest.Get() == LoggingLevel::Info); + REQUIRE(userSettingTest.Get() == Level::Info); REQUIRE(userSettingTest.GetWarnings().size() == 1); } SECTION("Bad value type") @@ -273,7 +275,7 @@ TEST_CASE("SettingLoggingLevelPreference", "[settings]") SetSetting(Stream::PrimaryUserSettings, json); UserSettingsTest userSettingTest; - REQUIRE(userSettingTest.Get() == LoggingLevel::Info); + REQUIRE(userSettingTest.Get() == Level::Info); REQUIRE(userSettingTest.GetWarnings().size() == 1); } } diff --git a/src/AppInstallerCommonCore/Public/winget/UserSettings.h b/src/AppInstallerCommonCore/Public/winget/UserSettings.h index 8956565180..709a471b70 100644 --- a/src/AppInstallerCommonCore/Public/winget/UserSettings.h +++ b/src/AppInstallerCommonCore/Public/winget/UserSettings.h @@ -2,6 +2,7 @@ // Licensed under the MIT License. #pragma once #include "AppInstallerStrings.h" +#include "AppInstallerLogging.h" #include "winget/GroupPolicy.h" #include "winget/Resources.h" @@ -56,17 +57,6 @@ namespace AppInstaller::Settings DeliveryOptimization, }; - // The level to use when logging command output - enum class LoggingLevel - { - Verbose, - Info, - Warning, - Error, - Crit, - }; - - // Enum of settings. // Must start at 0 to enable direct access to variant in UserSettings. // Max must be last and unused. @@ -90,9 +80,9 @@ namespace AppInstaller::Settings InstallArchitectureRequirement, InstallLocalePreference, InstallLocaleRequirement, - LoggingLevelPreference, EFDirectMSI, EnableSelfInitiatedMinidump, + LoggingLevelPreference, Max }; @@ -142,7 +132,7 @@ namespace AppInstaller::Settings SETTINGMAPPING_SPECIALIZATION(Setting::InstallLocaleRequirement, std::vector, std::vector, {}, ".installBehavior.requirements.locale"sv); SETTINGMAPPING_SPECIALIZATION(Setting::EFDirectMSI, bool, bool, false, ".experimentalFeatures.directMSI"sv); SETTINGMAPPING_SPECIALIZATION(Setting::EnableSelfInitiatedMinidump, bool, bool, false, ".debugging.enableSelfInitiatedMinidump"sv); - SETTINGMAPPING_SPECIALIZATION(Setting::LoggingLevelPreference, std::string, LoggingLevel, LoggingLevel::Info, ".logging.level"sv); + SETTINGMAPPING_SPECIALIZATION(Setting::LoggingLevelPreference, std::string, Logging::Level, Logging::Level::Info, ".logging.level"sv); // Used to deduce the SettingVariant type; making a variant that includes std::monostate and all SettingMapping types. template diff --git a/src/AppInstallerCommonCore/UserSettings.cpp b/src/AppInstallerCommonCore/UserSettings.cpp index 9853b46246..110f2efe98 100644 --- a/src/AppInstallerCommonCore/UserSettings.cpp +++ b/src/AppInstallerCommonCore/UserSettings.cpp @@ -16,6 +16,7 @@ namespace AppInstaller::Settings using namespace std::string_view_literals; using namespace Runtime; using namespace Utility; + using namespace Logging; static constexpr std::string_view s_SettingEmpty = R"({ @@ -328,23 +329,23 @@ namespace AppInstaller::Settings if (Utility::CaseInsensitiveEquals(value, s_logging_verbose)) { - return LoggingLevel::Verbose; + return Level::Verbose; } else if (Utility::CaseInsensitiveEquals(value, s_logging_info)) { - return LoggingLevel::Info; + return Level::Info; } else if (Utility::CaseInsensitiveEquals(value, s_logging_warning)) { - return LoggingLevel::Warning; + return Level::Warning; } else if (Utility::CaseInsensitiveEquals(value, s_logging_error)) { - return LoggingLevel::Error; + return Level::Error; } else if (Utility::CaseInsensitiveEquals(value, s_logging_critical)) { - return LoggingLevel::Crit; + return Level::Crit; } return {}; } From 5f345ba7ceb98658cc545ee56a28488d4d4ada1f Mon Sep 17 00:00:00 2001 From: Kaleb Luedtke Date: Tue, 15 Feb 2022 21:44:16 -0600 Subject: [PATCH 4/5] Forgot to update the markdown file --- doc/Settings.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/doc/Settings.md b/doc/Settings.md index 8e463f7b28..b648a7688e 100644 --- a/doc/Settings.md +++ b/doc/Settings.md @@ -106,6 +106,18 @@ See [details on telemetry](../README.md#datatelemetry), and our [primary privacy If set to true, the `telemetry.disable` setting will prevent any event from being written by the program. +## Logging + +The `logging` settings control the level of detail in log files. `--verbose-logs` will override this setting and always creates a verbose log. + +### level + +```json + "logging": { + "level": ["verbose", "info", "warning", "error", "critical"] + }, +``` + ## Network The `network` settings influence how winget uses the network to retrieve packages and metadata. From 7c48bbd02246d5aea7a3f60d12ab740cc556e23c Mon Sep 17 00:00:00 2001 From: Kaleb Luedtke Date: Tue, 15 Feb 2022 22:44:13 -0600 Subject: [PATCH 5/5] Chore: Add info regarding default --- doc/Settings.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/Settings.md b/doc/Settings.md index b648a7688e..b732139c61 100644 --- a/doc/Settings.md +++ b/doc/Settings.md @@ -109,6 +109,7 @@ If set to true, the `telemetry.disable` setting will prevent any event from bein ## Logging The `logging` settings control the level of detail in log files. `--verbose-logs` will override this setting and always creates a verbose log. +Defaults to `info` if value is not set or is invalid ### level