Merge pull request #1011 from p12tic/hotkey-tests

Add tests for Hotkey serialization

src/gui/CMakeLists.txt

@@ -8,16 +8,19 @@ set (CMAKE_INCLUDE_CURRENT_DIR ON)
 
 # files that are used both in tests and the app
 set(GUI_COMMON_SOURCE_FILES
+    src/Action.cpp
+    src/Hotkey.cpp
     src/KeySequence.cpp
 )
 
 set(GUI_COMMON_HEADER_FILES
+    src/Action.h
+    src/Hotkey.h
     src/KeySequence.h
 )
 
 set(GUI_SOURCE_FILES
     src/AboutDialog.cpp
-    src/Action.cpp
     src/ActionDialog.cpp
     src/AddClientDialog.cpp
     src/AppConfig.cpp
@@ -27,7 +30,6 @@ set(GUI_SOURCE_FILES
     src/DataDownloader.cpp
     src/DisplayIsValid.cpp
     src/Fingerprint.cpp
-    src/Hotkey.cpp
     src/HotkeyDialog.cpp
     src/IpcClient.cpp
     src/Ipc.cpp
@@ -60,7 +62,6 @@ set(GUI_SOURCE_FILES
 set(GUI_HEADER_FILES
     src/AboutDialog.h
     src/ActionDialog.h
-    src/Action.h
     src/AddClientDialog.h
     src/AppConfig.h
     src/BarrierLocale.h
@@ -71,7 +72,6 @@ set(GUI_HEADER_FILES
     src/ElevateMode.h
     src/Fingerprint.h
     src/HotkeyDialog.h
-    src/Hotkey.h
     src/IpcClient.h
     src/Ipc.h
     src/IpcReader.h
@@ -164,6 +164,7 @@ endif()
 if (BARRIER_BUILD_TESTS)
     set(GUI_TEST_SOURCE_FILES
         test/KeySequenceTests.cpp
+        test/HotkeyTests.cpp
         test/main.cpp
     )
 
@@ -176,5 +177,5 @@ if (BARRIER_BUILD_TESTS)
     add_test(guiunittests guiunittests)
 
     target_include_directories(guiunittests PUBLIC ../../ext)
-    target_link_libraries(guiunittests gtest gmock Qt5::Core Qt5::Widgets Qt5::Network pthread)
+    target_link_libraries(guiunittests gtest gmock Qt5::Core Qt5::Widgets Qt5::Network ${libs})
 endif()

src/gui/src/Action.cpp

@@ -51,7 +51,7 @@ QString Action::text() const
      * in the end but now argument inside. If you need a function with no
      * argument, it can not have () in the end.
      */
-    QString text = QString(m_ActionTypeNames[keySequence().isMouseButton() ?
+    QString text = QString(m_ActionTypeNames[m_KeySequence.isMouseButton() ?
                                              type() + int(mouseDown) : type()]);
 
     switch (type())
@@ -61,9 +61,9 @@ QString Action::text() const
         case keystroke:
             {
                 text += "(";
-                text += keySequence().toString();
+                text += m_KeySequence.toString();
 
-                if (!keySequence().isMouseButton())
+                if (!m_KeySequence.isMouseButton())
                 {
                     const QStringList& screens = typeScreenNames();
                     if (haveScreens() && !screens.isEmpty())
@@ -116,15 +116,15 @@ QString Action::text() const
 
 void Action::loadSettings(QSettings& settings)
 {
-    keySequence().loadSettings(settings);
+    m_KeySequence.loadSettings(settings);
     setType(settings.value("type", keyDown).toInt());
 
-    typeScreenNames().clear();
+    m_TypeScreenNames.clear();
     int numTypeScreens = settings.beginReadArray("typeScreenNames");
     for (int i = 0; i < numTypeScreens; i++)
     {
         settings.setArrayIndex(i);
-        typeScreenNames().append(settings.value("typeScreenName").toString());
+        m_TypeScreenNames.append(settings.value("typeScreenName").toString());
     }
     settings.endArray();
 
@@ -137,7 +137,7 @@ void Action::loadSettings(QSettings& settings)
 
 void Action::saveSettings(QSettings& settings) const
 {
-    keySequence().saveSettings(settings);
+    m_KeySequence.saveSettings(settings);
     settings.setValue("type", type());
 
     settings.beginWriteArray("typeScreenNames");

src/gui/src/Action.h

@@ -32,9 +32,6 @@ class QTextStream;
 
 class Action
 {
-    friend class ActionDialog;
-    friend QTextStream& operator<<(QTextStream& outStream, const Action& action);
-
     public:
         enum ActionType { keyDown, keyUp, keystroke,
                           switchToScreen, toggleScreen, switchInDirection,
@@ -48,25 +45,31 @@ class Action
     public:
         QString text() const;
         const KeySequence& keySequence() const { return m_KeySequence; }
+        void setKeySequence(const KeySequence& seq) { m_KeySequence = seq; }
+
         void loadSettings(QSettings& settings);
         void saveSettings(QSettings& settings) const;
-        int type() const { return m_Type; }
-        const QStringList& typeScreenNames() const { return m_TypeScreenNames; }
-        const QString& switchScreenName() const { return m_SwitchScreenName; }
-        int switchDirection() const { return m_SwitchDirection; }
-        int lockCursorMode() const { return m_LockCursorMode; }
-        bool activeOnRelease() const { return m_ActiveOnRelease; }
-        bool haveScreens() const { return m_HasScreens; }
 
-    protected:
+        int type() const { return m_Type; }
-        KeySequence& keySequence() { return m_KeySequence; }
-        void setKeySequence(const KeySequence& seq) { m_KeySequence = seq; }
         void setType(int t) { m_Type = t; }
-        QStringList& typeScreenNames() { return m_TypeScreenNames; }
+
+        const QStringList& typeScreenNames() const { return m_TypeScreenNames; }
+        void appendTypeScreenName(QString name) { m_TypeScreenNames.append(name); }
+        void clearTypeScreenNames() { m_TypeScreenNames.clear(); }
+
+        const QString& switchScreenName() const { return m_SwitchScreenName; }
         void setSwitchScreenName(const QString& n) { m_SwitchScreenName = n; }
+
+        int switchDirection() const { return m_SwitchDirection; }
         void setSwitchDirection(int d) { m_SwitchDirection = d; }
+
+        int lockCursorMode() const { return m_LockCursorMode; }
         void setLockCursorMode(int m) { m_LockCursorMode = m; }
+
+        bool activeOnRelease() const { return m_ActiveOnRelease; }
         void setActiveOnRelease(bool b) { m_ActiveOnRelease = b; }
+
+        bool haveScreens() const { return m_HasScreens; }
         void setHaveScreens(bool b) { m_HasScreens = b; }
 
     private:

src/gui/src/ActionDialog.cpp

@@ -83,9 +83,9 @@ void ActionDialog::accept()
     m_Action.setType(m_pButtonGroupType->checkedId());
     m_Action.setHaveScreens(m_pGroupBoxScreens->isChecked());
 
-    m_Action.typeScreenNames().clear();
+    m_Action.clearTypeScreenNames();
     foreach(const QListWidgetItem* pItem, m_pListScreens->selectedItems())
-        m_Action.typeScreenNames().append(pItem->text());
+        m_Action.appendTypeScreenName(pItem->text());
 
     m_Action.setSwitchScreenName(m_pComboSwitchToScreen->currentText());
     m_Action.setSwitchDirection(m_pComboSwitchInDirection->currentIndex());

src/gui/src/Hotkey.cpp

@@ -28,9 +28,9 @@ Hotkey::Hotkey() :
 
 QString Hotkey::text() const
 {
-    QString text = keySequence().toString();
+    QString text = m_KeySequence.toString();
 
-    if (keySequence().isMouseButton())
+    if (m_KeySequence.isMouseButton())
         return "mousebutton(" + text + ")";
 
     return "keystroke(" + text + ")";
@@ -38,16 +38,16 @@ QString Hotkey::text() const
 
 void Hotkey::loadSettings(QSettings& settings)
 {
-    keySequence().loadSettings(settings);
+    m_KeySequence.loadSettings(settings);
 
-    actions().clear();
+    m_Actions.clear();
     int num = settings.beginReadArray("actions");
     for (int i = 0; i < num; i++)
     {
         settings.setArrayIndex(i);
         Action a;
         a.loadSettings(settings);
-        actions().append(a);
+        m_Actions.append(a);
     }
 
     settings.endArray();
@@ -55,13 +55,13 @@ void Hotkey::loadSettings(QSettings& settings)
 
 void Hotkey::saveSettings(QSettings& settings) const
 {
-    keySequence().saveSettings(settings);
+    m_KeySequence.saveSettings(settings);
 
     settings.beginWriteArray("actions");
-    for (int i = 0; i < actions().size(); i++)
+    for (int i = 0; i < m_Actions.size(); i++)
     {
         settings.setArrayIndex(i);
-        actions()[i].saveSettings(settings);
+        m_Actions[i].saveSettings(settings);
     }
     settings.endArray();
 }

src/gui/src/Hotkey.h

@@ -33,26 +33,21 @@ class QSettings;
 
 class Hotkey
 {
-    friend class HotkeyDialog;
-    friend class ServerConfigDialog;
-    friend QTextStream& operator<<(QTextStream& outStream, const Hotkey& hotkey);
-
     public:
         Hotkey();
 
-    public:
         QString text() const;
         const KeySequence& keySequence() const { return m_KeySequence; }
+        void setKeySequence(const KeySequence& seq) { m_KeySequence = seq; }
+
         const ActionList& actions() const { return m_Actions; }
+        void appendAction(const Action& action) { m_Actions.append(action); }
+        void setAction(int index, const Action& action) { m_Actions[index] = action; }
+        void removeAction(int index) { m_Actions.removeAt(index); }
 
         void loadSettings(QSettings& settings);
         void saveSettings(QSettings& settings) const;
 
-    protected:
-        KeySequence& keySequence() { return m_KeySequence; }
-        void setKeySequence(const KeySequence& seq) { m_KeySequence = seq; }
-        ActionList& actions() { return m_Actions; }
-
 
     private:
         KeySequence m_KeySequence;

src/gui/src/ServerConfigDialog.cpp

@@ -183,7 +183,7 @@ void ServerConfigDialog::on_m_pButtonNewAction_clicked()
     ActionDialog dlg(this, serverConfig(), hotkey, action);
     if (dlg.exec() == QDialog::Accepted)
     {
-        hotkey.actions().append(action);
+        hotkey.appendAction(action);
         m_pListActions->addItem(action.text());
     }
 }
@@ -196,12 +196,14 @@ void ServerConfigDialog::on_m_pButtonEditAction_clicked()
 
     int idxAction = m_pListActions->currentRow();
     Q_ASSERT(idxAction >= 0 && idxAction < hotkey.actions().size());
-    Action& action = hotkey.actions()[idxAction];
+    Action action = hotkey.actions()[idxAction];
 
     ActionDialog dlg(this, serverConfig(), hotkey, action);
-    if (dlg.exec() == QDialog::Accepted)
+    if (dlg.exec() == QDialog::Accepted) {
+        hotkey.setAction(idxAction, action);
         m_pListActions->currentItem()->setText(action.text());
     }
+}
 
 void ServerConfigDialog::on_m_pButtonRemoveAction_clicked()
 {
@@ -212,7 +214,7 @@ void ServerConfigDialog::on_m_pButtonRemoveAction_clicked()
     int idxAction = m_pListActions->currentRow();
     Q_ASSERT(idxAction >= 0 && idxAction < hotkey.actions().size());
 
-    hotkey.actions().removeAt(idxAction);
+    hotkey.removeAction(idxAction);
     delete m_pListActions->currentItem();
 }
 

src/gui/test/HotkeyTests.cpp

@@ -0,0 +1,244 @@
+/*  barrier -- mouse and keyboard sharing utility
+    Copyright (C) 2021 Povilas Kanapickas <povilas@radix.lt>
+
+    This package is free software; you can redistribute it and/or
+    modify it under the terms of the GNU General Public License
+    found in the file LICENSE that should have accompanied this file.
+
+    This package is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+
+#include "../src/Hotkey.h"
+#include <gtest/gtest.h>
+#include "Utils.h"
+
+#include <QtCore/QSettings>
+
+struct TestAction
+{
+    Action::ActionType type = Action::keyDown;
+    std::vector<TestKey> keys;
+    std::vector<std::string> type_screen_names;
+    std::string screen_name;
+    Action::SwitchDirection switch_direction;
+    Action::LockCursorMode lock_cursor_mode;
+
+    static TestAction createKeyAction(Action::ActionType type, const std::vector<TestKey>& keys,
+                                      const std::vector<std::string>& type_screen_names = {})
+    {
+        TestAction action;
+        action.type = Action::keyDown;
+        action.keys = keys;
+        action.type_screen_names = type_screen_names;
+        return action;
+    }
+
+    static TestAction createKeyDown(const std::vector<TestKey>& keys,
+                                    const std::vector<std::string>& type_screen_names = {})
+    {
+        return createKeyAction(Action::keyDown, keys, type_screen_names);
+    }
+
+    static TestAction createKeyUp(const std::vector<TestKey>& keys,
+                                  const std::vector<std::string>& type_screen_names = {})
+    {
+        return createKeyAction(Action::keyUp, keys, type_screen_names);
+    }
+
+    static TestAction createKeyStroke(const std::vector<TestKey>& keys,
+                                      const std::vector<std::string>& type_screen_names = {})
+    {
+        return createKeyAction(Action::keystroke, keys, type_screen_names);
+    }
+
+    static TestAction createSwitchToScreen(const std::string& screen_name)
+    {
+        TestAction action;
+        action.type = Action::switchToScreen;
+        action.screen_name = screen_name;
+        return action;
+    }
+
+    static TestAction createToggleScreen()
+    {
+        TestAction action;
+        action.type = Action::toggleScreen;
+        return action;
+    }
+
+    static TestAction createSwitchInDirection(Action::SwitchDirection switch_direction)
+    {
+        TestAction action;
+        action.type = Action::switchInDirection;
+        action.switch_direction = switch_direction;
+        return action;
+    }
+
+    static TestAction createLockCursorToScreen(Action::LockCursorMode lock_cursor_mode)
+    {
+        TestAction action;
+        action.type = Action::lockCursorToScreen;
+        action.lock_cursor_mode = lock_cursor_mode;
+        return action;
+    }
+};
+
+struct TestHotKey
+{
+    std::vector<TestKey> keys;
+    std::vector<TestAction> actions;
+};
+
+Action createAction(const TestAction& test_action)
+{
+    Action action;
+    action.setType(test_action.type);
+
+    switch (test_action.type) {
+        case Action::keyDown:
+        case Action::keyUp:
+        case Action::keystroke: {
+            KeySequence sequence;
+            for (auto key : test_action.keys) {
+                sequence.appendKey(key.key, key.modifier);
+            }
+            action.setKeySequence(sequence);
+            for (const auto& type_screen_name : test_action.type_screen_names) {
+                action.appendTypeScreenName(QString::fromStdString(type_screen_name));
+            }
+            break;
+        }
+        case Action::switchToScreen:
+            action.setSwitchScreenName(QString::fromStdString(test_action.screen_name));
+            break;
+        case Action::toggleScreen:
+            break;
+        case Action::switchInDirection:
+            action.setSwitchDirection(test_action.switch_direction);
+            break;
+        case Action::lockCursorToScreen:
+            action.setLockCursorMode(test_action.lock_cursor_mode);
+            break;
+    }
+    return action;
+}
+
+void doHotkeyLoadSaveTest(const TestHotKey& test_hotkey, QSettings::Format format)
+{
+    auto filename = getTemporaryFilename();
+
+    Hotkey hotkey_before, hotkey_after;
+    {
+        QSettings settings(filename, format);
+
+
+        KeySequence sequence;
+        for (auto key : test_hotkey.keys) {
+            sequence.appendKey(key.key, key.modifier);
+        }
+        hotkey_before.setKeySequence(sequence);
+
+        for (auto action : test_hotkey.actions) {
+            hotkey_before.appendAction(createAction(action));
+        }
+
+        settings.beginGroup("test");
+        hotkey_before.saveSettings(settings);
+        settings.endGroup();
+    }
+    {
+        QSettings settings(filename, format);
+
+        settings.beginGroup("test");
+        hotkey_after.loadSettings(settings);
+        settings.endGroup();
+
+        ASSERT_EQ(hotkey_before.keySequence().sequence(), hotkey_after.keySequence().sequence());
+        ASSERT_EQ(hotkey_before.keySequence().modifiers(), hotkey_after.keySequence().modifiers());
+
+        const auto& actions_before = hotkey_before.actions();
+        const auto& actions_after = hotkey_after.actions();
+
+        ASSERT_EQ(actions_before.size(), actions_after.size());
+        for (int i = 0; i < actions_before.size(); ++i) {
+            const auto& action_before = actions_before[i];
+            const auto& action_after = actions_after[i];
+
+            ASSERT_EQ(action_before.keySequence().sequence(), action_after.keySequence().sequence());
+            ASSERT_EQ(action_before.keySequence().modifiers(), action_after.keySequence().modifiers());
+            ASSERT_EQ(action_before.type(), action_after.type());
+            ASSERT_EQ(action_before.typeScreenNames(), action_after.typeScreenNames());
+            ASSERT_EQ(action_before.switchScreenName(), action_after.switchScreenName());
+            ASSERT_EQ(action_before.switchDirection(), action_after.switchDirection());
+            ASSERT_EQ(action_before.lockCursorMode(), action_after.lockCursorMode());
+            ASSERT_EQ(action_before.activeOnRelease(), action_after.activeOnRelease());
+            ASSERT_EQ(action_before.haveScreens(), action_after.haveScreens());
+        }
+    }
+
+    QFile::remove(filename);
+}
+
+TEST(HotkeyLoadSaveTests, Empty)
+{
+    TestHotKey hotkey;
+    doHotkeyLoadSaveTest(hotkey, QSettings::NativeFormat);
+    doHotkeyLoadSaveTest(hotkey, QSettings::IniFormat);
+}
+
+TEST(HotkeyLoadSaveTests, KeysNoActions)
+{
+    TestHotKey hotkey = {{{Qt::Key_A, Qt::NoModifier}, {Qt::Key_B, Qt::NoModifier}}, {}};
+    doHotkeyLoadSaveTest(hotkey, QSettings::NativeFormat);
+    doHotkeyLoadSaveTest(hotkey, QSettings::IniFormat);
+}
+
+TEST(HotkeyLoadSaveTests, CommaKeyNoActions)
+{
+    TestHotKey hotkey = {
+        {
+            {Qt::Key_A, Qt::NoModifier},
+            {Qt::Key_Comma, Qt::NoModifier},
+            {Qt::Key_B, Qt::NoModifier}
+        }, {}};
+    doHotkeyLoadSaveTest(hotkey, QSettings::NativeFormat);
+    doHotkeyLoadSaveTest(hotkey, QSettings::IniFormat);
+}
+
+TEST(HotkeyLoadSaveTests, KeysSingleAction)
+{
+    TestHotKey hotkey = {
+        {
+            {Qt::Key_A, Qt::NoModifier},
+            {Qt::Key_B, Qt::NoModifier}
+        },
+        {
+            TestAction::createKeyDown({{Qt::Key_Z, Qt::NoModifier}})
+        }
+    };
+    doHotkeyLoadSaveTest(hotkey, QSettings::NativeFormat);
+    doHotkeyLoadSaveTest(hotkey, QSettings::IniFormat);
+}
+
+TEST(HotkeyLoadSaveTests, KeysMultipleAction)
+{
+    TestHotKey hotkey = {
+        {
+            {Qt::Key_A, Qt::NoModifier},
+            {Qt::Key_B, Qt::NoModifier}
+        },
+        {
+            TestAction::createKeyDown({{Qt::Key_Z, Qt::NoModifier}}),
+            TestAction::createSwitchToScreen("test_screen")
+        }
+    };
+    doHotkeyLoadSaveTest(hotkey, QSettings::NativeFormat);
+    doHotkeyLoadSaveTest(hotkey, QSettings::IniFormat);
+}

src/gui/test/KeySequenceTests.cpp

@@ -15,6 +15,7 @@
 */
 
 #include "../src/KeySequence.h"
+#include "Utils.h"
 #include <gtest/gtest.h>
 #include <cstdio>
 
@@ -74,13 +75,15 @@ namespace {
         Qt::Key_Select,
     };
 
-    QString getTemporaryFilename()
+    std::string keySequenceToString(const std::vector<TestKey>& key_pairs)
     {
-        QTemporaryFile temp_file;
+        KeySequence sequence;
-        temp_file.open();
+        for (auto key_pair : key_pairs) {
-        return temp_file.fileName();
+            sequence.appendKey(key_pair.key, key_pair.modifier);
         }
+        return sequence.toString().toStdString();
     }
+} // namespace
 
 class KeySequenceLoadSaveTestFixture :
         public ::testing::TestWithParam<std::tr1::tuple<Qt::Key, QSettings::Format>> {};
@@ -124,3 +127,21 @@ INSTANTIATE_TEST_CASE_P(
         KeySequenceLoadSaveTestFixture,
         ::testing::Combine(::testing::ValuesIn(s_key_sequence_test_keys),
                            ::testing::Values(QSettings::NativeFormat, QSettings::IniFormat)));
+
+TEST(KeySequenceTests, ToString)
+{
+    ASSERT_EQ(keySequenceToString({{Qt::Key_Menu, Qt::MetaModifier}}),
+              "Meta");
+    ASSERT_EQ(keySequenceToString({{Qt::Key_A, 0}, {Qt::Key_B, 0}}),
+              "a+b");
+    ASSERT_EQ(keySequenceToString({{Qt::Key_A, 0}, {Qt::Key_Comma, 0}, {Qt::Key_B, 0}}),
+              "a+,+b");
+    ASSERT_EQ(keySequenceToString({{Qt::Key_A, 0}, {Qt::Key_Semicolon, 0}, {Qt::Key_B, 0}}),
+              "a+;+b");
+    ASSERT_EQ(keySequenceToString({{Qt::Key_A, 0}, {Qt::Key_Shift, Qt::ShiftModifier},
+                                   {Qt::Key_0, Qt::ShiftModifier}}),
+              "a+Shift+0");
+    ASSERT_EQ(keySequenceToString({{Qt::Key_A, 0}, {Qt::Key_Control, Qt::ControlModifier},
+                                   {Qt::Key_0, Qt::ControlModifier}}),
+              "a+Control+0");
+}

src/gui/test/Utils.h

@@ -0,0 +1,38 @@
+/*  barrier -- mouse and keyboard sharing utility
+    Copyright (C) 2021 Povilas Kanapickas <povilas@radix.lt>
+
+    This package is free software; you can redistribute it and/or
+    modify it under the terms of the GNU General Public License
+    found in the file LICENSE that should have accompanied this file.
+
+    This package is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#ifndef BARRIER_GUI_TEST_UTILS_H
+#define BARRIER_GUI_TEST_UTILS_H
+
+#include <QtCore/QFile>
+#include <QtCore/QTemporaryFile>
+
+struct TestKey
+{
+    int key = 0;
+    int modifier = Qt::NoModifier;
+
+    TestKey(int key, int modifier) : key{key}, modifier{modifier} {}
+};
+
+inline QString getTemporaryFilename()
+{
+    QTemporaryFile temp_file;
+    temp_file.open();
+    return temp_file.fileName();
+}
+
+#endif // BARRIER_GUI_TEST_UTILS_H