From c0bcf3bfdf6814846c5f4ffdd66d8c68b5e9ac81 Mon Sep 17 00:00:00 2001 From: Fabrice de Gans Date: Wed, 8 May 2024 13:08:07 -0700 Subject: [PATCH] [Test] Add tests for many config classes This also fixes a coupld of minor issues that were found while adding tests. --- src/wx/config/CMakeLists.txt | 5 +- src/wx/config/command-test.cpp | 51 +++++ src/wx/config/command.cpp | 3 +- src/wx/config/option-test.cpp | 160 ++++++++++++++ src/wx/config/option.cpp | 3 + .../{strutils_test.cpp => strutils-test.cpp} | 0 src/wx/config/user-input-test.cpp | 203 ++++++++++++++++++ src/wx/config/user-input.cpp | 7 +- 8 files changed, 429 insertions(+), 3 deletions(-) create mode 100644 src/wx/config/command-test.cpp create mode 100644 src/wx/config/option-test.cpp rename src/wx/config/{strutils_test.cpp => strutils-test.cpp} (100%) create mode 100644 src/wx/config/user-input-test.cpp diff --git a/src/wx/config/CMakeLists.txt b/src/wx/config/CMakeLists.txt index 74369fe6..4bca610d 100644 --- a/src/wx/config/CMakeLists.txt +++ b/src/wx/config/CMakeLists.txt @@ -72,7 +72,10 @@ target_include_directories(vbam-wx-config PUBLIC ${NONSTD_INCLUDE_DIR}) if (BUILD_TESTING) add_executable(vbam-wx-config-tests - strutils_test.cpp + command-test.cpp + option-test.cpp + strutils-test.cpp + user-input-test.cpp ) target_link_libraries(vbam-wx-config-tests vbam-core diff --git a/src/wx/config/command-test.cpp b/src/wx/config/command-test.cpp new file mode 100644 index 00000000..52dbd31b --- /dev/null +++ b/src/wx/config/command-test.cpp @@ -0,0 +1,51 @@ +#include "wx/config/command.h" + +#include + +#include + +TEST(GameCommandTest, Basic) { + config::GameCommand command(config::GameJoy(0), config::GameKey::Up); + + EXPECT_EQ(command.joypad(), config::GameJoy(0)); + EXPECT_EQ(command.game_key(), config::GameKey::Up); + EXPECT_EQ(command.ToConfigString(), "Joypad/1/Up"); +} + +TEST(ShortcutCommandTest, Basic) { + config::ShortcutCommand command(wxID_OPEN); + + EXPECT_EQ(command.id(), wxID_OPEN); + EXPECT_EQ(command.ToConfigString(), "Keyboard/OPEN"); +} + +TEST(CommandTest, FromString) { + const auto game_command = config::Command::FromString("Joypad/1/Up"); + ASSERT_TRUE(game_command.has_value()); + ASSERT_TRUE(game_command->is_game()); + + const config::GameCommand& game = game_command->game(); + EXPECT_EQ(game.joypad(), config::GameJoy(0)); + EXPECT_EQ(game.game_key(), config::GameKey::Up); + + const auto shortcut_command = config::Command::FromString("Keyboard/OPEN"); + ASSERT_TRUE(shortcut_command.has_value()); + ASSERT_TRUE(shortcut_command->is_shortcut()); + + const config::ShortcutCommand& shortcut = shortcut_command->shortcut(); + EXPECT_EQ(shortcut.id(), wxID_OPEN); +} + +TEST(CommandTest, FromStringInvalid) { + // Need to disable logging to test for errors. + const wxLogNull disable_logging; + + const auto game_command = config::Command::FromString("Joypad/1/Invalid"); + EXPECT_FALSE(game_command.has_value()); + + const auto shortcut_command = config::Command::FromString("Keyboard/INVALID"); + EXPECT_FALSE(shortcut_command.has_value()); + + const auto invalid_command = config::Command::FromString("INVALID"); + EXPECT_FALSE(invalid_command.has_value()); +} diff --git a/src/wx/config/command.cpp b/src/wx/config/command.cpp index daf1e462..f8c3db31 100644 --- a/src/wx/config/command.cpp +++ b/src/wx/config/command.cpp @@ -1,5 +1,6 @@ #include "wx/config/command.h" +#include #include #include @@ -159,7 +160,7 @@ nonstd::optional Command::FromString(const wxString& name) { const auto iter = std::lower_bound(cmdtab.begin(), cmdtab.end(), cmditem{parts[1], wxString(), 0, 0, NULL}, cmditem_lt); - if (iter == cmdtab.end()) { + if (iter == cmdtab.end() || iter->cmd != parts[1]) { wxLogDebug("Command ID %s not found", parts[1]); return nonstd::nullopt; } diff --git a/src/wx/config/option-test.cpp b/src/wx/config/option-test.cpp new file mode 100644 index 00000000..82a19e1c --- /dev/null +++ b/src/wx/config/option-test.cpp @@ -0,0 +1,160 @@ +#include "wx/config/option.h" + +#include + +#include + +#include "wx/config/option-id.h" +#include "wx/config/option-proxy.h" + +// Basic tests for a boolean option. +TEST(OptionTest, Bool) { + config::Option* option = config::Option::ByID(config::OptionID::kDispBilinear); + ASSERT_TRUE(option); + + EXPECT_EQ(option->command(), "Bilinear"); + EXPECT_EQ(option->config_name(), "Display/Bilinear"); + EXPECT_EQ(option->id(), config::OptionID::kDispBilinear); + EXPECT_EQ(option->type(), config::Option::Type::kBool); + EXPECT_TRUE(option->is_bool()); + + EXPECT_TRUE(option->SetBool(false)); + EXPECT_FALSE(option->GetBool()); + +#if !defined(NDEBUG) + EXPECT_DEATH(option->SetDouble(2.0), ".*"); + EXPECT_DEATH(option->SetInt(2), ".*"); + EXPECT_DEATH(option->SetUnsigned(2), ".*"); + EXPECT_DEATH(option->SetString("foo"), ".*"); + EXPECT_DEATH(option->SetFilter(config::Filter::kNone), ".*"); + EXPECT_DEATH(option->SetInterframe(config::Interframe::kNone), ".*"); + EXPECT_DEATH(option->SetRenderMethod(config::RenderMethod::kSimple), ".*"); + EXPECT_DEATH(option->SetAudioApi(config::AudioApi::kOpenAL), ".*"); + EXPECT_DEATH(option->SetAudioRate(config::AudioRate::k11kHz), ".*"); + EXPECT_DEATH(option->SetGbPalette({0, 1, 2, 3, 4, 5, 6, 7}), ".*"); +#endif // !defined(NDEBUG) +} + +TEST(OptionTest, Double) { + config::Option* option = config::Option::ByID(config::OptionID::kDispScale); + ASSERT_TRUE(option); + + EXPECT_TRUE(option->command().empty()); + EXPECT_EQ(option->config_name(), "Display/Scale"); + EXPECT_EQ(option->id(), config::OptionID::kDispScale); + EXPECT_EQ(option->type(), config::Option::Type::kDouble); + EXPECT_TRUE(option->is_double()); + + EXPECT_TRUE(option->SetDouble(2.5)); + EXPECT_DOUBLE_EQ(option->GetDouble(), 2.5); + + // Need to disable logging to test for errors. + const wxLogNull disable_logging; + + // Test out of bounds values. + EXPECT_FALSE(option->SetDouble(-1.0)); + EXPECT_FALSE(option->SetDouble(7.0)); + EXPECT_DOUBLE_EQ(option->GetDouble(), 2.5); + +#if !defined(NDEBUG) + EXPECT_DEATH(option->SetBool(true), ".*"); + EXPECT_DEATH(option->SetInt(2), ".*"); + EXPECT_DEATH(option->SetUnsigned(2), ".*"); + EXPECT_DEATH(option->SetString("foo"), ".*"); + EXPECT_DEATH(option->SetFilter(config::Filter::kNone), ".*"); + EXPECT_DEATH(option->SetInterframe(config::Interframe::kNone), ".*"); + EXPECT_DEATH(option->SetRenderMethod(config::RenderMethod::kSimple), ".*"); + EXPECT_DEATH(option->SetAudioApi(config::AudioApi::kOpenAL), ".*"); + EXPECT_DEATH(option->SetAudioRate(config::AudioRate::k11kHz), ".*"); + EXPECT_DEATH(option->SetGbPalette({0, 1, 2, 3, 4, 5, 6, 7}), ".*"); +#endif // !defined(NDEBUG) +} + +TEST(OptionTest, Enum) { + config::Option* option = config::Option::ByID(config::OptionID::kDispRenderMethod); + ASSERT_TRUE(option); + + EXPECT_TRUE(option->command().empty()); + EXPECT_EQ(option->config_name(), "Display/RenderMethod"); + EXPECT_EQ(option->id(), config::OptionID::kDispRenderMethod); + EXPECT_EQ(option->type(), config::Option::Type::kRenderMethod); + EXPECT_TRUE(option->is_render_method()); + + EXPECT_TRUE(option->SetRenderMethod(config::RenderMethod::kSimple)); + EXPECT_EQ(option->GetRenderMethod(), config::RenderMethod::kSimple); + + EXPECT_TRUE(option->SetEnumString("opengl")); + EXPECT_EQ(option->GetRenderMethod(), config::RenderMethod::kOpenGL); +} + +TEST(Optiontest, GbPalette) { + config::Option* option = config::Option::ByID(config::OptionID::kGBPalette0); + ASSERT_TRUE(option); + + EXPECT_TRUE(option->command().empty()); + EXPECT_EQ(option->config_name(), "GB/Palette0"); + EXPECT_EQ(option->id(), config::OptionID::kGBPalette0); + EXPECT_EQ(option->type(), config::Option::Type::kGbPalette); + EXPECT_TRUE(option->is_gb_palette()); + + const std::array palette = {0, 1, 2, 3, 4, 5, 6, 7}; + EXPECT_TRUE(option->SetGbPalette(palette)); + EXPECT_EQ(option->GetGbPalette(), palette); + + EXPECT_EQ(option->GetGbPaletteString(), "0000,0001,0002,0003,0004,0005,0006,0007"); + + // Need to disable logging to test for errors. + const wxLogNull disable_logging; + + // Incorrect configuration string tests + EXPECT_FALSE(option->SetGbPaletteString("")); + EXPECT_FALSE(option->SetGbPaletteString("0000,0001,0002,0003,0004,0005,0006,000Q")); + +} + +TEST(OptionObserverTest, Basic) { + class TestOptionsObserver : public config::Option::Observer { + public: + TestOptionsObserver(config::OptionID id) : config::Option::Observer(id), changed_(false) {} + ~TestOptionsObserver() override = default; + + void OnValueChanged() override { changed_ = true; } + + bool changed() const { return changed_; } + void ResetChanged() { changed_ = false; } + + private: + bool changed_; + }; + + config::Option* option = config::Option::ByID(config::OptionID::kDispKeepOnTop); + ASSERT_TRUE(option); + const bool initial_value = option->GetBool(); + + TestOptionsObserver observer(config::OptionID::kDispKeepOnTop); + + // Test that the observer is not notified when the value does not change. + EXPECT_TRUE(option->SetBool(initial_value)); + EXPECT_FALSE(observer.changed()); + observer.ResetChanged(); + + // Test that the observer is notified when the value changes. + EXPECT_TRUE(option->SetBool(!initial_value)); + EXPECT_TRUE(observer.changed()); + observer.ResetChanged(); + + EXPECT_TRUE(option->SetBool(initial_value)); + EXPECT_TRUE(observer.changed()); +} + +// Tests that the option proxy correctly matches the types of the options. +TEST(OptionProxyTest, MatchingTypes) { + for (size_t i = 0; i < config::kNbOptions; i++) { + const config::OptionID id = static_cast(i); + const config::Option::Type proxy_type = config::kOptionsTypes[i]; + const config::Option* option = config::Option::ByID(id); + + ASSERT_TRUE(option); + EXPECT_EQ(option->type(), proxy_type); + } +} diff --git a/src/wx/config/option.cpp b/src/wx/config/option.cpp index 1851ac1e..ab579b3e 100644 --- a/src/wx/config/option.cpp +++ b/src/wx/config/option.cpp @@ -487,6 +487,9 @@ bool Option::SetGbPaletteString(const wxString& value) { long temp = 0; if (number.ToLong(&temp, 16)) { new_value[i] = temp; + } else { + wxLogWarning(_("Invalid value %s for option %s"), value, config_name_); + return false; } } diff --git a/src/wx/config/strutils_test.cpp b/src/wx/config/strutils-test.cpp similarity index 100% rename from src/wx/config/strutils_test.cpp rename to src/wx/config/strutils-test.cpp diff --git a/src/wx/config/user-input-test.cpp b/src/wx/config/user-input-test.cpp new file mode 100644 index 00000000..b9c8a6b7 --- /dev/null +++ b/src/wx/config/user-input-test.cpp @@ -0,0 +1,203 @@ +#include "wx/config/user-input.h" + +#include +#include + +TEST(KeyboardInputTest, Basic) { + config::KeyboardInput input(wxKeyCode::WXK_F1); + + EXPECT_EQ(input.key(), wxKeyCode::WXK_F1); + EXPECT_EQ(input.mod(), wxKeyModifier::wxMOD_NONE); + EXPECT_EQ(input.ToConfigString(), "F1"); +} + +TEST(KeyboardInputTest, Modifiers) { + config::KeyboardInput input(wxKeyCode::WXK_F1, wxMOD_SHIFT); + EXPECT_EQ(input.key(), wxKeyCode::WXK_F1); + EXPECT_EQ(input.mod(), wxMOD_SHIFT); + EXPECT_EQ(input.ToConfigString(), "SHIFT+F1"); +} + +TEST(KeyboardInputTest, Char) { + config::KeyboardInput input('A'); + + EXPECT_EQ(input.key(), 'A'); + EXPECT_EQ(input.mod(), wxKeyModifier::wxMOD_NONE); + EXPECT_EQ(input.ToConfigString(), "A"); +} + +TEST(KeyboardInputTest, MultipleModifiers) { + config::KeyboardInput input('A', static_cast(wxMOD_SHIFT | wxMOD_CONTROL)); + + EXPECT_EQ(input.key(), 'A'); + EXPECT_EQ(input.mod(), static_cast(wxMOD_SHIFT | wxMOD_CONTROL)); + EXPECT_EQ(input.ToConfigString(), "CTRL+SHIFT+A"); +} + +TEST(KeyboardInputTest, ModOnly) { + config::KeyboardInput input(wxKeyCode::WXK_SHIFT, wxMOD_SHIFT); + + EXPECT_EQ(input.key(), wxKeyCode::WXK_SHIFT); + EXPECT_EQ(input.mod(), wxMOD_SHIFT); + EXPECT_EQ(input.ToConfigString(), "SHIFT"); +} + +TEST(KeyboardInputTest, FromBasicConfigString) { + const std::unordered_set inputs = config::UserInput::FromConfigString("F1"); + ASSERT_EQ(inputs.size(), 1); + + const config::UserInput input = *inputs.begin(); + ASSERT_TRUE(input.is_keyboard()); + + EXPECT_EQ(input.keyboard_input().key(), wxKeyCode::WXK_F1); + EXPECT_EQ(input.keyboard_input().mod(), wxKeyModifier::wxMOD_NONE); +} + +TEST(KeyboardInputTest, FromConfigStringWithModifiers) { + const std::unordered_set inputs = + config::UserInput::FromConfigString("SHIFT+F1"); + ASSERT_EQ(inputs.size(), 1); + + const config::UserInput input = *inputs.begin(); + ASSERT_TRUE(input.is_keyboard()); + + EXPECT_EQ(input.keyboard_input().key(), wxKeyCode::WXK_F1); + EXPECT_EQ(input.keyboard_input().mod(), wxMOD_SHIFT); +} + +TEST(KeyboardInputTest, FromCommaConfigString) { + // A standalone comma should parse as a keyboard input. + const std::unordered_set inputs = config::UserInput::FromConfigString(","); + ASSERT_EQ(inputs.size(), 1); + + const config::UserInput input = *inputs.begin(); + ASSERT_TRUE(input.is_keyboard()); + + EXPECT_EQ(input.keyboard_input().key(), ','); + EXPECT_EQ(input.keyboard_input().mod(), wxMOD_NONE); +} + +TEST(KeyboardInputTest, CommaConfigString) { + const config::KeyboardInput input(','); + EXPECT_EQ(input.key(), ','); + EXPECT_EQ(input.mod(), wxMOD_NONE); + EXPECT_EQ(input.ToConfigString(), "44:0"); + + // Parse the config string back. + const std::unordered_set inputs = + config::UserInput::FromConfigString("44:0,44:1"); + ASSERT_EQ(inputs.size(), 2); + + EXPECT_TRUE(inputs.find(config::KeyboardInput(',', wxMOD_NONE)) != inputs.end()); + EXPECT_TRUE(inputs.find(config::KeyboardInput(',', wxMOD_ALT)) != inputs.end()); +} + +TEST(KeyboardInputTest, FromColonConfigString) { + // A standalone colon should parse as a keyboard input. + const std::unordered_set inputs = config::UserInput::FromConfigString(":"); + ASSERT_EQ(inputs.size(), 1); + + const config::UserInput input = *inputs.begin(); + ASSERT_TRUE(input.is_keyboard()); + + EXPECT_EQ(input.keyboard_input().key(), ':'); + EXPECT_EQ(input.keyboard_input().mod(), wxMOD_NONE); +} + +TEST(KeyboardInputTest, ColonConfigString) { + const config::KeyboardInput input(':'); + EXPECT_EQ(input.key(), ':'); + EXPECT_EQ(input.mod(), wxMOD_NONE); + EXPECT_EQ(input.ToConfigString(), "58:0"); + + // Parse the config string back. + const std::unordered_set inputs = + config::UserInput::FromConfigString("58:0,58:1"); + ASSERT_EQ(inputs.size(), 2); + + EXPECT_TRUE(inputs.find(config::KeyboardInput(':', wxMOD_NONE)) != inputs.end()); + EXPECT_TRUE(inputs.find(config::KeyboardInput(':', wxMOD_ALT)) != inputs.end()); +} + +TEST(KeyboardInputTest, ColonCommaConfigString) { + const std::unordered_set inputs = config::UserInput::FromConfigString(",,:"); + ASSERT_EQ(inputs.size(), 2); + + EXPECT_TRUE(inputs.find(config::KeyboardInput(',', wxMOD_NONE)) != inputs.end()); + EXPECT_TRUE(inputs.find(config::KeyboardInput(':', wxMOD_NONE)) != inputs.end()); +} + +TEST(JoyInputTest, Basic) { + config::JoyInput input(config::JoyId(0), config::JoyControl::AxisPlus, 2); + + EXPECT_EQ(input.joy(), config::JoyId(0)); + EXPECT_EQ(input.control(), config::JoyControl::AxisPlus); + EXPECT_EQ(input.control_index(), 2); + EXPECT_EQ(input.ToConfigString(), "Joy1-Axis2+"); +} + +TEST(JoyInputTest, FromAxisConfigString) { + const std::unordered_set inputs = + config::UserInput::FromConfigString("Joy1-Axis2+"); + ASSERT_EQ(inputs.size(), 1); + + const config::UserInput input = *inputs.begin(); + ASSERT_TRUE(input.is_joystick()); + + EXPECT_EQ(input.joy_input().joy(), config::JoyId(0)); + EXPECT_EQ(input.joy_input().control(), config::JoyControl::AxisPlus); + EXPECT_EQ(input.joy_input().control_index(), 2); +} + +TEST(JoyInputTest, FromHatConfigString) { + const std::unordered_set inputs = + config::UserInput::FromConfigString("Joy1-Hat0N"); + ASSERT_EQ(inputs.size(), 1); + + const config::UserInput input = *inputs.begin(); + ASSERT_TRUE(input.is_joystick()); + + EXPECT_EQ(input.joy_input().joy(), config::JoyId(0)); + EXPECT_EQ(input.joy_input().control(), config::JoyControl::HatNorth); + EXPECT_EQ(input.joy_input().control_index(), 0); +} + +TEST(JoyInputTest, FromButtonConfigString) { + const std::unordered_set inputs = + config::UserInput::FromConfigString("Joy10-Button20"); + ASSERT_EQ(inputs.size(), 1); + + const config::UserInput input = *inputs.begin(); + ASSERT_TRUE(input.is_joystick()); + + EXPECT_EQ(input.joy_input().joy(), config::JoyId(9)); + EXPECT_EQ(input.joy_input().control(), config::JoyControl::Button); + EXPECT_EQ(input.joy_input().control_index(), 20); +} + +TEST(UserInputTest, FromMixedConfigString) { + const std::unordered_set inputs = + config::UserInput::FromConfigString("CTRL+SHIFT+F1,Joy1-Axis2+,A,SHIFT"); + ASSERT_EQ(inputs.size(), 4); + + EXPECT_TRUE(inputs.find(config::KeyboardInput( + wxKeyCode::WXK_F1, static_cast(wxMOD_SHIFT | wxMOD_CONTROL))) != + inputs.end()); + EXPECT_TRUE(inputs.find(config::JoyInput(config::JoyId(0), config::JoyControl::AxisPlus, 2)) != + inputs.end()); + EXPECT_TRUE(inputs.find(config::KeyboardInput('A')) != inputs.end()); + EXPECT_TRUE(inputs.find(config::KeyboardInput(wxKeyCode::WXK_SHIFT, wxMOD_SHIFT)) != + inputs.end()); +} + +TEST(UserInputTest, InvalidConfigString) { + EXPECT_TRUE(config::UserInput::FromConfigString("").empty()); + // While this should parse as "SHIFT+," and ",", the comma handling can't parse this. + EXPECT_TRUE(config::UserInput::FromConfigString("SHIFT+,,,").empty()); + EXPECT_TRUE(config::UserInput::FromConfigString("CTRL+SHIFT+F1,asdf").empty()); + EXPECT_TRUE(config::UserInput::FromConfigString("abc").empty()); + EXPECT_TRUE(config::UserInput::FromConfigString("Joy1-Axis2").empty()); + EXPECT_TRUE(config::UserInput::FromConfigString("Joy1-Button").empty()); + EXPECT_TRUE(config::UserInput::FromConfigString("Joy1-HatN").empty()); + EXPECT_TRUE(config::UserInput::FromConfigString("Joy1-Hat0").empty()); +} diff --git a/src/wx/config/user-input.cpp b/src/wx/config/user-input.cpp index 361d8194..a3401555 100644 --- a/src/wx/config/user-input.cpp +++ b/src/wx/config/user-input.cpp @@ -289,7 +289,7 @@ wxString JoyInput::ToLocalizedString() const { wxString KeyboardInput::ToConfigString() const { // Handle the modifier case separately. if (KeyIsModifier(key_)) { - return wxString::Format("%d:%d", key_, mod_); + return ModToConfigString(mod_).RemoveLast(); } // Custom overrides. @@ -298,6 +298,11 @@ wxString KeyboardInput::ToConfigString() const { return ModToConfigString(mod_) + iter->second.config_name; } + if (key_ == ',' || key_ == ':') { + // Special case for comma and colon to avoid parsing issues. + return wxString::Format("%d:%d", key_, mod_); + } + const wxString accel_string = wxAcceleratorEntry(mod_, key_).ToRawString().MakeUpper(); if (!accel_string.IsAscii()) { // Unicode handling.