From 7913e7cf82bb60089590df59b0d7922f232ceeb1 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Wed, 19 Feb 2020 01:06:39 +0100 Subject: [PATCH] Added unit tests. --- TODO | 2 +- src/laybasic/laybasic/gsiDeclLayMenu.cc | 11 ++- src/laybasic/laybasic/layAbstractMenu.cc | 7 +- src/laybasic/laybasic/layAbstractMenu.h | 3 +- ...bstractMenu.cc => layAbstractMenuTests.cc} | 54 ++++++++++++++ src/laybasic/unit_tests/unit_tests.pro | 4 +- testdata/ruby/layMenuTest.rb | 72 ++++++++++++++----- 7 files changed, 125 insertions(+), 28 deletions(-) rename src/laybasic/unit_tests/{layAbstractMenu.cc => layAbstractMenuTests.cc} (74%) diff --git a/TODO b/TODO index 03c30fbb8..89e60ff94 100644 --- a/TODO +++ b/TODO @@ -32,7 +32,7 @@ DONE 8. We can move menu initialization of LV-specifics to the LayoutView and 12. Default ctor for MainWindow so a pymod script can instantiate one (?) -13. Re-think concept of Action as "smart pointers": ConfigureAction +DONE 13. Re-think concept of Action as "smart pointers": ConfigureAction violates this. ConfigureAction should be ActionHandle variant? Action as pointer and smart pointers for managing? diff --git a/src/laybasic/laybasic/gsiDeclLayMenu.cc b/src/laybasic/laybasic/gsiDeclLayMenu.cc index 61a0c2db2..4a3680a99 100644 --- a/src/laybasic/laybasic/gsiDeclLayMenu.cc +++ b/src/laybasic/laybasic/gsiDeclLayMenu.cc @@ -86,7 +86,14 @@ static std::map unpack_menu_items_hidden (const std::string & return kb; } +static lay::AbstractMenu *new_menu () +{ + return new lay::AbstractMenu (0); +} + Class decl_AbstractMenu ("lay", "AbstractMenu", + // for test purposes mainly: + constructor ("new", &new_menu, "@hide") + method ("pack_key_binding", &pack_key_binding, gsi::arg ("path_to_keys"), "@brief Serializes a key binding definition into a single string\n" "The serialized format is used by the 'key-bindings' config key. " @@ -119,8 +126,8 @@ Class decl_AbstractMenu ("lay", "AbstractMenu", "@brief Get the reference to a Action object associated with the given path\n" "@args path\n" "\n" - "@param path The path to the item. This must be a valid path.\n" - "@return A reference to a Action object associated with this path\n" + "@param path The path to the item.\n" + "@return A reference to a Action object associated with this path or nil if the path is not valid\n" ) + method ("items", &lay::AbstractMenu::items, "@brief Get the subitems for a given submenu\n" diff --git a/src/laybasic/laybasic/layAbstractMenu.cc b/src/laybasic/laybasic/layAbstractMenu.cc index 9c4214448..ba9a44f01 100644 --- a/src/laybasic/laybasic/layAbstractMenu.cc +++ b/src/laybasic/laybasic/layAbstractMenu.cc @@ -240,7 +240,7 @@ AbstractMenuItem::set_action (Action *a, bool copy_properties) { tl_assert (a != 0); - a->keep_object (); + a->keep (); if (copy_properties && mp_action->qaction () && a->qaction ()) { a->qaction ()->setIcon (mp_action->qaction ()->icon ()); @@ -1192,10 +1192,7 @@ const Action *AbstractMenu::action(const std::string &path) const Action *AbstractMenu::action(const std::string &path) { AbstractMenuItem *item = find_item_exact (path); - if (! item) { - throw tl::Exception (tl::to_string (QObject::tr ("Not a valid menu item path: ")) + path); - } - return item->action (); + return item ? item->action () : 0; } std::vector diff --git a/src/laybasic/laybasic/layAbstractMenu.h b/src/laybasic/laybasic/layAbstractMenu.h index eda1c8454..eed9b5728 100644 --- a/src/laybasic/laybasic/layAbstractMenu.h +++ b/src/laybasic/laybasic/layAbstractMenu.h @@ -86,7 +86,8 @@ LAYBASIC_PUBLIC std::string pack_menu_items_hidden (const std::vector action (new lay::Action ("title:n1")); + + { + lay::AbstractMenu menu (0); + EXPECT_EQ (menu_to_string (menu), ""); + EXPECT_EQ (menu.action ("s1.n1") == 0, true); + EXPECT_EQ (menu.action ("s1") == 0, true); + + menu.insert_menu ("end", "s1", "submenu1"); + menu.insert_menu ("end", "s2", "submenu2"); + + menu.insert_item ("s1.end", "n1", action.get ()); + menu.insert_item ("s2.end", "n1", action.get ()); + EXPECT_EQ (menu_to_string (menu), "(s1(s1.n1),s2(s2.n1))"); + + EXPECT_EQ (menu.action ("s1.n1") == action.get (), true); + EXPECT_EQ (menu.action ("s2.n1") == action.get (), true); + } + + // the action is deleted because it's owned by the menu + EXPECT_EQ (action.get () == 0, true); +} + +TEST(3_ActionReferences) +{ + tl::weak_ptr action (new lay::Action ("title:n1")); + + { + lay::AbstractMenu menu (0); + EXPECT_EQ (menu_to_string (menu), ""); + EXPECT_EQ (menu.action ("s1.n1") == 0, true); + EXPECT_EQ (menu.action ("s1") == 0, true); + + menu.insert_menu ("end", "s1", "submenu1"); + menu.insert_menu ("end", "s2", "submenu2"); + + menu.insert_item ("s1.end", "n1", action.get ()); + menu.insert_item ("s2.end", "n1", action.get ()); + EXPECT_EQ (menu_to_string (menu), "(s1(s1.n1),s2(s2.n1))"); + + menu.delete_item ("s2"); + + EXPECT_EQ (menu.action ("s1.n1") != 0, true); + EXPECT_EQ (menu.action ("s1.n1") == action.get (), true); + EXPECT_EQ (menu.action ("s2.n1") == 0, true); + } + + // the action is deleted because it's owned by the menu + EXPECT_EQ (action.get () == 0, true); +} + diff --git a/src/laybasic/unit_tests/unit_tests.pro b/src/laybasic/unit_tests/unit_tests.pro index a7392b824..0aeab4b3d 100644 --- a/src/laybasic/unit_tests/unit_tests.pro +++ b/src/laybasic/unit_tests/unit_tests.pro @@ -14,9 +14,9 @@ SOURCES = \ layParsedLayerSource.cc \ layRenderer.cc \ laySnap.cc \ - layAbstractMenu.cc \ layNetlistBrowserModelTests.cc \ - layNetlistBrowserTreeModelTests.cc + layNetlistBrowserTreeModelTests.cc \ + layAbstractMenuTests.cc INCLUDEPATH += $$TL_INC $$LAYBASIC_INC $$DB_INC $$GSI_INC $$OUT_PWD/../laybasic DEPENDPATH += $$TL_INC $$LAYBASIC_INC $$DB_INC $$GSI_INC $$OUT_PWD/../laybasic diff --git a/testdata/ruby/layMenuTest.rb b/testdata/ruby/layMenuTest.rb index 9007f5d56..90452929b 100644 --- a/testdata/ruby/layMenuTest.rb +++ b/testdata/ruby/layMenuTest.rb @@ -38,26 +38,20 @@ class LAYMenuTest_TestClass < TestBase assert_equal(a.shortcut, "Shift+F1") assert_equal(a.effective_shortcut, "Shift+F1") - assert_equal(b.shortcut, "Shift+F1") - assert_equal(b.effective_shortcut, "Shift+F1") + assert_equal(b.shortcut, "") + assert_equal(b.effective_shortcut, "") a.default_shortcut = "X" assert_equal(a.default_shortcut, "X") assert_equal(a.shortcut, "Shift+F1") assert_equal(a.effective_shortcut, "Shift+F1") - assert_equal(b.default_shortcut, "X") - assert_equal(b.shortcut, "Shift+F1") - assert_equal(b.effective_shortcut, "Shift+F1") a.shortcut = "" assert_equal(a.default_shortcut, "X") assert_equal(a.shortcut, "") assert_equal(a.effective_shortcut, "X") - assert_equal(b.default_shortcut, "X") - assert_equal(b.shortcut, "") - assert_equal(b.effective_shortcut, "X") a.shortcut = RBA::Action::NoKeyBound assert_equal(a.default_shortcut, "X") @@ -74,9 +68,6 @@ class LAYMenuTest_TestClass < TestBase assert_equal(a.is_hidden?, false) assert_equal(a.is_effective_visible?, false) assert_equal(a.effective_shortcut, "X") - assert_equal(b.is_visible?, false) - assert_equal(b.is_hidden?, false) - assert_equal(b.is_effective_visible?, false) a.hidden = false a.visible = true @@ -85,9 +76,6 @@ class LAYMenuTest_TestClass < TestBase assert_equal(a.is_hidden?, false) assert_equal(a.is_effective_visible?, true) assert_equal(a.effective_shortcut, "X") - assert_equal(b.is_visible?, true) - assert_equal(b.is_hidden?, false) - assert_equal(b.is_effective_visible?, true) a.hidden = true a.visible = true @@ -96,9 +84,6 @@ class LAYMenuTest_TestClass < TestBase assert_equal(a.is_hidden?, true) assert_equal(a.is_effective_visible?, false) assert_equal(a.effective_shortcut, "") - assert_equal(b.is_visible?, true) - assert_equal(b.is_hidden?, true) - assert_equal(b.is_effective_visible?, false) end @@ -345,6 +330,59 @@ RESULT end + def test_4 + + action = RBA::Action::new + action.title = "title:n1" + + menu = RBA::AbstractMenu::new + + assert_equal(menu.action("s1.n1"), nil) + assert_equal(menu.action("s1"), nil) + + menu.insert_menu("end", "s1", "submenu1") + menu.insert_menu("end", "s2", "submenu2") + + menu.insert_item("s1.end", "n1", action) + menu.insert_item("s2.end", "n1", action) + + assert_equal(menu.action("s1.n1") == action, true) + assert_equal(menu.action("s2.n1") == action, true) + + menu._destroy + + # proof of transfer of ownership + assert_equal(action._destroyed?, true) + + end + + def test_5 + + action = RBA::Action::new + action.title = "title:n1" + + menu = RBA::AbstractMenu::new + + assert_equal(menu.action("s1.n1"), nil) + assert_equal(menu.action("s1"), nil) + + menu.insert_menu("end", "s1", "submenu1") + menu.insert_menu("end", "s2", "submenu2") + + menu.insert_item("s1.end", "n1", action) + menu.insert_item("s2.end", "n1", action) + + menu.delete_item ("s2"); + + assert_equal(menu.action("s1.n1") == action, true) + assert_equal(menu.action("s2.n1") == nil, true) + + menu._destroy + + assert_equal(action._destroyed?, true) + + end + end load("test_epilogue.rb")