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.

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

Fix a crash on startup with a folder entry without entries #14629

Merged
5 commits merged into from
Jan 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
107 changes: 107 additions & 0 deletions src/cascadia/LocalTests_SettingsModel/NewTabMenuTests.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

#include "pch.h"

#include "../TerminalSettingsModel/NewTabMenuEntry.h"
#include "../TerminalSettingsModel/FolderEntry.h"
#include "../TerminalSettingsModel/CascadiaSettings.h"
#include "../types/inc/colorTable.hpp"
#include "JsonTestClass.h"

#include <defaults.h>

using namespace Microsoft::Console;
using namespace winrt::Microsoft::Terminal;
using namespace winrt::Microsoft::Terminal::Settings::Model::implementation;
using namespace WEX::Logging;
using namespace WEX::TestExecution;
using namespace WEX::Common;

namespace SettingsModelLocalTests
{
// TODO:microsoft/terminal#3838:
// Unfortunately, these tests _WILL NOT_ work in our CI. We're waiting for
// an updated TAEF that will let us install framework packages when the test
// package is deployed. Until then, these tests won't deploy in CI.

class NewTabMenuTests : public JsonTestClass
{
// Use a custom AppxManifest to ensure that we can activate winrt types
// from our test. This property will tell taef to manually use this as
// the AppxManifest for this test class.
// This does not yet work for anything XAML-y. See TabTests.cpp for more
// details on that.
BEGIN_TEST_CLASS(NewTabMenuTests)
TEST_CLASS_PROPERTY(L"RunAs", L"UAP")
TEST_CLASS_PROPERTY(L"UAP:AppXManifest", L"TestHostAppXManifest.xml")
END_TEST_CLASS()

TEST_METHOD(DefaultsToRemainingProfiles);
TEST_METHOD(ParseEmptyFolder);
};

void NewTabMenuTests::DefaultsToRemainingProfiles()
{
Log::Comment(L"If the user doesn't customize the menu, put one entry for each profile");

static constexpr std::string_view settingsString{ R"json({
})json" };

try
{
const auto settings{ winrt::make_self<CascadiaSettings>(settingsString, DefaultJson) };

VERIFY_ARE_EQUAL(0u, settings->Warnings().Size());

const auto& entries = settings->GlobalSettings().NewTabMenu();
VERIFY_ARE_EQUAL(1u, entries.Size());
VERIFY_ARE_EQUAL(winrt::Microsoft::Terminal::Settings::Model::NewTabMenuEntryType::RemainingProfiles, entries.GetAt(0).Type());
}
catch (const SettingsException& ex)
{
auto loadError = ex.Error();
loadError;
throw ex;
}
catch (const SettingsTypedDeserializationException& e)
{
auto deserializationErrorMessage = til::u8u16(e.what());
Log::Comment(NoThrowString().Format(deserializationErrorMessage.c_str()));
throw e;
}
}

void NewTabMenuTests::ParseEmptyFolder()
{
Log::Comment(L"GH #14557 - An empty folder entry shouldn't crash");

static constexpr std::string_view settingsString{ R"json({
"newTabMenu": [
{ "type": "folder" }
]
})json" };

try
{
const auto settings{ winrt::make_self<CascadiaSettings>(settingsString, DefaultJson) };

VERIFY_ARE_EQUAL(0u, settings->Warnings().Size());

const auto& entries = settings->GlobalSettings().NewTabMenu();
VERIFY_ARE_EQUAL(1u, entries.Size());
}
catch (const SettingsException& ex)
{
auto loadError = ex.Error();
loadError;
throw ex;
}
catch (const SettingsTypedDeserializationException& e)
{
auto deserializationErrorMessage = til::u8u16(e.what());
Log::Comment(NoThrowString().Format(deserializationErrorMessage.c_str()));
throw e;
}
}
}
6 changes: 6 additions & 0 deletions src/cascadia/LocalTests_SettingsModel/SerializationTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,12 @@ namespace SettingsModelLocalTests
"$schema" : "https://aka.ms/terminal-profiles-schema",
"defaultProfile": "{61c54bbd-1111-5271-96e7-009a87ff44bf}",
"disabledProfileSources": [ "Windows.Terminal.Wsl" ],
"newTabMenu":
[
{
"type": "remainingProfiles"
}
],
"profiles": {
"defaults": {
"font": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
<ClCompile Include="KeyBindingsTests.cpp" />
<ClCompile Include="CommandTests.cpp" />
<ClCompile Include="DeserializationTests.cpp" />
<ClCompile Include="NewTabMenuTests.cpp" />
<ClCompile Include="SerializationTests.cpp" />
<ClCompile Include="TerminalSettingsTests.cpp" />
<ClCompile Include="ThemeTests.cpp" />
Expand Down
4 changes: 4 additions & 0 deletions src/cascadia/TerminalSettingsModel/FolderEntry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ IVector<NewTabMenuEntryModel> FolderEntry::Entries() const
// We filter the full list of entries from JSON to just include the
// non-empty ones.
IVector<NewTabMenuEntryModel> result{ winrt::single_threaded_vector<NewTabMenuEntryModel>() };
if (_Entries == nullptr)
{
return result;
}

for (const auto& entry : _Entries)
{
Expand Down