Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Warnings during settings load #2422

Merged
12 commits merged into from
Aug 16, 2019
Merged

Add Warnings during settings load #2422

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
80c18fb
Warn the user when their settings are bad
zadjii-msft Aug 6, 2019
7a2f0e6
Merge remote-tracking branch 'origin/master' into dev/migrie/f/1348-s…
zadjii-msft Aug 13, 2019
7d6fd68
Display an error dialog for errors during validation
zadjii-msft Aug 13, 2019
3c28f8b
Polish for PR
zadjii-msft Aug 13, 2019
566d6ba
Apply fixes for dustin's thoughts from PR
zadjii-msft Aug 14, 2019
13a8100
Add a proper exception type, use an array instead of a map
zadjii-msft Aug 14, 2019
0b2215a
Merge remote-tracking branch 'origin/master' into dev/migrie/f/1348-s…
zadjii-msft Aug 15, 2019
d149177
PR Fixes
zadjii-msft Aug 15, 2019
6f816f1
guid->GUID
zadjii-msft Aug 15, 2019
cf2a198
Address Michael's PR comments
zadjii-msft Aug 16, 2019
96c9784
Clean up this error text, and catch exceptions better
zadjii-msft Aug 16, 2019
704359b
Update src/cascadia/TerminalApp/Resources/en-US/Resources.resw
zadjii-msft Aug 16, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
326 changes: 326 additions & 0 deletions src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@
#include "precomp.h"

#include "../TerminalApp/ColorScheme.h"
#include "../TerminalApp/CascadiaSettings.h"

using namespace Microsoft::Console;
using namespace TerminalApp;
using namespace WEX::Logging;
using namespace WEX::TestExecution;
using namespace WEX::Common;

namespace TerminalAppLocalTests
{
Expand All @@ -29,8 +31,31 @@ namespace TerminalAppLocalTests
END_TEST_CLASS()

TEST_METHOD(TryCreateWinRTType);
TEST_METHOD(ValidateProfilesExist);
TEST_METHOD(ValidateDefaultProfileExists);
TEST_METHOD(ValidateDuplicateProfiles);
TEST_METHOD(ValidateManyWarnings);

TEST_CLASS_SETUP(ClassSetup)
{
reader = std::unique_ptr<Json::CharReader>(Json::CharReaderBuilder::CharReaderBuilder().newCharReader());
return true;
}
Json::Value VerifyParseSucceeded(std::string content);

private:
std::unique_ptr<Json::CharReader> reader;
};

Json::Value SettingsTests::VerifyParseSucceeded(std::string content)
{
Json::Value root;
std::string errs;
const bool parseResult = reader->parse(content.c_str(), content.c_str() + content.size(), &root, &errs);
VERIFY_IS_TRUE(parseResult, winrt::to_hstring(errs).c_str());
return root;
}

void SettingsTests::TryCreateWinRTType()
{
winrt::Microsoft::Terminal::Settings::TerminalSettings settings{};
Expand All @@ -41,4 +66,305 @@ namespace TerminalAppLocalTests
VERIFY_ARE_NOT_EQUAL(oldFontSize, newFontSize);
}

void SettingsTests::ValidateProfilesExist()
{
const std::string settingsWithProfiles{ R"(
{
"profiles": [
{
"name" : "profile0"
}
]
})" };

const std::string settingsWithoutProfiles{ R"(
{
"defaultProfile": "{6239a42c-1de4-49a3-80bd-e8fdd045185c}"
})" };

const std::string settingsWithEmptyProfiles{ R"(
{
"profiles": []
})" };

{
// Case 1: Good settings
const auto settingsObject = VerifyParseSucceeded(settingsWithProfiles);
auto settings = CascadiaSettings::FromJson(settingsObject);
settings->_ValidateProfilesExist();
}
{
// Case 2: Bad settings
const auto settingsObject = VerifyParseSucceeded(settingsWithoutProfiles);
auto settings = CascadiaSettings::FromJson(settingsObject);
try
{
settings->_ValidateProfilesExist();
}
catch (const ::TerminalApp::SettingsException ex)
{
VERIFY_IS_TRUE(ex.Error() == ::TerminalApp::SettingsLoadErrors::NoProfiles);
}
catch (...)
{
VERIFY_FAIL(L"Expected a SettingsLoadErrors::NoProfiles, but caught another exception type");
}
}
{
// Case 3: Bad settings
const auto settingsObject = VerifyParseSucceeded(settingsWithEmptyProfiles);
auto settings = CascadiaSettings::FromJson(settingsObject);
try
{
settings->_ValidateProfilesExist();
}
catch (const ::TerminalApp::SettingsException ex)
{
VERIFY_IS_TRUE(ex.Error() == ::TerminalApp::SettingsLoadErrors::NoProfiles);
}
catch (...)
{
VERIFY_FAIL(L"Expected a SettingsLoadErrors::NoProfiles, but caught another exception type");
}
}
}

void SettingsTests::ValidateDefaultProfileExists()
{
const std::string goodProfiles{ R"(
{
"globals": {
"defaultProfile": "{6239a42c-1111-49a3-80bd-e8fdd045185c}"
},
"profiles": [
{
"name" : "profile0",
"guid": "{6239a42c-1111-49a3-80bd-e8fdd045185c}"
},
{
"name" : "profile0",
"guid": "{6239a42c-2222-49a3-80bd-e8fdd045185c}"
}
]
})" };

const std::string badProfiles{ R"(
{
"globals": {
"defaultProfile": "{6239a42c-1111-49a3-80bd-e8fdd045185c}"
},
"profiles": [
{
"name" : "profile0",
"guid": "{6239a42c-3333-49a3-80bd-e8fdd045185c}"
},
{
"name" : "profile1",
"guid": "{6239a42c-4444-49a3-80bd-e8fdd045185c}"
}
]
})" };

const std::string noDefaultAtAll{ R"(
{
"globals": {
"alwaysShowTabs": true
},
"profiles": [
{
"name" : "profile0",
"guid": "{6239a42c-5555-49a3-80bd-e8fdd045185c}"
},
{
"name" : "profile1",
"guid": "{6239a42c-6666-49a3-80bd-e8fdd045185c}"
}
]
})" };

{
// Case 1: Good settings
Log::Comment(NoThrowString().Format(
L"Testing a pair of profiles with unique guids, and the defaultProfile is one of those guids"));
const auto settingsObject = VerifyParseSucceeded(goodProfiles);
auto settings = CascadiaSettings::FromJson(settingsObject);
settings->_ValidateDefaultProfileExists();
VERIFY_ARE_EQUAL(static_cast<size_t>(0), settings->_warnings.size());
VERIFY_ARE_EQUAL(static_cast<size_t>(2), settings->_profiles.size());
VERIFY_ARE_EQUAL(settings->_globals.GetDefaultProfile(), settings->_profiles.at(0).GetGuid());
}
{
// Case 2: Bad settings
Log::Comment(NoThrowString().Format(
L"Testing a pair of profiles with unique guids, but the defaultProfile is NOT one of those guids"));
const auto settingsObject = VerifyParseSucceeded(badProfiles);
auto settings = CascadiaSettings::FromJson(settingsObject);
settings->_ValidateDefaultProfileExists();
VERIFY_ARE_EQUAL(static_cast<size_t>(1), settings->_warnings.size());
VERIFY_ARE_EQUAL(::TerminalApp::SettingsLoadWarnings::MissingDefaultProfile, settings->_warnings.at(0));

VERIFY_ARE_EQUAL(static_cast<size_t>(2), settings->_profiles.size());
VERIFY_ARE_EQUAL(settings->_globals.GetDefaultProfile(), settings->_profiles.at(0).GetGuid());
}
{
// Case 2: Bad settings
Log::Comment(NoThrowString().Format(
L"Testing a pair of profiles with unique guids, and no defaultProfile at all"));
const auto settingsObject = VerifyParseSucceeded(badProfiles);
auto settings = CascadiaSettings::FromJson(settingsObject);
settings->_ValidateDefaultProfileExists();
VERIFY_ARE_EQUAL(static_cast<size_t>(1), settings->_warnings.size());
VERIFY_ARE_EQUAL(::TerminalApp::SettingsLoadWarnings::MissingDefaultProfile, settings->_warnings.at(0));

VERIFY_ARE_EQUAL(static_cast<size_t>(2), settings->_profiles.size());
VERIFY_ARE_EQUAL(settings->_globals.GetDefaultProfile(), settings->_profiles.at(0).GetGuid());
}
}

void SettingsTests::ValidateDuplicateProfiles()
{
const std::string goodProfiles{ R"(
{
"profiles": [
{
"name" : "profile0",
"guid": "{6239a42c-1111-49a3-80bd-e8fdd045185c}"
},
{
"name" : "profile0",
"guid": "{6239a42c-2222-49a3-80bd-e8fdd045185c}"
}
]
})" };

const std::string badProfiles{ R"(
{
"profiles": [
{
"name" : "profile0",
"guid": "{6239a42c-3333-49a3-80bd-e8fdd045185c}"
},
{
"name" : "profile1",
"guid": "{6239a42c-3333-49a3-80bd-e8fdd045185c}"
}
]
})" };

const std::string veryBadProfiles{ R"(
{
"profiles": [
{
"name" : "profile0",
"guid": "{6239a42c-4444-49a3-80bd-e8fdd045185c}"
},
{
"name" : "profile1",
"guid": "{6239a42c-5555-49a3-80bd-e8fdd045185c}"
},
{
"name" : "profile2",
"guid": "{6239a42c-4444-49a3-80bd-e8fdd045185c}"
},
{
"name" : "profile3",
"guid": "{6239a42c-4444-49a3-80bd-e8fdd045185c}"
},
{
"name" : "profile4",
"guid": "{6239a42c-6666-49a3-80bd-e8fdd045185c}"
},
{
"name" : "profile5",
"guid": "{6239a42c-5555-49a3-80bd-e8fdd045185c}"
},
{
"name" : "profile6",
"guid": "{6239a42c-7777-49a3-80bd-e8fdd045185c}"
}
]
})" };

{
// Case 1: Good settings
Log::Comment(NoThrowString().Format(
L"Testing a pair of profiles with unique guids"));
const auto settingsObject = VerifyParseSucceeded(goodProfiles);
auto settings = CascadiaSettings::FromJson(settingsObject);
settings->_ValidateNoDuplicateProfiles();
VERIFY_ARE_EQUAL(static_cast<size_t>(0), settings->_warnings.size());
VERIFY_ARE_EQUAL(static_cast<size_t>(2), settings->_profiles.size());
}
{
// Case 2: Bad settings
Log::Comment(NoThrowString().Format(
L"Testing a pair of profiles with the same guid"));
const auto settingsObject = VerifyParseSucceeded(badProfiles);
auto settings = CascadiaSettings::FromJson(settingsObject);

settings->_ValidateNoDuplicateProfiles();

VERIFY_ARE_EQUAL(static_cast<size_t>(1), settings->_warnings.size());
VERIFY_ARE_EQUAL(::TerminalApp::SettingsLoadWarnings::DuplicateProfile, settings->_warnings.at(0));

VERIFY_ARE_EQUAL(static_cast<size_t>(1), settings->_profiles.size());
VERIFY_ARE_EQUAL(L"profile0", settings->_profiles.at(0).GetName());
}
{
// Case 3: Very bad settings
Log::Comment(NoThrowString().Format(
L"Testing a set of profiles, many of which with duplicated guids"));
const auto settingsObject = VerifyParseSucceeded(veryBadProfiles);
auto settings = CascadiaSettings::FromJson(settingsObject);
settings->_ValidateNoDuplicateProfiles();
VERIFY_ARE_EQUAL(static_cast<size_t>(1), settings->_warnings.size());
VERIFY_ARE_EQUAL(::TerminalApp::SettingsLoadWarnings::DuplicateProfile, settings->_warnings.at(0));

VERIFY_ARE_EQUAL(static_cast<size_t>(4), settings->_profiles.size());
VERIFY_ARE_EQUAL(L"profile0", settings->_profiles.at(0).GetName());
VERIFY_ARE_EQUAL(L"profile1", settings->_profiles.at(1).GetName());
VERIFY_ARE_EQUAL(L"profile4", settings->_profiles.at(2).GetName());
VERIFY_ARE_EQUAL(L"profile6", settings->_profiles.at(3).GetName());
}
}

void SettingsTests::ValidateManyWarnings()
{
const std::string badProfiles{ R"(
{
"globals": {
"defaultProfile": "{6239a42c-1111-49a3-80bd-e8fdd045185c}"
},
"profiles": [
{
"name" : "profile0",
"guid": "{6239a42c-2222-49a3-80bd-e8fdd045185c}"
},
{
"name" : "profile1",
"guid": "{6239a42c-3333-49a3-80bd-e8fdd045185c}"
},
{
"name" : "profile2",
"guid": "{6239a42c-2222-49a3-80bd-e8fdd045185c}"
}
]
})" };

// Case 2: Bad settings
Log::Comment(NoThrowString().Format(
L"Testing a pair of profiles with the same guid"));
const auto settingsObject = VerifyParseSucceeded(badProfiles);
auto settings = CascadiaSettings::FromJson(settingsObject);

settings->_ValidateSettings();

VERIFY_ARE_EQUAL(static_cast<size_t>(2), settings->_warnings.size());
VERIFY_ARE_EQUAL(::TerminalApp::SettingsLoadWarnings::DuplicateProfile, settings->_warnings.at(0));
VERIFY_ARE_EQUAL(::TerminalApp::SettingsLoadWarnings::MissingDefaultProfile, settings->_warnings.at(1));

VERIFY_ARE_EQUAL(static_cast<size_t>(2), settings->_profiles.size());
VERIFY_ARE_EQUAL(settings->_globals.GetDefaultProfile(), settings->_profiles.at(0).GetGuid());
}

}
Loading