From f0fd6294a8e01db6e2208fbd2aa4186eae65fca0 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Mon, 16 Oct 2023 19:18:12 +0100 Subject: [PATCH 001/139] [tests] Add file to test base snapshots Add a file to test BaseSnapshot, with basic structure and a placeholder test. --- tests/CMakeLists.txt | 1 + tests/test_base_snapshot.cpp | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 tests/test_base_snapshot.cpp diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 0bc8f6050f..96f733e42b 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -54,6 +54,7 @@ add_executable(multipass_tests temp_file.cpp test_alias_dict.cpp test_argparser.cpp + test_base_snapshot.cpp test_base_virtual_machine.cpp test_base_virtual_machine_factory.cpp test_basic_process.cpp diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp new file mode 100644 index 0000000000..5096d45f79 --- /dev/null +++ b/tests/test_base_snapshot.cpp @@ -0,0 +1,35 @@ +/* + * Copyright (C) Canonical, Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 3. + * + * This program 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 . + * + */ + +#include "common.h" + +#include "shared/base_snapshot.h" + +namespace mp = multipass; +namespace mpt = multipass::test; +using namespace testing; + +namespace +{ +class TestBaseSnapshot : public Test +{ +}; + +TEST_F(TestBaseSnapshot, test_placeholder) +{ +} +} // namespace From a25bc2a8c5983366e692ccd81fcf26ea92530d28 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 15 Nov 2023 16:49:03 +0000 Subject: [PATCH 002/139] [tests] Test BaseSnapshot adopts given name --- tests/test_base_snapshot.cpp | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index 5096d45f79..75f19c4b50 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -16,20 +16,50 @@ */ #include "common.h" +#include "mock_virtual_machine.h" +#include "multipass/vm_specs.h" #include "shared/base_snapshot.h" +#include + namespace mp = multipass; namespace mpt = multipass::test; using namespace testing; namespace { -class TestBaseSnapshot : public Test +class MockBaseSnapshot : public mp::BaseSnapshot { +public: + using mp::BaseSnapshot::BaseSnapshot; + + MOCK_METHOD(void, capture_impl, (), (override)); + MOCK_METHOD(void, erase_impl, (), (override)); + MOCK_METHOD(void, apply_impl, (), (override)); +}; + +struct TestBaseSnapshot : public Test +{ + static mp::VMSpecs stub_specs() + { + mp::VMSpecs ret{}; + ret.num_cores = 3; + ret.mem_size = mp::MemorySize{"1.5G"}; + ret.disk_space = mp::MemorySize{"10G"}; + ret.default_mac_address = "12:12:12:12:12:12"; + + return ret; + } + + mp::VMSpecs specs = stub_specs(); + mpt::MockVirtualMachine vm{"a-vm"}; }; -TEST_F(TestBaseSnapshot, test_placeholder) +TEST_F(TestBaseSnapshot, adopts_given_valid_name) { + auto name = "a-name"; + auto snapshot = MockBaseSnapshot{name, "", nullptr, specs, vm}; + EXPECT_EQ(snapshot.get_name(), name); } } // namespace From 4d49b4f35fa475694529ddb9afd64252498f8416 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Mon, 16 Oct 2023 23:31:05 +0100 Subject: [PATCH 003/139] [tests] Test a couple other snapshot properties --- tests/test_base_snapshot.cpp | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index 75f19c4b50..a0ca25ccff 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -62,4 +62,24 @@ TEST_F(TestBaseSnapshot, adopts_given_valid_name) auto snapshot = MockBaseSnapshot{name, "", nullptr, specs, vm}; EXPECT_EQ(snapshot.get_name(), name); } + +TEST_F(TestBaseSnapshot, adopts_given_comment) +{ + auto comment = "some comment"; + auto snapshot = MockBaseSnapshot{"whatever", comment, nullptr, specs, vm}; + EXPECT_EQ(snapshot.get_comment(), comment); +} + +TEST_F(TestBaseSnapshot, adopts_given_parent) +{ + auto parent = std::make_shared("root", "asdf", nullptr, specs, vm); + auto snapshot = MockBaseSnapshot{"descendant", "descends", parent, specs, vm}; + EXPECT_EQ(snapshot.get_parent(), parent); +} + +TEST_F(TestBaseSnapshot, adopts_null_parent) +{ + auto snapshot = MockBaseSnapshot{"descendant", "descends", nullptr, specs, vm}; + EXPECT_EQ(snapshot.get_parent(), nullptr); +} } // namespace From 180ee91bbd400eda2d816367ff7b09b44ba5b374 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Mon, 16 Oct 2023 23:06:39 +0100 Subject: [PATCH 004/139] [tests] Test that BaseSnapshot rejects empty name --- tests/test_base_snapshot.cpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index a0ca25ccff..691a30176b 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -23,6 +23,8 @@ #include +#include + namespace mp = multipass; namespace mpt = multipass::test; using namespace testing; @@ -53,7 +55,7 @@ struct TestBaseSnapshot : public Test } mp::VMSpecs specs = stub_specs(); - mpt::MockVirtualMachine vm{"a-vm"}; + mpt::MockVirtualMachine vm{"a-vm"}; // TODO@no-merge nice? }; TEST_F(TestBaseSnapshot, adopts_given_valid_name) @@ -63,6 +65,14 @@ TEST_F(TestBaseSnapshot, adopts_given_valid_name) EXPECT_EQ(snapshot.get_name(), name); } +TEST_F(TestBaseSnapshot, rejects_empty_name) +{ + std::string empty{}; + MP_EXPECT_THROW_THAT((MockBaseSnapshot{empty, "asdf", nullptr, specs, vm}), + std::runtime_error, + mpt::match_what(HasSubstr("empty"))); +} + TEST_F(TestBaseSnapshot, adopts_given_comment) { auto comment = "some comment"; From 81385ee5f37349de94dea83f135103d6dbfd7ebd Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Mon, 16 Oct 2023 23:30:09 +0100 Subject: [PATCH 005/139] [tests] Check that snapshots adopt given specs --- tests/test_base_snapshot.cpp | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index 691a30176b..47196f5a43 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -92,4 +92,39 @@ TEST_F(TestBaseSnapshot, adopts_null_parent) auto snapshot = MockBaseSnapshot{"descendant", "descends", nullptr, specs, vm}; EXPECT_EQ(snapshot.get_parent(), nullptr); } + +TEST_F(TestBaseSnapshot, adopts_given_specs) +{ + auto snapshot = MockBaseSnapshot{"snapshot", "", nullptr, specs, vm}; + EXPECT_EQ(snapshot.get_num_cores(), specs.num_cores); + EXPECT_EQ(snapshot.get_mem_size(), specs.mem_size); + EXPECT_EQ(snapshot.get_disk_space(), specs.disk_space); + EXPECT_EQ(snapshot.get_state(), specs.state); + EXPECT_EQ(snapshot.get_mounts(), specs.mounts); + EXPECT_EQ(snapshot.get_metadata(), specs.metadata); +} + +TEST_F(TestBaseSnapshot, adopts_custom_mounts) +{ + specs.mounts["toto"] = + mp::VMMount{"src", {{123, 234}, {567, 678}}, {{19, 91}}, multipass::VMMount::MountType::Classic}; + specs.mounts["tata"] = + mp::VMMount{"fountain", {{234, 123}}, {{81, 18}, {9, 10}}, multipass::VMMount::MountType::Native}; + + auto snapshot = MockBaseSnapshot{"snapshot", "", nullptr, specs, vm}; + EXPECT_EQ(snapshot.get_mounts(), specs.mounts); +} + +TEST_F(TestBaseSnapshot, adopts_custom_metadata) +{ + QJsonObject json; + QJsonObject data; + data.insert("an-int", 7); + data.insert("a-str", "str"); + json.insert("meta", data); + specs.metadata = json; + + auto snapshot = MockBaseSnapshot{"snapshot", "", nullptr, specs, vm}; + EXPECT_EQ(snapshot.get_metadata(), specs.metadata); +} } // namespace From 5502ceb0b371f3bd6f74e97a0891900e8cfa4ea4 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Mon, 16 Oct 2023 23:08:18 +0100 Subject: [PATCH 006/139] [tests] Check snapshots index --- tests/test_base_snapshot.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index 47196f5a43..09ad2a5914 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -127,4 +127,13 @@ TEST_F(TestBaseSnapshot, adopts_custom_metadata) auto snapshot = MockBaseSnapshot{"snapshot", "", nullptr, specs, vm}; EXPECT_EQ(snapshot.get_metadata(), specs.metadata); } + +TEST_F(TestBaseSnapshot, adopts_next_index) +{ + int count = 123; + EXPECT_CALL(vm, get_snapshot_count).WillOnce(Return(count)); + + auto snapshot = MockBaseSnapshot{"tau", "ceti", nullptr, specs, vm}; + EXPECT_EQ(snapshot.get_index(), count + 1); +} } // namespace From abe717c09009e8ff5b04ddb4f099a2e6c3988334 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Mon, 16 Oct 2023 23:08:28 +0100 Subject: [PATCH 007/139] [tests] Check parent properties --- tests/test_base_snapshot.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index 09ad2a5914..d56e4b3288 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -136,4 +136,18 @@ TEST_F(TestBaseSnapshot, adopts_next_index) auto snapshot = MockBaseSnapshot{"tau", "ceti", nullptr, specs, vm}; EXPECT_EQ(snapshot.get_index(), count + 1); } + +TEST_F(TestBaseSnapshot, retrieves_parents_properties) +{ + int parent_index = 11; + std::string parent_name = "parent"; + + EXPECT_CALL(vm, get_snapshot_count).WillOnce(Return(parent_index - 1)).WillOnce(Return(31)); + + auto parent = std::make_shared(parent_name, "", nullptr, specs, vm); + + auto child = MockBaseSnapshot{"child", "", parent, specs, vm}; + EXPECT_EQ(child.get_parents_index(), parent_index); + EXPECT_EQ(child.get_parents_name(), parent_name); +} } // namespace From ced651a2dca46c3fa7aca15e8f52816a09075500 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Mon, 16 Oct 2023 23:14:40 +0100 Subject: [PATCH 008/139] [tests] Check snapshot timestamp --- tests/test_base_snapshot.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index d56e4b3288..fdd63bf4f6 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -150,4 +150,15 @@ TEST_F(TestBaseSnapshot, retrieves_parents_properties) EXPECT_EQ(child.get_parents_index(), parent_index); EXPECT_EQ(child.get_parents_name(), parent_name); } + +TEST_F(TestBaseSnapshot, adopts_current_timestamp) +{ + auto before = QDateTime::currentDateTimeUtc(); + auto snapshot = MockBaseSnapshot{"foo", "", nullptr, specs, vm}; + auto after = QDateTime::currentDateTimeUtc(); + + EXPECT_GE(snapshot.get_creation_timestamp(), before); + EXPECT_LE(snapshot.get_creation_timestamp(), after); +} + } // namespace From 089e58bfa0999be0dbd528b911001c6570f35c7f Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 15 Nov 2023 12:06:08 +0000 Subject: [PATCH 009/139] [tests] Check required state for snapshots --- tests/test_base_snapshot.cpp | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index fdd63bf4f6..639bfb465f 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -161,4 +161,26 @@ TEST_F(TestBaseSnapshot, adopts_current_timestamp) EXPECT_LE(snapshot.get_creation_timestamp(), after); } +class TestSnapshotRejectedStates : public TestBaseSnapshot, public WithParamInterface +{ +}; + +TEST_P(TestSnapshotRejectedStates, rejects_active_state) +{ + specs.state = GetParam(); + MP_EXPECT_THROW_THAT((MockBaseSnapshot{"snapshot", "comment", nullptr, specs, vm}), + std::runtime_error, + mpt::match_what(HasSubstr("Unsupported VM state"))); +} + +INSTANTIATE_TEST_SUITE_P(TestBaseSnapshot, + TestSnapshotRejectedStates, + Values(mp::VirtualMachine::State::starting, + mp::VirtualMachine::State::restarting, + mp::VirtualMachine::State::running, + mp::VirtualMachine::State::delayed_shutdown, + mp::VirtualMachine::State::suspending, + mp::VirtualMachine::State::suspended, + mp::VirtualMachine::State::unknown)); + } // namespace From 993355e1e46e49f74aaf6588b0ea04d270ae1507 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 15 Nov 2023 12:24:58 +0000 Subject: [PATCH 010/139] [tests] Check invalid number of cores in snapshot --- tests/test_base_snapshot.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index 639bfb465f..1c0f7ba426 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -183,4 +183,18 @@ INSTANTIATE_TEST_SUITE_P(TestBaseSnapshot, mp::VirtualMachine::State::suspended, mp::VirtualMachine::State::unknown)); +class TestSnapshotInvalidCores : public TestBaseSnapshot, public WithParamInterface +{ +}; + +TEST_P(TestSnapshotInvalidCores, rejects_invalid_number_of_cores) +{ + specs.num_cores = GetParam(); + MP_EXPECT_THROW_THAT((MockBaseSnapshot{"snapshot", "comment", nullptr, specs, vm}), + std::runtime_error, + mpt::match_what(HasSubstr("Invalid number of cores"))); +} + +INSTANTIATE_TEST_SUITE_P(TestBaseSnapshot, TestSnapshotInvalidCores, Values(0, -1, -12345, -3e9)); + } // namespace From 9059a35aaa2ae681a83ecd8eeedc408337dd58ab Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 15 Nov 2023 12:29:25 +0000 Subject: [PATCH 011/139] [tests] Verify invalid memory and disk in snapshot --- tests/test_base_snapshot.cpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index 1c0f7ba426..d04451515e 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -197,4 +197,20 @@ TEST_P(TestSnapshotInvalidCores, rejects_invalid_number_of_cores) INSTANTIATE_TEST_SUITE_P(TestBaseSnapshot, TestSnapshotInvalidCores, Values(0, -1, -12345, -3e9)); +TEST_F(TestBaseSnapshot, rejects_null_memory_size) +{ + specs.mem_size = mp::MemorySize{"0B"}; + MP_EXPECT_THROW_THAT((MockBaseSnapshot{"snapshot", "comment", nullptr, specs, vm}), + std::runtime_error, + mpt::match_what(HasSubstr("Invalid memory size"))); +} + +TEST_F(TestBaseSnapshot, rejects_null_disk_size) +{ + specs.disk_space = mp::MemorySize{"0B"}; + MP_EXPECT_THROW_THAT((MockBaseSnapshot{"snapshot", "comment", nullptr, specs, vm}), + std::runtime_error, + mpt::match_what(HasSubstr("Invalid disk size"))); +} + } // namespace From 47d8312ab0fede70f3f9cb4369867e7683599662 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 15 Nov 2023 16:43:22 +0000 Subject: [PATCH 012/139] [tests] Reorganize includes slightly --- tests/test_base_snapshot.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index d04451515e..241bacc2ec 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -18,10 +18,9 @@ #include "common.h" #include "mock_virtual_machine.h" -#include "multipass/vm_specs.h" -#include "shared/base_snapshot.h" - #include +#include +#include #include From 6bb202e966b0b0b49224f1fbd803ecf9f0c42c85 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 15 Nov 2023 17:28:06 +0000 Subject: [PATCH 013/139] [tests] Check snapshot reconstruction from JSON --- tests/test_base_snapshot.cpp | 8 +++++ tests/test_data/test_snapshot.json | 48 ++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) create mode 100644 tests/test_data/test_snapshot.json diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index 241bacc2ec..2c168a6245 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -16,7 +16,9 @@ */ #include "common.h" +#include "file_operations.h" #include "mock_virtual_machine.h" +#include "path.h" #include #include @@ -53,6 +55,7 @@ struct TestBaseSnapshot : public Test return ret; } + static constexpr auto* test_json_filename = "test_snapshot.json"; mp::VMSpecs specs = stub_specs(); mpt::MockVirtualMachine vm{"a-vm"}; // TODO@no-merge nice? }; @@ -212,4 +215,9 @@ TEST_F(TestBaseSnapshot, rejects_null_disk_size) mpt::match_what(HasSubstr("Invalid disk size"))); } +TEST_F(TestBaseSnapshot, reconstructs_from_json) +{ + MockBaseSnapshot{multipass::test::test_data_path_for(test_json_filename), vm}; +} + } // namespace diff --git a/tests/test_data/test_snapshot.json b/tests/test_data/test_snapshot.json new file mode 100644 index 0000000000..0c24c7a430 --- /dev/null +++ b/tests/test_data/test_snapshot.json @@ -0,0 +1,48 @@ +{ + "snapshot": { + "comment": "A comment", + "creation_timestamp": "2023-11-15T12:35:13.585Z", + "disk_space": "5368709120", + "index": 3, + "mem_size": "1073741824", + "metadata": { + "arguments": [ + "-bios", + "OVMF.fd", + "--enable-kvm", + "-cpu", + "host", + "-nic", + "tap,ifname=tap-a3a4a846832,script=no,downscript=no,model=virtio-net-pci,mac=52:54:00:2a:84:b9", + "-device", + "virtio-scsi-pci,id=scsi0", + "-drive", + "file=/var/snap/multipass/common/data/multipassd/vault/instances/phlegmatic-orca/ubuntu-22.04-server-cloudimg-amd64.img,if=none,format=qcow2,discard=unmap,id=hda", + "-device", + "scsi-hd,drive=hda,bus=scsi0.0", + "-smp", + "1", + "-m", + "1024M", + "-qmp", + "stdio", + "-chardev", + "null,id=char0", + "-serial", + "chardev:char0", + "-nographic", + "-cdrom", + "/var/snap/multipass/common/data/multipassd/vault/instances/phlegmatic-orca/cloud-init-config.iso" + ], + "machine_type": "pc-i440fx-8.0", + "mount_data": { + } + }, + "mounts": [ + ], + "name": "snapshot3", + "num_cores": 1, + "parent": 2, + "state": 0 + } +} From 84e2bfa534e82a88e4f196b067de66424c13e793 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 15 Nov 2023 19:05:31 +0000 Subject: [PATCH 014/139] [tests] Fix dangling temporary path in MockVM --- tests/mock_virtual_machine.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/mock_virtual_machine.h b/tests/mock_virtual_machine.h index eb596d6f69..a0c2f1bf8b 100644 --- a/tests/mock_virtual_machine.h +++ b/tests/mock_virtual_machine.h @@ -25,6 +25,8 @@ #include #include +#include + using namespace testing; namespace multipass @@ -41,7 +43,7 @@ struct MockVirtualMachineT : public T template MockVirtualMachineT(std::unique_ptr&& tmp_dir, Args&&... args) - : T{std::forward(args)..., tmp_dir->path()} + : T{std::forward(args)..., tmp_dir->path()}, tmp_dir{std::move(tmp_dir)} { ON_CALL(*this, current_state()).WillByDefault(Return(multipass::VirtualMachine::State::off)); ON_CALL(*this, ssh_port()).WillByDefault(Return(42)); @@ -92,6 +94,8 @@ struct MockVirtualMachineT : public T MOCK_METHOD(void, load_snapshots, (), (override)); MOCK_METHOD(std::vector, get_childrens_names, (const Snapshot*), (const, override)); MOCK_METHOD(int, get_snapshot_count, (), (const, override)); + + std::unique_ptr tmp_dir; }; using MockVirtualMachine = MockVirtualMachineT<>; From 5bca2e1c91f88a9df1f51901ba2977e2ad1012d6 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 15 Nov 2023 19:06:24 +0000 Subject: [PATCH 015/139] [tests] Check snapshot name from JSON --- tests/test_base_snapshot.cpp | 46 ++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index 2c168a6245..2a8ec30039 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -24,6 +24,9 @@ #include #include +#include +#include + #include namespace mp = multipass; @@ -55,6 +58,39 @@ struct TestBaseSnapshot : public Test return ret; } + static QJsonObject test_snapshot_json() + { + static auto json_doc = [] { + QJsonParseError parse_error{}; + const auto ret = QJsonDocument::fromJson(mpt::load_test_file(test_json_filename), &parse_error); + if (parse_error.error) + throw std::runtime_error{ + fmt::format("Bad JSON test data in {}; error: {}", test_json_filename, parse_error.errorString())}; + return ret; + }(); + + return json_doc.object(); + } + + static void mod_snapshot_json(QJsonObject& json, const QString& key, QJsonValue new_value) + { + const auto snapshot_key = QStringLiteral("snapshot"); + auto snapshot_json_ref = json[snapshot_key]; + auto snapshot_json_copy = snapshot_json_ref.toObject(); + snapshot_json_copy[key] = std::move(new_value); + snapshot_json_ref = std::move(snapshot_json_copy); + } + + QString plant_snapshot_json(const QJsonObject& object, const QString& filename = "snapshot.json") const + { + const auto file_path = vm.tmp_dir->filePath(filename); + + QJsonDocument doc{object}; + mpt::make_file_with_content(file_path, doc.toJson().toStdString()); + + return file_path; + } + static constexpr auto* test_json_filename = "test_snapshot.json"; mp::VMSpecs specs = stub_specs(); mpt::MockVirtualMachine vm{"a-vm"}; // TODO@no-merge nice? @@ -220,4 +256,14 @@ TEST_F(TestBaseSnapshot, reconstructs_from_json) MockBaseSnapshot{multipass::test::test_data_path_for(test_json_filename), vm}; } +TEST_F(TestBaseSnapshot, adopts_name_from_json) +{ + constexpr auto* snapshot_name = "cheeseball"; + auto json = test_snapshot_json(); + mod_snapshot_json(json, "name", snapshot_name); + + auto snapshot = MockBaseSnapshot{plant_snapshot_json(json), vm}; + EXPECT_EQ(snapshot.get_name(), snapshot_name); +} + } // namespace From d29b24d5e7f1bef8771dbdefc948b14126162f50 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 15 Nov 2023 19:45:49 +0000 Subject: [PATCH 016/139] [tests] Check snapshot comment from JSON --- tests/test_base_snapshot.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index 2a8ec30039..882dd2dc42 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -266,4 +266,14 @@ TEST_F(TestBaseSnapshot, adopts_name_from_json) EXPECT_EQ(snapshot.get_name(), snapshot_name); } +TEST_F(TestBaseSnapshot, adopts_comment_from_json) +{ + constexpr auto* snapshot_comment = "Look behind you, a three-headed monkey!"; + auto json = test_snapshot_json(); + mod_snapshot_json(json, "comment", snapshot_comment); + + auto snapshot = MockBaseSnapshot{plant_snapshot_json(json), vm}; + EXPECT_EQ(snapshot.get_comment(), snapshot_comment); +} + } // namespace From 3c5b0bdb20ee8ba98fbe8f4c670457835840d13a Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Tue, 21 Nov 2023 18:50:23 +0000 Subject: [PATCH 017/139] [tests] Use camelCase names in base snapshot tests --- tests/test_base_snapshot.cpp | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index 882dd2dc42..f0bb415ca2 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -96,14 +96,14 @@ struct TestBaseSnapshot : public Test mpt::MockVirtualMachine vm{"a-vm"}; // TODO@no-merge nice? }; -TEST_F(TestBaseSnapshot, adopts_given_valid_name) +TEST_F(TestBaseSnapshot, adoptsGivenValidName) { auto name = "a-name"; auto snapshot = MockBaseSnapshot{name, "", nullptr, specs, vm}; EXPECT_EQ(snapshot.get_name(), name); } -TEST_F(TestBaseSnapshot, rejects_empty_name) +TEST_F(TestBaseSnapshot, rejectsEmptyName) { std::string empty{}; MP_EXPECT_THROW_THAT((MockBaseSnapshot{empty, "asdf", nullptr, specs, vm}), @@ -111,27 +111,27 @@ TEST_F(TestBaseSnapshot, rejects_empty_name) mpt::match_what(HasSubstr("empty"))); } -TEST_F(TestBaseSnapshot, adopts_given_comment) +TEST_F(TestBaseSnapshot, adoptsGivenComment) { auto comment = "some comment"; auto snapshot = MockBaseSnapshot{"whatever", comment, nullptr, specs, vm}; EXPECT_EQ(snapshot.get_comment(), comment); } -TEST_F(TestBaseSnapshot, adopts_given_parent) +TEST_F(TestBaseSnapshot, adoptsGivenParent) { auto parent = std::make_shared("root", "asdf", nullptr, specs, vm); auto snapshot = MockBaseSnapshot{"descendant", "descends", parent, specs, vm}; EXPECT_EQ(snapshot.get_parent(), parent); } -TEST_F(TestBaseSnapshot, adopts_null_parent) +TEST_F(TestBaseSnapshot, adoptsNullParent) { auto snapshot = MockBaseSnapshot{"descendant", "descends", nullptr, specs, vm}; EXPECT_EQ(snapshot.get_parent(), nullptr); } -TEST_F(TestBaseSnapshot, adopts_given_specs) +TEST_F(TestBaseSnapshot, adoptsGivenSpecs) { auto snapshot = MockBaseSnapshot{"snapshot", "", nullptr, specs, vm}; EXPECT_EQ(snapshot.get_num_cores(), specs.num_cores); @@ -142,7 +142,7 @@ TEST_F(TestBaseSnapshot, adopts_given_specs) EXPECT_EQ(snapshot.get_metadata(), specs.metadata); } -TEST_F(TestBaseSnapshot, adopts_custom_mounts) +TEST_F(TestBaseSnapshot, adoptsCustomMounts) { specs.mounts["toto"] = mp::VMMount{"src", {{123, 234}, {567, 678}}, {{19, 91}}, multipass::VMMount::MountType::Classic}; @@ -153,7 +153,7 @@ TEST_F(TestBaseSnapshot, adopts_custom_mounts) EXPECT_EQ(snapshot.get_mounts(), specs.mounts); } -TEST_F(TestBaseSnapshot, adopts_custom_metadata) +TEST_F(TestBaseSnapshot, adoptsCustomMetadata) { QJsonObject json; QJsonObject data; @@ -166,7 +166,7 @@ TEST_F(TestBaseSnapshot, adopts_custom_metadata) EXPECT_EQ(snapshot.get_metadata(), specs.metadata); } -TEST_F(TestBaseSnapshot, adopts_next_index) +TEST_F(TestBaseSnapshot, adoptsNextIndex) { int count = 123; EXPECT_CALL(vm, get_snapshot_count).WillOnce(Return(count)); @@ -175,7 +175,7 @@ TEST_F(TestBaseSnapshot, adopts_next_index) EXPECT_EQ(snapshot.get_index(), count + 1); } -TEST_F(TestBaseSnapshot, retrieves_parents_properties) +TEST_F(TestBaseSnapshot, retrievesParentsProperties) { int parent_index = 11; std::string parent_name = "parent"; @@ -189,7 +189,7 @@ TEST_F(TestBaseSnapshot, retrieves_parents_properties) EXPECT_EQ(child.get_parents_name(), parent_name); } -TEST_F(TestBaseSnapshot, adopts_current_timestamp) +TEST_F(TestBaseSnapshot, adoptsCurrentTimestamp) { auto before = QDateTime::currentDateTimeUtc(); auto snapshot = MockBaseSnapshot{"foo", "", nullptr, specs, vm}; @@ -203,7 +203,7 @@ class TestSnapshotRejectedStates : public TestBaseSnapshot, public WithParamInte { }; -TEST_P(TestSnapshotRejectedStates, rejects_active_state) +TEST_P(TestSnapshotRejectedStates, rejectsActiveState) { specs.state = GetParam(); MP_EXPECT_THROW_THAT((MockBaseSnapshot{"snapshot", "comment", nullptr, specs, vm}), @@ -225,7 +225,7 @@ class TestSnapshotInvalidCores : public TestBaseSnapshot, public WithParamInterf { }; -TEST_P(TestSnapshotInvalidCores, rejects_invalid_number_of_cores) +TEST_P(TestSnapshotInvalidCores, rejectsInvalidNumberOfCores) { specs.num_cores = GetParam(); MP_EXPECT_THROW_THAT((MockBaseSnapshot{"snapshot", "comment", nullptr, specs, vm}), @@ -235,7 +235,7 @@ TEST_P(TestSnapshotInvalidCores, rejects_invalid_number_of_cores) INSTANTIATE_TEST_SUITE_P(TestBaseSnapshot, TestSnapshotInvalidCores, Values(0, -1, -12345, -3e9)); -TEST_F(TestBaseSnapshot, rejects_null_memory_size) +TEST_F(TestBaseSnapshot, rejectsNullMemorySize) { specs.mem_size = mp::MemorySize{"0B"}; MP_EXPECT_THROW_THAT((MockBaseSnapshot{"snapshot", "comment", nullptr, specs, vm}), @@ -243,7 +243,7 @@ TEST_F(TestBaseSnapshot, rejects_null_memory_size) mpt::match_what(HasSubstr("Invalid memory size"))); } -TEST_F(TestBaseSnapshot, rejects_null_disk_size) +TEST_F(TestBaseSnapshot, rejectsNullDiskSize) { specs.disk_space = mp::MemorySize{"0B"}; MP_EXPECT_THROW_THAT((MockBaseSnapshot{"snapshot", "comment", nullptr, specs, vm}), @@ -251,12 +251,12 @@ TEST_F(TestBaseSnapshot, rejects_null_disk_size) mpt::match_what(HasSubstr("Invalid disk size"))); } -TEST_F(TestBaseSnapshot, reconstructs_from_json) +TEST_F(TestBaseSnapshot, reconstructsFromJson) { MockBaseSnapshot{multipass::test::test_data_path_for(test_json_filename), vm}; } -TEST_F(TestBaseSnapshot, adopts_name_from_json) +TEST_F(TestBaseSnapshot, adoptsNameFromJson) { constexpr auto* snapshot_name = "cheeseball"; auto json = test_snapshot_json(); @@ -266,7 +266,7 @@ TEST_F(TestBaseSnapshot, adopts_name_from_json) EXPECT_EQ(snapshot.get_name(), snapshot_name); } -TEST_F(TestBaseSnapshot, adopts_comment_from_json) +TEST_F(TestBaseSnapshot, adoptsCommentFromJson) { constexpr auto* snapshot_comment = "Look behind you, a three-headed monkey!"; auto json = test_snapshot_json(); From ca9f60043487ac6aac5c6f27ec87694551f54ab1 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Tue, 21 Nov 2023 19:19:30 +0000 Subject: [PATCH 018/139] [tests] Check snapshot parent from JSON --- tests/test_base_snapshot.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index f0bb415ca2..63c06820a2 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -276,4 +276,18 @@ TEST_F(TestBaseSnapshot, adoptsCommentFromJson) EXPECT_EQ(snapshot.get_comment(), snapshot_comment); } +TEST_F(TestBaseSnapshot, linksToParentFromJson) +{ + constexpr auto parent_idx = 42; + constexpr auto parent_name = "s42"; + auto json = test_snapshot_json(); + mod_snapshot_json(json, "parent", parent_idx); + + EXPECT_CALL(vm, get_snapshot(TypedEq(parent_idx))) + .WillOnce(Return(std::make_shared(parent_name, "mock parent snapshot", nullptr, specs, vm))); + + auto snapshot = MockBaseSnapshot{plant_snapshot_json(json), vm}; + EXPECT_EQ(snapshot.get_parents_name(), parent_name); +} + } // namespace From fa811d36cbf154c0d28daabe3b4197169b59e80b Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Tue, 21 Nov 2023 19:24:04 +0000 Subject: [PATCH 019/139] [tests] Remove no-op moves Unfortunately assignment to QJsonValueRef does not support moving. --- tests/test_base_snapshot.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index 63c06820a2..ce9681d597 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -72,13 +72,13 @@ struct TestBaseSnapshot : public Test return json_doc.object(); } - static void mod_snapshot_json(QJsonObject& json, const QString& key, QJsonValue new_value) + static void mod_snapshot_json(QJsonObject& json, const QString& key, const QJsonValue& new_value) { const auto snapshot_key = QStringLiteral("snapshot"); auto snapshot_json_ref = json[snapshot_key]; auto snapshot_json_copy = snapshot_json_ref.toObject(); - snapshot_json_copy[key] = std::move(new_value); - snapshot_json_ref = std::move(snapshot_json_copy); + snapshot_json_copy[key] = new_value; + snapshot_json_ref = snapshot_json_copy; } QString plant_snapshot_json(const QJsonObject& object, const QString& filename = "snapshot.json") const From 750868233d389140d5f8a53849fa561ff13acae3 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Tue, 21 Nov 2023 19:32:52 +0000 Subject: [PATCH 020/139] [tests] Add a few missing consts --- tests/test_base_snapshot.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index ce9681d597..bb9a63be6e 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -85,7 +85,7 @@ struct TestBaseSnapshot : public Test { const auto file_path = vm.tmp_dir->filePath(filename); - QJsonDocument doc{object}; + const QJsonDocument doc{object}; mpt::make_file_with_content(file_path, doc.toJson().toStdString()); return file_path; @@ -98,14 +98,14 @@ struct TestBaseSnapshot : public Test TEST_F(TestBaseSnapshot, adoptsGivenValidName) { - auto name = "a-name"; + constexpr auto name = "a-name"; auto snapshot = MockBaseSnapshot{name, "", nullptr, specs, vm}; EXPECT_EQ(snapshot.get_name(), name); } TEST_F(TestBaseSnapshot, rejectsEmptyName) { - std::string empty{}; + const std::string empty{}; MP_EXPECT_THROW_THAT((MockBaseSnapshot{empty, "asdf", nullptr, specs, vm}), std::runtime_error, mpt::match_what(HasSubstr("empty"))); @@ -113,14 +113,14 @@ TEST_F(TestBaseSnapshot, rejectsEmptyName) TEST_F(TestBaseSnapshot, adoptsGivenComment) { - auto comment = "some comment"; + constexpr auto comment = "some comment"; auto snapshot = MockBaseSnapshot{"whatever", comment, nullptr, specs, vm}; EXPECT_EQ(snapshot.get_comment(), comment); } TEST_F(TestBaseSnapshot, adoptsGivenParent) { - auto parent = std::make_shared("root", "asdf", nullptr, specs, vm); + const auto parent = std::make_shared("root", "asdf", nullptr, specs, vm); auto snapshot = MockBaseSnapshot{"descendant", "descends", parent, specs, vm}; EXPECT_EQ(snapshot.get_parent(), parent); } @@ -168,7 +168,7 @@ TEST_F(TestBaseSnapshot, adoptsCustomMetadata) TEST_F(TestBaseSnapshot, adoptsNextIndex) { - int count = 123; + const int count = 123; EXPECT_CALL(vm, get_snapshot_count).WillOnce(Return(count)); auto snapshot = MockBaseSnapshot{"tau", "ceti", nullptr, specs, vm}; @@ -177,8 +177,8 @@ TEST_F(TestBaseSnapshot, adoptsNextIndex) TEST_F(TestBaseSnapshot, retrievesParentsProperties) { - int parent_index = 11; - std::string parent_name = "parent"; + constexpr auto parent_name = "parent"; + const int parent_index = 11; EXPECT_CALL(vm, get_snapshot_count).WillOnce(Return(parent_index - 1)).WillOnce(Return(31)); From 388121d9c1ebd6ea911ade2f1e31eda0b025a8e7 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 22 Nov 2023 12:15:57 +0000 Subject: [PATCH 021/139] [tests] Check snapshot index from JSON --- tests/test_base_snapshot.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index bb9a63be6e..86abd34223 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -290,4 +290,13 @@ TEST_F(TestBaseSnapshot, linksToParentFromJson) EXPECT_EQ(snapshot.get_parents_name(), parent_name); } +TEST_F(TestBaseSnapshot, adoptsIndexFromJson) +{ + constexpr auto index = 31; + auto json = test_snapshot_json(); + mod_snapshot_json(json, "index", index); + + auto snapshot = MockBaseSnapshot{plant_snapshot_json(json), vm}; + EXPECT_EQ(snapshot.get_index(), index); +} } // namespace From 2603808a96486310f89c70ff473f3c81eb96af2d Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 22 Nov 2023 12:25:10 +0000 Subject: [PATCH 022/139] [tests] Check snapshot timestamp from JSON --- tests/test_base_snapshot.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index 86abd34223..f425729fad 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -299,4 +299,15 @@ TEST_F(TestBaseSnapshot, adoptsIndexFromJson) auto snapshot = MockBaseSnapshot{plant_snapshot_json(json), vm}; EXPECT_EQ(snapshot.get_index(), index); } + +TEST_F(TestBaseSnapshot, adoptsTimestampFromJson) +{ + constexpr auto timestamp = "1990-10-01T01:02:03.999Z"; + auto json = test_snapshot_json(); + mod_snapshot_json(json, "creation_timestamp", timestamp); + + auto snapshot = MockBaseSnapshot{plant_snapshot_json(json), vm}; + EXPECT_EQ(snapshot.get_creation_timestamp().toString(Qt::ISODateWithMs), timestamp); +} + } // namespace From 3dc41ab0b1c0d848fcb0afd7c9093ec044fcb188 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 22 Nov 2023 12:42:51 +0000 Subject: [PATCH 023/139] [tests] Check snapshot cores from JSON --- tests/test_base_snapshot.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index f425729fad..32b927a36f 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -310,4 +310,13 @@ TEST_F(TestBaseSnapshot, adoptsTimestampFromJson) EXPECT_EQ(snapshot.get_creation_timestamp().toString(Qt::ISODateWithMs), timestamp); } +TEST_F(TestBaseSnapshot, adoptsNumCoresFromJson) +{ + constexpr auto num_cores = 9; + auto json = test_snapshot_json(); + mod_snapshot_json(json, "num_cores", num_cores); + + auto snapshot = MockBaseSnapshot{plant_snapshot_json(json), vm}; + EXPECT_EQ(snapshot.get_num_cores(), num_cores); +} } // namespace From 8ddf57c98b1df25843ac26e5fa33075efdec0354 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 22 Nov 2023 12:43:06 +0000 Subject: [PATCH 024/139] [tests] Check snapshot mem from JSON --- tests/test_base_snapshot.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index 32b927a36f..992c9e576d 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -319,4 +319,15 @@ TEST_F(TestBaseSnapshot, adoptsNumCoresFromJson) auto snapshot = MockBaseSnapshot{plant_snapshot_json(json), vm}; EXPECT_EQ(snapshot.get_num_cores(), num_cores); } + +TEST_F(TestBaseSnapshot, adoptsMemSizeFromJson) +{ + constexpr auto mem = "1073741824"; + auto json = test_snapshot_json(); + mod_snapshot_json(json, "mem_size", mem); + + auto snapshot = MockBaseSnapshot{plant_snapshot_json(json), vm}; + EXPECT_EQ(snapshot.get_mem_size().in_bytes(), QString{mem}.toInt()); +} + } // namespace From 0bcd54fae35caa6eea785ec174bd2642332fdec4 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 22 Nov 2023 12:46:18 +0000 Subject: [PATCH 025/139] [tests] Check snapshot disk from JSON --- tests/test_base_snapshot.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index 992c9e576d..31fadaafd1 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -330,4 +330,14 @@ TEST_F(TestBaseSnapshot, adoptsMemSizeFromJson) EXPECT_EQ(snapshot.get_mem_size().in_bytes(), QString{mem}.toInt()); } +TEST_F(TestBaseSnapshot, adoptsDiskSpaceFromJson) +{ + constexpr auto disk = "1073741824"; + auto json = test_snapshot_json(); + mod_snapshot_json(json, "disk_space", disk); + + auto snapshot = MockBaseSnapshot{plant_snapshot_json(json), vm}; + EXPECT_EQ(snapshot.get_disk_space().in_bytes(), QString{disk}.toInt()); +} + } // namespace From bd1c1f846706e524ef4c2a819ad928d6e9495b33 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 22 Nov 2023 13:04:32 +0000 Subject: [PATCH 026/139] [tests] Check snapshot state from JSON --- tests/test_base_snapshot.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index 31fadaafd1..254e9a49eb 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -340,4 +340,13 @@ TEST_F(TestBaseSnapshot, adoptsDiskSpaceFromJson) EXPECT_EQ(snapshot.get_disk_space().in_bytes(), QString{disk}.toInt()); } +TEST_F(TestBaseSnapshot, adoptsStateFromJson) +{ + constexpr auto state = mp::VirtualMachine::State::stopped; + auto json = test_snapshot_json(); + mod_snapshot_json(json, "state", static_cast(state)); + + auto snapshot = MockBaseSnapshot{plant_snapshot_json(json), vm}; + EXPECT_EQ(snapshot.get_state(), state); +} } // namespace From e6cf46bff66f61ca44996c2b531a4106f829881c Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 22 Nov 2023 13:05:05 +0000 Subject: [PATCH 027/139] [tests] Check snapshot metadata from JSON --- tests/test_base_snapshot.cpp | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index 254e9a49eb..689fdbd924 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -349,4 +349,26 @@ TEST_F(TestBaseSnapshot, adoptsStateFromJson) auto snapshot = MockBaseSnapshot{plant_snapshot_json(json), vm}; EXPECT_EQ(snapshot.get_state(), state); } + +TEST_F(TestBaseSnapshot, adoptsMetadataFromJson) +{ + auto metadata = QJsonObject{}; + metadata["arguments"] = "Meathook:\n" + "You've got a real attitude problem!\n" + "\n" + "Guybrush Threepwood:\n" + "Well... you've got a real hair problem!\n" + "\n" + "Meathook:\n" + "You just don't know when to quit, do you?\n" + "\n" + "Guybrush Threepwood:\n" + "Neither did your barber."; + + auto json = test_snapshot_json(); + mod_snapshot_json(json, "metadata", metadata); + + auto snapshot = MockBaseSnapshot{plant_snapshot_json(json), vm}; + EXPECT_EQ(snapshot.get_metadata(), metadata); +} } // namespace From f3e5b3d11f043d3ec49d32097b966e1f795e8046 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 22 Nov 2023 18:20:53 +0000 Subject: [PATCH 028/139] [tests] Check snapshot mounts from JSON --- tests/test_base_snapshot.cpp | 58 ++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index 689fdbd924..5e057ed18f 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -24,6 +24,7 @@ #include #include +#include #include #include @@ -371,4 +372,61 @@ TEST_F(TestBaseSnapshot, adoptsMetadataFromJson) auto snapshot = MockBaseSnapshot{plant_snapshot_json(json), vm}; EXPECT_EQ(snapshot.get_metadata(), metadata); } + +TEST_F(TestBaseSnapshot, adoptsMountsFromJson) +{ + constexpr auto src_path = "You fight like a dairy farmer."; + constexpr auto dst_path = "How appropriate. You fight like a cow."; + constexpr auto host_uid = 1, instance_uid = 2, host_gid = 3, instance_gid = 4; + constexpr auto mount_type = mp::VMMount::MountType::Native; + + QJsonArray mounts{}; + QJsonObject mount{}; + QJsonArray uid_mappings{}; + QJsonObject uid_mapping{}; + QJsonArray gid_mappings{}; + QJsonObject gid_mapping{}; + + uid_mapping["host_uid"] = host_uid; + uid_mapping["instance_uid"] = instance_uid; + uid_mappings.append(uid_mapping); + + gid_mapping["host_gid"] = host_gid; + gid_mapping["instance_gid"] = instance_gid; + gid_mappings.append(gid_mapping); + + mount["source_path"] = src_path; + mount["target_path"] = dst_path; + mount["uid_mappings"] = uid_mappings; + mount["gid_mappings"] = gid_mappings; + mount["mount_type"] = static_cast(mount_type); + + mounts.append(mount); + + auto json = test_snapshot_json(); + mod_snapshot_json(json, "mounts", mounts); + + auto snapshot = MockBaseSnapshot{plant_snapshot_json(json), vm}; + auto snapshot_mounts = snapshot.get_mounts(); + + ASSERT_THAT(snapshot_mounts, SizeIs(mounts.size())); + const auto [snapshot_mnt_dst, snapshot_mount] = *snapshot_mounts.begin(); + + EXPECT_EQ(snapshot_mnt_dst, dst_path); + EXPECT_EQ(snapshot_mount.source_path, src_path); + EXPECT_EQ(snapshot_mount.mount_type, mount_type); + + ASSERT_THAT(snapshot_mount.uid_mappings, SizeIs(uid_mappings.size())); + const auto [snapshot_host_uid, snapshot_instance_uid] = snapshot_mount.uid_mappings.front(); + + EXPECT_EQ(snapshot_host_uid, host_uid); + EXPECT_EQ(snapshot_instance_uid, instance_uid); + + ASSERT_THAT(snapshot_mount.gid_mappings, SizeIs(gid_mappings.size())); + const auto [snapshot_host_gid, snapshot_instance_gid] = snapshot_mount.gid_mappings.front(); + + EXPECT_EQ(snapshot_host_gid, host_gid); + EXPECT_EQ(snapshot_instance_gid, instance_gid); +} + } // namespace From ca85ff82a84ee86914adacb5e790a6633167e7f9 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 22 Nov 2023 19:05:52 +0000 Subject: [PATCH 029/139] [snapshot] Fix assert when loading from disk Move assert of non-negative indices from constructor that is called when loading snapshots from files that could have been tampered with (we need to throw in that case). --- src/platform/backends/shared/base_snapshot.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/platform/backends/shared/base_snapshot.cpp b/src/platform/backends/shared/base_snapshot.cpp index d613993b95..03b84d3c46 100644 --- a/src/platform/backends/shared/base_snapshot.cpp +++ b/src/platform/backends/shared/base_snapshot.cpp @@ -124,7 +124,6 @@ mp::BaseSnapshot::BaseSnapshot(const std::string& name, // NOLINT(modernize-p storage_dir{storage_dir}, captured{captured} { - assert(index > 0 && "snapshot indices need to start at 1"); using St = VirtualMachine::State; if (state != St::off && state != St::stopped) throw std::runtime_error{fmt::format("Unsupported VM state in snapshot: {}", static_cast(state))}; @@ -161,6 +160,7 @@ mp::BaseSnapshot::BaseSnapshot(const std::string& name, vm.instance_directory(), /*captured=*/false} { + assert(index > 0 && "snapshot indices need to start at 1"); } mp::BaseSnapshot::BaseSnapshot(const QString& filename, VirtualMachine& vm) From 52993d0fe6d2aa501119c1ea10855b143a692e3c Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 22 Nov 2023 19:08:33 +0000 Subject: [PATCH 030/139] [tests] Check refused non-positive snapshot index --- tests/test_base_snapshot.cpp | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index 5e057ed18f..611f8a80bc 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -429,4 +429,21 @@ TEST_F(TestBaseSnapshot, adoptsMountsFromJson) EXPECT_EQ(snapshot_instance_gid, instance_gid); } +class TestSnapshotRejectedNonPositiveIndices : public TestBaseSnapshot, public WithParamInterface +{ +}; + +TEST_P(TestSnapshotRejectedNonPositiveIndices, refusesNonPositiveIndexFromJson) +{ + const auto index = GetParam(); + auto json = test_snapshot_json(); + mod_snapshot_json(json, "index", index); + + MP_EXPECT_THROW_THAT((MockBaseSnapshot{plant_snapshot_json(json), vm}), + std::runtime_error, + mpt::match_what(AllOf(HasSubstr("not positive"), HasSubstr(std::to_string(index))))); +} + +INSTANTIATE_TEST_SUITE_P(TestBaseSnapshot, TestSnapshotRejectedNonPositiveIndices, Values(0, -1, -31)); + } // namespace From 8b3f8231ca0edde8e38cf227dcb6fa1558f46e32 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 22 Nov 2023 19:13:53 +0000 Subject: [PATCH 031/139] [tests] Check refused snapshot index over max --- tests/test_base_snapshot.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index 611f8a80bc..0c50239c01 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -446,4 +446,15 @@ TEST_P(TestSnapshotRejectedNonPositiveIndices, refusesNonPositiveIndexFromJson) INSTANTIATE_TEST_SUITE_P(TestBaseSnapshot, TestSnapshotRejectedNonPositiveIndices, Values(0, -1, -31)); +TEST_F(TestBaseSnapshot, refusesIndexAboveMax) +{ + constexpr auto index = 25623956; + auto json = test_snapshot_json(); + mod_snapshot_json(json, "index", index); + + MP_EXPECT_THROW_THAT((MockBaseSnapshot{plant_snapshot_json(json), vm}), + std::runtime_error, + mpt::match_what(AllOf(HasSubstr("Maximum"), HasSubstr(std::to_string(index))))); +} + } // namespace From 0b1c7fba9e15588a397bad19265b9c7dfbfc623a Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 22 Nov 2023 19:51:27 +0000 Subject: [PATCH 032/139] [tests] Check snapshot persistence on edition --- tests/test_base_snapshot.cpp | 62 ++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index 0c50239c01..733bb2307f 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -29,6 +29,7 @@ #include #include +#include namespace mp = multipass; namespace mpt = multipass::test; @@ -44,8 +45,37 @@ class MockBaseSnapshot : public mp::BaseSnapshot MOCK_METHOD(void, capture_impl, (), (override)); MOCK_METHOD(void, erase_impl, (), (override)); MOCK_METHOD(void, apply_impl, (), (override)); + + friend bool operator==(const MockBaseSnapshot& a, const MockBaseSnapshot& b); }; +bool operator==(const MockBaseSnapshot& a, const MockBaseSnapshot& b) +{ + return std::tuple(a.get_index(), + a.get_name(), + a.get_comment(), + a.get_creation_timestamp(), + a.get_num_cores(), + a.get_mem_size(), + a.get_disk_space(), + a.get_state(), + a.get_mounts(), + a.get_metadata(), + a.get_parent(), + a.get_id()) == std::tuple(b.get_index(), + b.get_name(), + b.get_comment(), + b.get_creation_timestamp(), + b.get_num_cores(), + b.get_mem_size(), + b.get_disk_space(), + b.get_state(), + b.get_mounts(), + b.get_metadata(), + b.get_parent(), + b.get_id()); +} + struct TestBaseSnapshot : public Test { static mp::VMSpecs stub_specs() @@ -92,6 +122,11 @@ struct TestBaseSnapshot : public Test return file_path; } + QString derive_persisted_snapshot_filename(int index) + { + return vm.tmp_dir->filePath(QString{"%1"}.arg(index, 4, 10, QLatin1Char('0')) + ".snapshot.json"); + } + static constexpr auto* test_json_filename = "test_snapshot.json"; mp::VMSpecs specs = stub_specs(); mpt::MockVirtualMachine vm{"a-vm"}; // TODO@no-merge nice? @@ -457,4 +492,31 @@ TEST_F(TestBaseSnapshot, refusesIndexAboveMax) mpt::match_what(AllOf(HasSubstr("Maximum"), HasSubstr(std::to_string(index))))); } +class TestSnapshotPersistence : public TestBaseSnapshot, + public WithParamInterface> +{ +}; + +TEST_P(TestSnapshotPersistence, persistsOnEdition) +{ + constexpr auto index = 55; + auto setter = GetParam(); + + auto json = test_snapshot_json(); + mod_snapshot_json(json, "index", index); + + MockBaseSnapshot snapshot_orig{plant_snapshot_json(json), vm}; + setter(snapshot_orig); + + const auto filename = derive_persisted_snapshot_filename(index); + const MockBaseSnapshot snapshot_edited{filename, vm}; + EXPECT_EQ(snapshot_edited, snapshot_orig); +} + +INSTANTIATE_TEST_SUITE_P(TestBaseSnapshot, + TestSnapshotPersistence, + Values([](MockBaseSnapshot& s) { s.set_name("asdf"); }, + [](MockBaseSnapshot& s) { s.set_comment("fdsa"); }, + [](MockBaseSnapshot& s) { s.set_parent(nullptr); })); + } // namespace From 44cb34bbd62ce46e59050bc9bc0f7f9cc681b3a0 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 22 Nov 2023 20:33:18 +0000 Subject: [PATCH 033/139] [tests] Check snapshot setters --- tests/test_base_snapshot.cpp | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index 733bb2307f..6648c374c3 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -492,6 +492,34 @@ TEST_F(TestBaseSnapshot, refusesIndexAboveMax) mpt::match_what(AllOf(HasSubstr("Maximum"), HasSubstr(std::to_string(index))))); } +TEST_F(TestBaseSnapshot, setsName) +{ + constexpr auto new_name = "Murray"; + auto snapshot = MockBaseSnapshot{multipass::test::test_data_path_for(test_json_filename), vm}; + + snapshot.set_name(new_name); + EXPECT_EQ(snapshot.get_name(), new_name); +} + +TEST_F(TestBaseSnapshot, setsComment) +{ + constexpr auto new_comment = "I once owned a dog that was smarter than you.\n" + "He must have taught you everything you know."; + auto snapshot = MockBaseSnapshot{multipass::test::test_data_path_for(test_json_filename), vm}; + + snapshot.set_comment(new_comment); + EXPECT_EQ(snapshot.get_comment(), new_comment); +} + +TEST_F(TestBaseSnapshot, setsParent) +{ + auto child = MockBaseSnapshot{multipass::test::test_data_path_for(test_json_filename), vm}; + auto parent = std::make_shared("parent", "", nullptr, specs, vm); + + child.set_parent(parent); + EXPECT_EQ(child.get_parent(), parent); +} + class TestSnapshotPersistence : public TestBaseSnapshot, public WithParamInterface> { From a3aa61c60bb4125c0d518dbaf86ba795ba130273 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Thu, 23 Nov 2023 11:47:36 +0000 Subject: [PATCH 034/139] [tests] Check capture persists snapshots --- tests/test_base_snapshot.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index 6648c374c3..1f40c677a5 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -547,4 +547,13 @@ INSTANTIATE_TEST_SUITE_P(TestBaseSnapshot, [](MockBaseSnapshot& s) { s.set_comment("fdsa"); }, [](MockBaseSnapshot& s) { s.set_parent(nullptr); })); +TEST_F(TestBaseSnapshot, capturePersists) +{ + MockBaseSnapshot snapshot{"Big Whoop", "treasure", nullptr, specs, vm}; + snapshot.capture(); + + const auto expected_file = QFileInfo{derive_persisted_snapshot_filename(snapshot.get_index())}; + EXPECT_TRUE(expected_file.exists()); + EXPECT_TRUE(expected_file.isFile()); +} } // namespace From bedbadb9afeaf111624cea5f452d26c3d1340a87 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Thu, 23 Nov 2023 11:48:41 +0000 Subject: [PATCH 035/139] [tests] Check operations that delegate to impl --- tests/test_base_snapshot.cpp | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index 1f40c677a5..950747d3c9 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -556,4 +556,30 @@ TEST_F(TestBaseSnapshot, capturePersists) EXPECT_TRUE(expected_file.exists()); EXPECT_TRUE(expected_file.isFile()); } + +TEST_F(TestBaseSnapshot, captureCallsImpl) +{ + MockBaseSnapshot snapshot{"LeChuck", "'s Revenge", nullptr, specs, vm}; + EXPECT_CALL(snapshot, capture_impl).Times(1); + + snapshot.capture(); +} + +TEST_F(TestBaseSnapshot, applyCallsImpl) +{ + MockBaseSnapshot snapshot{"Guybrush", "fears porcelain", nullptr, specs, vm}; + EXPECT_CALL(snapshot, apply_impl).Times(1); + + snapshot.apply(); +} + +TEST_F(TestBaseSnapshot, eraseCallsImpl) +{ + MockBaseSnapshot snapshot{"House of Mojo", "voodoo", nullptr, specs, vm}; + snapshot.capture(); + + EXPECT_CALL(snapshot, erase_impl).Times(1); + snapshot.erase(); +} + } // namespace From 48b6ccb4df87a2029ea16af7f8e4a5db924f1225 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Thu, 23 Nov 2023 12:09:09 +0000 Subject: [PATCH 036/139] [tests] Verify that erase deletes snapshot file --- tests/test_base_snapshot.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index 950747d3c9..a212d0e557 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -582,4 +582,16 @@ TEST_F(TestBaseSnapshot, eraseCallsImpl) snapshot.erase(); } +TEST_F(TestBaseSnapshot, eraseRemovesFile) +{ + MockBaseSnapshot snapshot{"House of Mojo", "voodoo", nullptr, specs, vm}; + snapshot.capture(); + + const auto expected_filename = derive_persisted_snapshot_filename(snapshot.get_index()); + ASSERT_TRUE(QFileInfo{expected_filename}.exists()); + + snapshot.erase(); + EXPECT_FALSE(QFileInfo{expected_filename}.exists()); +} + } // namespace From bbd86a765e3f5722e20aa377157f13dc0dbab02f Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Thu, 23 Nov 2023 12:30:44 +0000 Subject: [PATCH 037/139] [tests] Check erase throws if unable to move file --- tests/test_base_snapshot.cpp | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index a212d0e557..1ef442ac59 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -17,6 +17,7 @@ #include "common.h" #include "file_operations.h" +#include "mock_file_ops.h" #include "mock_virtual_machine.h" #include "path.h" @@ -594,4 +595,18 @@ TEST_F(TestBaseSnapshot, eraseRemovesFile) EXPECT_FALSE(QFileInfo{expected_filename}.exists()); } +TEST_F(TestBaseSnapshot, eraseThrowsIfUnableToRenameFile) +{ + MockBaseSnapshot snapshot{"voodoo-sword", "Cursed Cutlass of Kaflu", nullptr, specs, vm}; + snapshot.capture(); + + auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); + const auto expected_filename = derive_persisted_snapshot_filename(snapshot.get_index()); + EXPECT_CALL(*mock_file_ops, rename(Property(&QFile::fileName, Eq(expected_filename)), _)).WillOnce(Return(false)); + + MP_EXPECT_THROW_THAT(snapshot.erase(), + std::runtime_error, + mpt::match_what(HasSubstr("Failed to move snapshot file"))); +} + } // namespace From b5b3c60b4b26b5023018599e164a6b90aeb84dbe Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Thu, 23 Nov 2023 16:13:08 +0000 Subject: [PATCH 038/139] [tests] Check rollback on failure to erase --- tests/test_base_snapshot.cpp | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index 1ef442ac59..f23f09b1dd 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -609,4 +609,32 @@ TEST_F(TestBaseSnapshot, eraseThrowsIfUnableToRenameFile) mpt::match_what(HasSubstr("Failed to move snapshot file"))); } +TEST_F(TestBaseSnapshot, restoresFileOnFailureToErase) +{ + MockBaseSnapshot snapshot{"ultimate-insult", + "A powerful weapon capable of crippling even the toughest pirate's ego.", + nullptr, + specs, + vm}; + snapshot.capture(); + + const auto expected_filename = derive_persisted_snapshot_filename(snapshot.get_index()); + ASSERT_TRUE(QFileInfo{expected_filename}.exists()); + + EXPECT_CALL(snapshot, erase_impl).WillOnce([&expected_filename] { + ASSERT_FALSE(QFileInfo{expected_filename}.exists()); + throw std::runtime_error{"test"}; + }); + + try + { + snapshot.erase(); + FAIL() << "shouldn't be here"; + } + catch (const std::runtime_error&) + { + EXPECT_TRUE(QFileInfo{expected_filename}.exists()); + } +} + } // namespace From 1682f42bbfd6c7e6913db4acefcd33fe2e93a43a Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Thu, 23 Nov 2023 16:24:12 +0000 Subject: [PATCH 039/139] [tests] Cover mounts in JSON-based snapshot tests --- tests/test_data/test_snapshot.json | 34 ++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/test_data/test_snapshot.json b/tests/test_data/test_snapshot.json index 0c24c7a430..fc8a889895 100644 --- a/tests/test_data/test_snapshot.json +++ b/tests/test_data/test_snapshot.json @@ -39,6 +39,40 @@ } }, "mounts": [ + { + "gid_mappings": [ + { + "host_gid": 1000, + "instance_gid": -1 + } + ], + "mount_type": 1, + "source_path": "/home/talking/skull", + "target_path": "murray", + "uid_mappings": [ + { + "host_uid": 1000, + "instance_uid": -1 + } + ] + }, + { + "gid_mappings": [ + { + "host_gid": 1000, + "instance_gid": -1 + } + ], + "mount_type": 0, + "source_path": "/home/mighty/pirate", + "target_path": "guybrush", + "uid_mappings": [ + { + "host_uid": 1000, + "instance_uid": -1 + } + ] + } ], "name": "snapshot3", "num_cores": 1, From 5d7356ad394b4a57fd3d29f293b2c8db47c80d1d Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Thu, 23 Nov 2023 17:15:18 +0000 Subject: [PATCH 040/139] [tests] Check handling of inability to open file --- tests/test_base_snapshot.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index f23f09b1dd..08348b9557 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -637,4 +637,18 @@ TEST_F(TestBaseSnapshot, restoresFileOnFailureToErase) } } +TEST_F(TestBaseSnapshot, throwsIfUnableToOpenFile) +{ + auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); + + const auto snapshot_filename = multipass::test::test_data_path_for(test_json_filename); + EXPECT_CALL(*mock_file_ops, open(Property(&QFileDevice::fileName, Eq(snapshot_filename)), _)) + .WillOnce(Return(false)); + + MP_EXPECT_THROW_THAT( + (MockBaseSnapshot{snapshot_filename, vm}), + std::runtime_error, + mpt::match_what(AllOf(HasSubstr("Could not open"), HasSubstr(snapshot_filename.toStdString())))); +} + } // namespace From f5e9414d563ff1ac5e692353329f88ac4a8120eb Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Thu, 23 Nov 2023 18:32:03 +0000 Subject: [PATCH 041/139] [tests] Test snapshot handling of bad JSON --- tests/test_base_snapshot.cpp | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index 08348b9557..561c2b73b8 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -651,4 +651,32 @@ TEST_F(TestBaseSnapshot, throwsIfUnableToOpenFile) mpt::match_what(AllOf(HasSubstr("Could not open"), HasSubstr(snapshot_filename.toStdString())))); } +TEST_F(TestBaseSnapshot, throwsOnEmptyJson) +{ + const auto snapshot_filename = plant_snapshot_json(QJsonObject{}); + MP_EXPECT_THROW_THAT((MockBaseSnapshot{snapshot_filename, vm}), + std::runtime_error, + mpt::match_what(HasSubstr("Empty"))); +} + +TEST_F(TestBaseSnapshot, throwsOnBadFormat) +{ + const auto snapshot_filename = vm.tmp_dir->filePath("wrong"); + mpt::make_file_with_content( + snapshot_filename, + "(Guybrush): Can I call you Bob?\n" + "\n" + "(Murray): You may call me Murray! I am a powerful demonic force! I'm the harbinger of your doom, and the " + "forces of darkness will applaude me as I stride through the gates of hell, carrying your head on a pike!\n" + "\n" + "(Guybrush): \"Stride\"?\n" + "\n" + "(Murray): Alright, then. ROLL! I shall ROLL through the gates of hell! Must you take the fun out of " + "everything?"); + + MP_EXPECT_THROW_THAT((MockBaseSnapshot{snapshot_filename, vm}), + std::runtime_error, + mpt::match_what(HasSubstr("Could not parse snapshot JSON"))); +} + } // namespace From c5a12d8ff9ac0dc29fe18c232ae0ebd9e15e13cd Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Thu, 23 Nov 2023 18:32:28 +0000 Subject: [PATCH 042/139] [snapshot] Fix wrong order in error message --- src/platform/backends/shared/base_snapshot.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/platform/backends/shared/base_snapshot.cpp b/src/platform/backends/shared/base_snapshot.cpp index 03b84d3c46..7b42cd4e2b 100644 --- a/src/platform/backends/shared/base_snapshot.cpp +++ b/src/platform/backends/shared/base_snapshot.cpp @@ -60,8 +60,8 @@ QJsonObject read_snapshot_json(const QString& filename) if (const auto json = QJsonDocument::fromJson(data, &parse_error).object(); parse_error.error) throw std::runtime_error{fmt::format("Could not parse snapshot JSON; error: {}; file: {}", - file.fileName(), - parse_error.errorString())}; + parse_error.errorString(), + file.fileName())}; else if (json.isEmpty()) throw std::runtime_error{fmt::format("Empty snapshot JSON: {}", file.fileName())}; else From c73385da3ff9430a6b52ecc6e47312c10ed1a030 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Thu, 23 Nov 2023 18:44:05 +0000 Subject: [PATCH 043/139] [tests] Check handling of missing snapshot parent --- tests/test_base_snapshot.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index 561c2b73b8..95a73d06c7 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -679,4 +679,12 @@ TEST_F(TestBaseSnapshot, throwsOnBadFormat) mpt::match_what(HasSubstr("Could not parse snapshot JSON"))); } +TEST_F(TestBaseSnapshot, throwsOnMissingParent) +{ + EXPECT_CALL(vm, get_snapshot(An())).WillOnce(Throw(std::out_of_range{"Incognito"})); + MP_EXPECT_THROW_THAT((MockBaseSnapshot{multipass::test::test_data_path_for(test_json_filename), vm}), + std::runtime_error, + mpt::match_what(HasSubstr("Missing snapshot parent"))); // TODO@ricab extract file path +} + } // namespace From e893695f8804474b87c2f6975c184d6db6dfd2ab Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Thu, 23 Nov 2023 19:12:25 +0000 Subject: [PATCH 044/139] [tests] Refactor test-snapshot file path --- tests/test_base_snapshot.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index 95a73d06c7..261f8ad5fd 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -131,6 +131,7 @@ struct TestBaseSnapshot : public Test static constexpr auto* test_json_filename = "test_snapshot.json"; mp::VMSpecs specs = stub_specs(); mpt::MockVirtualMachine vm{"a-vm"}; // TODO@no-merge nice? + QString test_json_filepath = mpt::test_data_path_for(test_json_filename); }; TEST_F(TestBaseSnapshot, adoptsGivenValidName) @@ -290,7 +291,7 @@ TEST_F(TestBaseSnapshot, rejectsNullDiskSize) TEST_F(TestBaseSnapshot, reconstructsFromJson) { - MockBaseSnapshot{multipass::test::test_data_path_for(test_json_filename), vm}; + MockBaseSnapshot{test_json_filepath, vm}; } TEST_F(TestBaseSnapshot, adoptsNameFromJson) @@ -496,7 +497,7 @@ TEST_F(TestBaseSnapshot, refusesIndexAboveMax) TEST_F(TestBaseSnapshot, setsName) { constexpr auto new_name = "Murray"; - auto snapshot = MockBaseSnapshot{multipass::test::test_data_path_for(test_json_filename), vm}; + auto snapshot = MockBaseSnapshot{test_json_filepath, vm}; snapshot.set_name(new_name); EXPECT_EQ(snapshot.get_name(), new_name); @@ -506,7 +507,7 @@ TEST_F(TestBaseSnapshot, setsComment) { constexpr auto new_comment = "I once owned a dog that was smarter than you.\n" "He must have taught you everything you know."; - auto snapshot = MockBaseSnapshot{multipass::test::test_data_path_for(test_json_filename), vm}; + auto snapshot = MockBaseSnapshot{test_json_filepath, vm}; snapshot.set_comment(new_comment); EXPECT_EQ(snapshot.get_comment(), new_comment); @@ -514,7 +515,7 @@ TEST_F(TestBaseSnapshot, setsComment) TEST_F(TestBaseSnapshot, setsParent) { - auto child = MockBaseSnapshot{multipass::test::test_data_path_for(test_json_filename), vm}; + auto child = MockBaseSnapshot{test_json_filepath, vm}; auto parent = std::make_shared("parent", "", nullptr, specs, vm); child.set_parent(parent); @@ -641,7 +642,7 @@ TEST_F(TestBaseSnapshot, throwsIfUnableToOpenFile) { auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); - const auto snapshot_filename = multipass::test::test_data_path_for(test_json_filename); + const auto snapshot_filename = test_json_filepath; EXPECT_CALL(*mock_file_ops, open(Property(&QFileDevice::fileName, Eq(snapshot_filename)), _)) .WillOnce(Return(false)); @@ -682,9 +683,9 @@ TEST_F(TestBaseSnapshot, throwsOnBadFormat) TEST_F(TestBaseSnapshot, throwsOnMissingParent) { EXPECT_CALL(vm, get_snapshot(An())).WillOnce(Throw(std::out_of_range{"Incognito"})); - MP_EXPECT_THROW_THAT((MockBaseSnapshot{multipass::test::test_data_path_for(test_json_filename), vm}), + MP_EXPECT_THROW_THAT((MockBaseSnapshot{test_json_filepath, vm}), std::runtime_error, - mpt::match_what(HasSubstr("Missing snapshot parent"))); // TODO@ricab extract file path + mpt::match_what(HasSubstr("Missing snapshot parent"))); } } // namespace From eef6012ae57d43e522649786c123afcb02e29fe8 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Thu, 23 Nov 2023 19:02:41 +0000 Subject: [PATCH 045/139] [tests] Name file-path symbols consistently --- tests/test_base_snapshot.cpp | 57 ++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index 261f8ad5fd..3c4f42ccd7 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -123,7 +123,7 @@ struct TestBaseSnapshot : public Test return file_path; } - QString derive_persisted_snapshot_filename(int index) + QString derive_persisted_snapshot_file_path(int index) { return vm.tmp_dir->filePath(QString{"%1"}.arg(index, 4, 10, QLatin1Char('0')) + ".snapshot.json"); } @@ -131,7 +131,7 @@ struct TestBaseSnapshot : public Test static constexpr auto* test_json_filename = "test_snapshot.json"; mp::VMSpecs specs = stub_specs(); mpt::MockVirtualMachine vm{"a-vm"}; // TODO@no-merge nice? - QString test_json_filepath = mpt::test_data_path_for(test_json_filename); + QString test_json_file_path = mpt::test_data_path_for(test_json_filename); }; TEST_F(TestBaseSnapshot, adoptsGivenValidName) @@ -291,7 +291,7 @@ TEST_F(TestBaseSnapshot, rejectsNullDiskSize) TEST_F(TestBaseSnapshot, reconstructsFromJson) { - MockBaseSnapshot{test_json_filepath, vm}; + MockBaseSnapshot{test_json_file_path, vm}; } TEST_F(TestBaseSnapshot, adoptsNameFromJson) @@ -497,7 +497,7 @@ TEST_F(TestBaseSnapshot, refusesIndexAboveMax) TEST_F(TestBaseSnapshot, setsName) { constexpr auto new_name = "Murray"; - auto snapshot = MockBaseSnapshot{test_json_filepath, vm}; + auto snapshot = MockBaseSnapshot{test_json_file_path, vm}; snapshot.set_name(new_name); EXPECT_EQ(snapshot.get_name(), new_name); @@ -507,7 +507,7 @@ TEST_F(TestBaseSnapshot, setsComment) { constexpr auto new_comment = "I once owned a dog that was smarter than you.\n" "He must have taught you everything you know."; - auto snapshot = MockBaseSnapshot{test_json_filepath, vm}; + auto snapshot = MockBaseSnapshot{test_json_file_path, vm}; snapshot.set_comment(new_comment); EXPECT_EQ(snapshot.get_comment(), new_comment); @@ -515,7 +515,7 @@ TEST_F(TestBaseSnapshot, setsComment) TEST_F(TestBaseSnapshot, setsParent) { - auto child = MockBaseSnapshot{test_json_filepath, vm}; + auto child = MockBaseSnapshot{test_json_file_path, vm}; auto parent = std::make_shared("parent", "", nullptr, specs, vm); child.set_parent(parent); @@ -538,8 +538,8 @@ TEST_P(TestSnapshotPersistence, persistsOnEdition) MockBaseSnapshot snapshot_orig{plant_snapshot_json(json), vm}; setter(snapshot_orig); - const auto filename = derive_persisted_snapshot_filename(index); - const MockBaseSnapshot snapshot_edited{filename, vm}; + const auto file_path = derive_persisted_snapshot_file_path(index); + const MockBaseSnapshot snapshot_edited{file_path, vm}; EXPECT_EQ(snapshot_edited, snapshot_orig); } @@ -554,7 +554,7 @@ TEST_F(TestBaseSnapshot, capturePersists) MockBaseSnapshot snapshot{"Big Whoop", "treasure", nullptr, specs, vm}; snapshot.capture(); - const auto expected_file = QFileInfo{derive_persisted_snapshot_filename(snapshot.get_index())}; + const auto expected_file = QFileInfo{derive_persisted_snapshot_file_path(snapshot.get_index())}; EXPECT_TRUE(expected_file.exists()); EXPECT_TRUE(expected_file.isFile()); } @@ -589,11 +589,11 @@ TEST_F(TestBaseSnapshot, eraseRemovesFile) MockBaseSnapshot snapshot{"House of Mojo", "voodoo", nullptr, specs, vm}; snapshot.capture(); - const auto expected_filename = derive_persisted_snapshot_filename(snapshot.get_index()); - ASSERT_TRUE(QFileInfo{expected_filename}.exists()); + const auto expected_file_path = derive_persisted_snapshot_file_path(snapshot.get_index()); + ASSERT_TRUE(QFileInfo{expected_file_path}.exists()); snapshot.erase(); - EXPECT_FALSE(QFileInfo{expected_filename}.exists()); + EXPECT_FALSE(QFileInfo{expected_file_path}.exists()); } TEST_F(TestBaseSnapshot, eraseThrowsIfUnableToRenameFile) @@ -602,8 +602,8 @@ TEST_F(TestBaseSnapshot, eraseThrowsIfUnableToRenameFile) snapshot.capture(); auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); - const auto expected_filename = derive_persisted_snapshot_filename(snapshot.get_index()); - EXPECT_CALL(*mock_file_ops, rename(Property(&QFile::fileName, Eq(expected_filename)), _)).WillOnce(Return(false)); + const auto expected_file_path = derive_persisted_snapshot_file_path(snapshot.get_index()); + EXPECT_CALL(*mock_file_ops, rename(Property(&QFile::fileName, Eq(expected_file_path)), _)).WillOnce(Return(false)); MP_EXPECT_THROW_THAT(snapshot.erase(), std::runtime_error, @@ -619,11 +619,11 @@ TEST_F(TestBaseSnapshot, restoresFileOnFailureToErase) vm}; snapshot.capture(); - const auto expected_filename = derive_persisted_snapshot_filename(snapshot.get_index()); - ASSERT_TRUE(QFileInfo{expected_filename}.exists()); + const auto expected_file_path = derive_persisted_snapshot_file_path(snapshot.get_index()); + ASSERT_TRUE(QFileInfo{expected_file_path}.exists()); - EXPECT_CALL(snapshot, erase_impl).WillOnce([&expected_filename] { - ASSERT_FALSE(QFileInfo{expected_filename}.exists()); + EXPECT_CALL(snapshot, erase_impl).WillOnce([&expected_file_path] { + ASSERT_FALSE(QFileInfo{expected_file_path}.exists()); throw std::runtime_error{"test"}; }); @@ -634,7 +634,7 @@ TEST_F(TestBaseSnapshot, restoresFileOnFailureToErase) } catch (const std::runtime_error&) { - EXPECT_TRUE(QFileInfo{expected_filename}.exists()); + EXPECT_TRUE(QFileInfo{expected_file_path}.exists()); } } @@ -642,29 +642,28 @@ TEST_F(TestBaseSnapshot, throwsIfUnableToOpenFile) { auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); - const auto snapshot_filename = test_json_filepath; - EXPECT_CALL(*mock_file_ops, open(Property(&QFileDevice::fileName, Eq(snapshot_filename)), _)) + EXPECT_CALL(*mock_file_ops, open(Property(&QFileDevice::fileName, Eq(test_json_file_path)), _)) .WillOnce(Return(false)); MP_EXPECT_THROW_THAT( - (MockBaseSnapshot{snapshot_filename, vm}), + (MockBaseSnapshot{test_json_file_path, vm}), std::runtime_error, - mpt::match_what(AllOf(HasSubstr("Could not open"), HasSubstr(snapshot_filename.toStdString())))); + mpt::match_what(AllOf(HasSubstr("Could not open"), HasSubstr(test_json_file_path.toStdString())))); } TEST_F(TestBaseSnapshot, throwsOnEmptyJson) { - const auto snapshot_filename = plant_snapshot_json(QJsonObject{}); - MP_EXPECT_THROW_THAT((MockBaseSnapshot{snapshot_filename, vm}), + const auto snapshot_file_path = plant_snapshot_json(QJsonObject{}); + MP_EXPECT_THROW_THAT((MockBaseSnapshot{snapshot_file_path, vm}), std::runtime_error, mpt::match_what(HasSubstr("Empty"))); } TEST_F(TestBaseSnapshot, throwsOnBadFormat) { - const auto snapshot_filename = vm.tmp_dir->filePath("wrong"); + const auto snapshot_file_path = vm.tmp_dir->filePath("wrong"); mpt::make_file_with_content( - snapshot_filename, + snapshot_file_path, "(Guybrush): Can I call you Bob?\n" "\n" "(Murray): You may call me Murray! I am a powerful demonic force! I'm the harbinger of your doom, and the " @@ -675,7 +674,7 @@ TEST_F(TestBaseSnapshot, throwsOnBadFormat) "(Murray): Alright, then. ROLL! I shall ROLL through the gates of hell! Must you take the fun out of " "everything?"); - MP_EXPECT_THROW_THAT((MockBaseSnapshot{snapshot_filename, vm}), + MP_EXPECT_THROW_THAT((MockBaseSnapshot{snapshot_file_path, vm}), std::runtime_error, mpt::match_what(HasSubstr("Could not parse snapshot JSON"))); } @@ -683,7 +682,7 @@ TEST_F(TestBaseSnapshot, throwsOnBadFormat) TEST_F(TestBaseSnapshot, throwsOnMissingParent) { EXPECT_CALL(vm, get_snapshot(An())).WillOnce(Throw(std::out_of_range{"Incognito"})); - MP_EXPECT_THROW_THAT((MockBaseSnapshot{test_json_filepath, vm}), + MP_EXPECT_THROW_THAT((MockBaseSnapshot{test_json_file_path, vm}), std::runtime_error, mpt::match_what(HasSubstr("Missing snapshot parent"))); } From ad0a29f669a8c5cee89ddd3219cf4539e8c08262 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Thu, 23 Nov 2023 19:09:02 +0000 Subject: [PATCH 046/139] [tests] Silence benign mock warnings --- tests/test_base_snapshot.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index 3c4f42ccd7..576e16cdf9 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -130,7 +130,7 @@ struct TestBaseSnapshot : public Test static constexpr auto* test_json_filename = "test_snapshot.json"; mp::VMSpecs specs = stub_specs(); - mpt::MockVirtualMachine vm{"a-vm"}; // TODO@no-merge nice? + NiceMock vm{"a-vm"}; QString test_json_file_path = mpt::test_data_path_for(test_json_filename); }; @@ -551,7 +551,7 @@ INSTANTIATE_TEST_SUITE_P(TestBaseSnapshot, TEST_F(TestBaseSnapshot, capturePersists) { - MockBaseSnapshot snapshot{"Big Whoop", "treasure", nullptr, specs, vm}; + NiceMock snapshot{"Big Whoop", "treasure", nullptr, specs, vm}; snapshot.capture(); const auto expected_file = QFileInfo{derive_persisted_snapshot_file_path(snapshot.get_index())}; @@ -577,7 +577,7 @@ TEST_F(TestBaseSnapshot, applyCallsImpl) TEST_F(TestBaseSnapshot, eraseCallsImpl) { - MockBaseSnapshot snapshot{"House of Mojo", "voodoo", nullptr, specs, vm}; + NiceMock snapshot{"House of Mojo", "voodoo", nullptr, specs, vm}; snapshot.capture(); EXPECT_CALL(snapshot, erase_impl).Times(1); @@ -586,7 +586,7 @@ TEST_F(TestBaseSnapshot, eraseCallsImpl) TEST_F(TestBaseSnapshot, eraseRemovesFile) { - MockBaseSnapshot snapshot{"House of Mojo", "voodoo", nullptr, specs, vm}; + NiceMock snapshot{"House of Mojo", "voodoo", nullptr, specs, vm}; snapshot.capture(); const auto expected_file_path = derive_persisted_snapshot_file_path(snapshot.get_index()); @@ -598,7 +598,7 @@ TEST_F(TestBaseSnapshot, eraseRemovesFile) TEST_F(TestBaseSnapshot, eraseThrowsIfUnableToRenameFile) { - MockBaseSnapshot snapshot{"voodoo-sword", "Cursed Cutlass of Kaflu", nullptr, specs, vm}; + NiceMock snapshot{"voodoo-sword", "Cursed Cutlass of Kaflu", nullptr, specs, vm}; snapshot.capture(); auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); @@ -612,11 +612,11 @@ TEST_F(TestBaseSnapshot, eraseThrowsIfUnableToRenameFile) TEST_F(TestBaseSnapshot, restoresFileOnFailureToErase) { - MockBaseSnapshot snapshot{"ultimate-insult", - "A powerful weapon capable of crippling even the toughest pirate's ego.", - nullptr, - specs, - vm}; + NiceMock snapshot{"ultimate-insult", + "A powerful weapon capable of crippling even the toughest pirate's ego.", + nullptr, + specs, + vm}; snapshot.capture(); const auto expected_file_path = derive_persisted_snapshot_file_path(snapshot.get_index()); From 34d13fa2a26066ce3ca2fc86aa30f330279fd4a3 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 24 Nov 2023 20:46:59 +0000 Subject: [PATCH 047/139] [tests] Add macros to delegate mock calls on base --- tests/common.h | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/common.h b/tests/common.h index d35d50d131..3967417afe 100644 --- a/tests/common.h +++ b/tests/common.h @@ -62,6 +62,21 @@ // work around warning: https://github.com/google/googletest/issues/2271#issuecomment-665742471 #define MP_TYPED_TEST_SUITE(suite_name, types_param) TYPED_TEST_SUITE(suite_name, types_param, ) +// Macros to make a mock delegate calls on a base class by default. +// For example, if `mock_widget` is an object of type `MockWidget` which mocks `Widget`, one can say: +// `MP_DELEGATE_MOCK_CALLS_ON_BASE(mock_widget, Widget, render);` +// This will cause calls to `mock_widget.render()` to delegate on the base implementation in `MockWidget`. +#define MP_DELEGATE_MOCK_CALLS_ON_BASE(mock, method, BaseT) \ + MP_DELEGATE_MOCK_CALLS_ON_BASE_WITH_MATCHERS(mock, method, BaseT, ) + +// This second form accepts matchers, which are useful to disambiguate overloaded methods. For example: +// `MP_DELEGATE_MOCK_CALLS_ON_BASE_WITH_MATCHERS(mock_widget, Widget, render, (A()))` +// This will redirect the version of `MockWidget::render` that takes one argument of type `Canvas`. +#define MP_DELEGATE_MOCK_CALLS_ON_BASE_WITH_MATCHERS(mock, method, BaseT, ...) \ + ON_CALL(mock, method __VA_ARGS__).WillByDefault([&mock](auto&&... args) { \ + return mock.BaseT::method(std::forward(args)...); \ + }) + // Teach GTest to print Qt stuff QT_BEGIN_NAMESPACE class QString; From ea37faa002226514c2cf7c2321812c740400f065 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Tue, 19 Dec 2023 11:36:47 +0000 Subject: [PATCH 048/139] [tests] Implement `MockBaseVirtualMachine` --- tests/test_base_virtual_machine.cpp | 35 +++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 917834ab10..a9f6477e5c 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -18,6 +18,7 @@ #include "common.h" #include "dummy_ssh_key_provider.h" #include "mock_ssh_test_fixture.h" +#include "mock_virtual_machine.h" #include "temp_dir.h" #include @@ -32,6 +33,40 @@ using namespace testing; namespace { +struct MockBaseVirtualMachine : public mpt::MockVirtualMachineT +{ + template + MockBaseVirtualMachine(Args&&... args) + : mpt::MockVirtualMachineT{std::forward(args)...} + { + auto& self = *this; + MP_DELEGATE_MOCK_CALLS_ON_BASE(self, take_snapshot, mp::BaseVirtualMachine); + MP_DELEGATE_MOCK_CALLS_ON_BASE(self, view_snapshots, mp::BaseVirtualMachine); + MP_DELEGATE_MOCK_CALLS_ON_BASE(self, get_num_snapshots, mp::BaseVirtualMachine); + MP_DELEGATE_MOCK_CALLS_ON_BASE(self, take_snapshot, mp::BaseVirtualMachine); + MP_DELEGATE_MOCK_CALLS_ON_BASE(self, rename_snapshot, mp::BaseVirtualMachine); + MP_DELEGATE_MOCK_CALLS_ON_BASE(self, delete_snapshot, mp::BaseVirtualMachine); + MP_DELEGATE_MOCK_CALLS_ON_BASE(self, restore_snapshot, mp::BaseVirtualMachine); + MP_DELEGATE_MOCK_CALLS_ON_BASE(self, load_snapshots, mp::BaseVirtualMachine); + MP_DELEGATE_MOCK_CALLS_ON_BASE(self, get_childrens_names, mp::BaseVirtualMachine); + MP_DELEGATE_MOCK_CALLS_ON_BASE(self, get_snapshot_count, mp::BaseVirtualMachine); + MP_DELEGATE_MOCK_CALLS_ON_BASE_WITH_MATCHERS(self, get_snapshot, mp::BaseVirtualMachine, (An())); + MP_DELEGATE_MOCK_CALLS_ON_BASE_WITH_MATCHERS(self, + get_snapshot, + mp::BaseVirtualMachine, + (A())); + } + + MOCK_METHOD(void, require_snapshots_support, (), (const, override)); + MOCK_METHOD(std::shared_ptr, make_specific_snapshot, (const QString& filename), (override)); + MOCK_METHOD(std::shared_ptr, + make_specific_snapshot, + (const std::string& snapshot_name, + const std::string& comment, + const mp::VMSpecs& specs, + std::shared_ptr parent), + (override)); +}; struct StubBaseVirtualMachine : public mp::BaseVirtualMachine { StubBaseVirtualMachine(mp::VirtualMachine::State s = mp::VirtualMachine::State::off) From 307a348ed44c789171e3e064cdadb1c531b81920 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 24 Nov 2023 20:56:36 +0000 Subject: [PATCH 049/139] [tests] Implement `MockSnapshot` --- tests/test_base_virtual_machine.cpp | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index a9f6477e5c..5be69cfd33 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -24,6 +24,7 @@ #include #include +#include #include namespace mp = multipass; @@ -67,6 +68,31 @@ struct MockBaseVirtualMachine : public mpt::MockVirtualMachineT parent), (override)); }; + +struct MockSnapshot : public mp::Snapshot +{ + MOCK_METHOD(int, get_index, (), (const, noexcept, override)); + MOCK_METHOD(std::string, get_name, (), (const, override)); + MOCK_METHOD(std::string, get_comment, (), (const, override)); + MOCK_METHOD(QDateTime, get_creation_timestamp, (), (const, noexcept, override)); + MOCK_METHOD(int, get_num_cores, (), (const, noexcept, override)); + MOCK_METHOD(mp::MemorySize, get_mem_size, (), (const, noexcept, override)); + MOCK_METHOD(mp::MemorySize, get_disk_space, (), (const, noexcept, override)); + MOCK_METHOD(mp::VirtualMachine::State, get_state, (), (const, noexcept, override)); + MOCK_METHOD((const std::unordered_map&), get_mounts, (), (const, noexcept, override)); + MOCK_METHOD(const QJsonObject&, get_metadata, (), (const, noexcept, override)); + MOCK_METHOD(std::shared_ptr, get_parent, (), (const, override)); + MOCK_METHOD(std::shared_ptr, get_parent, (), (override)); + MOCK_METHOD(std::string, get_parents_name, (), (const, override)); + MOCK_METHOD(int, get_parents_index, (), (const, override)); + MOCK_METHOD(void, set_name, (const std::string&), (override)); + MOCK_METHOD(void, set_comment, (const std::string&), (override)); + MOCK_METHOD(void, set_parent, (std::shared_ptr), (override)); + MOCK_METHOD(void, capture, (), (override)); + MOCK_METHOD(void, erase, (), (override)); + MOCK_METHOD(void, apply, (), (override)); +}; + struct StubBaseVirtualMachine : public mp::BaseVirtualMachine { StubBaseVirtualMachine(mp::VirtualMachine::State s = mp::VirtualMachine::State::off) From 489c227c7b85ab0c99ecc8832193234d0fa38aae Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 24 Nov 2023 20:57:07 +0000 Subject: [PATCH 050/139] [tests] Check that BaseVM takes/deletes snapshots --- tests/test_base_virtual_machine.cpp | 32 +++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 5be69cfd33..a3d507a881 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -26,6 +26,7 @@ #include #include #include +#include namespace mp = multipass; namespace mpl = multipass::logging; @@ -295,4 +296,35 @@ INSTANTIATE_TEST_SUITE_P( {"192.168.2.8", "192.168.3.1", "10.172.66.5"}}, IpTestParams{0, "", {}})); +TEST(BaseVMSnapshots, startsWithNoSnapshots) +{ + MockBaseVirtualMachine vm{"mock-vm"}; + EXPECT_EQ(vm.get_num_snapshots(), 0); +} + +TEST(BaseVMSnapshots, takesSnapshots) +{ + auto snapshot = std::make_shared(); + EXPECT_CALL(*snapshot, capture).Times(AnyNumber()); + + MockBaseVirtualMachine vm{"mock-vm"}; + EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillRepeatedly(Return(snapshot)); + + vm.take_snapshot(mp::VMSpecs{}, "s1", ""); + EXPECT_EQ(vm.get_num_snapshots(), 1); +} + +TEST(BaseVMSnapshots, deletesSnapshots) +{ + auto snapshot = std::make_shared(); + EXPECT_CALL(*snapshot, capture).Times(AnyNumber()); + EXPECT_CALL(*snapshot, erase).Times(AnyNumber()); + + MockBaseVirtualMachine vm{"mock-vm"}; + EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillRepeatedly(Return(snapshot)); + + vm.take_snapshot(mp::VMSpecs{}, "s1", ""); + vm.delete_snapshot("s1"); + EXPECT_EQ(vm.get_num_snapshots(), 0); +} } // namespace From 17859cee36a886bc540e035adf5fa35f721cfe78 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 24 Nov 2023 20:57:52 +0000 Subject: [PATCH 051/139] [tests] Check that BaseVM counts current snapshots --- tests/test_base_virtual_machine.cpp | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index a3d507a881..dd17a583ea 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -327,4 +327,33 @@ TEST(BaseVMSnapshots, deletesSnapshots) vm.delete_snapshot("s1"); EXPECT_EQ(vm.get_num_snapshots(), 0); } + +TEST(BaseVMSnapshots, countsCurrentSnapshots) +{ + const mp::VMSpecs specs{}; + MockBaseVirtualMachine vm{"mock-vm"}; + EXPECT_EQ(vm.get_num_snapshots(), 0); + + auto snapshot = std::make_shared(); + EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillRepeatedly(Return(snapshot)); + EXPECT_CALL(*snapshot, capture).Times(AnyNumber()); + EXPECT_CALL(*snapshot, erase).Times(AnyNumber()); + + vm.take_snapshot(specs, "s1", ""); + EXPECT_EQ(vm.get_num_snapshots(), 1); + + vm.take_snapshot(specs, "s2", ""); + vm.take_snapshot(specs, "s3", ""); + EXPECT_EQ(vm.get_num_snapshots(), 3); + + vm.delete_snapshot("s1"); + EXPECT_EQ(vm.get_num_snapshots(), 2); + + vm.delete_snapshot("s2"); + vm.delete_snapshot("s3"); + EXPECT_EQ(vm.get_num_snapshots(), 0); + + vm.take_snapshot(specs, "s4", ""); + EXPECT_EQ(vm.get_num_snapshots(), 1); +} } // namespace From efecaeed857cef11797d39d9f670e4c110d9a79d Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 24 Nov 2023 21:02:40 +0000 Subject: [PATCH 052/139] [tests] Check that BaseVM tracks snapshot totals --- tests/test_base_virtual_machine.cpp | 32 +++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index dd17a583ea..e6694766f6 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -356,4 +356,36 @@ TEST(BaseVMSnapshots, countsCurrentSnapshots) vm.take_snapshot(specs, "s4", ""); EXPECT_EQ(vm.get_num_snapshots(), 1); } + +TEST(BaseVMSnapshots, countsTotalSnapshots) +{ + const mp::VMSpecs specs{}; + MockBaseVirtualMachine vm{"mock-vm"}; + EXPECT_EQ(vm.get_num_snapshots(), 0); + + auto snapshot = std::make_shared(); + EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillRepeatedly(Return(snapshot)); + EXPECT_CALL(*snapshot, capture).Times(AnyNumber()); + EXPECT_CALL(*snapshot, erase).Times(AnyNumber()); + + vm.take_snapshot(specs, "s1", ""); + vm.take_snapshot(specs, "s2", ""); + vm.take_snapshot(specs, "s3", ""); + EXPECT_EQ(vm.get_snapshot_count(), 3); + + vm.take_snapshot(specs, "s4", ""); + vm.take_snapshot(specs, "s5", ""); + EXPECT_EQ(vm.get_snapshot_count(), 5); + + vm.delete_snapshot("s1"); + vm.delete_snapshot("s2"); + EXPECT_EQ(vm.get_snapshot_count(), 5); + + vm.delete_snapshot("s4"); + EXPECT_EQ(vm.get_snapshot_count(), 5); + + vm.take_snapshot(specs, "s6", ""); + EXPECT_EQ(vm.get_snapshot_count(), 6); +} + } // namespace From 972a02f4f77419f2a3a8210734350b08426434c3 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Thu, 7 Dec 2023 16:56:55 +0000 Subject: [PATCH 053/139] [tests] Move BaseVM-snapshot tests to common suite --- tests/test_base_virtual_machine.cpp | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index e6694766f6..6204dd68d5 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -201,6 +201,7 @@ struct BaseVM : public Test { mpt::MockSSHTestFixture mock_ssh_test_fixture; const mpt::DummyKeyProvider key_provider{"keeper of the seven keys"}; + MockBaseVirtualMachine vm{"mock-vm"}; }; TEST_F(BaseVM, get_all_ipv4_works_when_ssh_throws_opening_a_session) @@ -296,42 +297,38 @@ INSTANTIATE_TEST_SUITE_P( {"192.168.2.8", "192.168.3.1", "10.172.66.5"}}, IpTestParams{0, "", {}})); -TEST(BaseVMSnapshots, startsWithNoSnapshots) +TEST_F(BaseVM, startsWithNoSnapshots) { - MockBaseVirtualMachine vm{"mock-vm"}; EXPECT_EQ(vm.get_num_snapshots(), 0); } -TEST(BaseVMSnapshots, takesSnapshots) +TEST_F(BaseVM, takesSnapshots) { auto snapshot = std::make_shared(); EXPECT_CALL(*snapshot, capture).Times(AnyNumber()); - MockBaseVirtualMachine vm{"mock-vm"}; EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillRepeatedly(Return(snapshot)); - vm.take_snapshot(mp::VMSpecs{}, "s1", ""); + EXPECT_EQ(vm.get_num_snapshots(), 1); } -TEST(BaseVMSnapshots, deletesSnapshots) +TEST_F(BaseVM, deletesSnapshots) { auto snapshot = std::make_shared(); EXPECT_CALL(*snapshot, capture).Times(AnyNumber()); EXPECT_CALL(*snapshot, erase).Times(AnyNumber()); - MockBaseVirtualMachine vm{"mock-vm"}; EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillRepeatedly(Return(snapshot)); - vm.take_snapshot(mp::VMSpecs{}, "s1", ""); vm.delete_snapshot("s1"); + EXPECT_EQ(vm.get_num_snapshots(), 0); } -TEST(BaseVMSnapshots, countsCurrentSnapshots) +TEST_F(BaseVM, countsCurrentSnapshots) { const mp::VMSpecs specs{}; - MockBaseVirtualMachine vm{"mock-vm"}; EXPECT_EQ(vm.get_num_snapshots(), 0); auto snapshot = std::make_shared(); @@ -357,10 +354,9 @@ TEST(BaseVMSnapshots, countsCurrentSnapshots) EXPECT_EQ(vm.get_num_snapshots(), 1); } -TEST(BaseVMSnapshots, countsTotalSnapshots) +TEST_F(BaseVM, countsTotalSnapshots) { const mp::VMSpecs specs{}; - MockBaseVirtualMachine vm{"mock-vm"}; EXPECT_EQ(vm.get_num_snapshots(), 0); auto snapshot = std::make_shared(); From 475fe1c901ce2a898348845bb00694b3944c39d4 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Thu, 7 Dec 2023 16:57:32 +0000 Subject: [PATCH 054/139] [tests] Check that BaseVM provides a snapshot view --- tests/test_base_virtual_machine.cpp | 35 +++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 6204dd68d5..67b96b9c38 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -384,4 +384,39 @@ TEST_F(BaseVM, countsTotalSnapshots) EXPECT_EQ(vm.get_snapshot_count(), 6); } +TEST_F(BaseVM, providesSnapshotsView) +{ + const mp::VMSpecs specs{}; + EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillRepeatedly([this](auto&&...) { + auto ret = std::make_shared(); + EXPECT_CALL(*ret, capture).Times(AnyNumber()); + EXPECT_CALL(*ret, erase).Times(AnyNumber()); + EXPECT_CALL(*ret, get_index).WillRepeatedly(Return(vm.get_snapshot_count() + 1)); + + return ret; + }); + + auto sname = [](int i) { return fmt::format("s{}", i); }; + for (int i = 1; i < 6; ++i) // +5 + vm.take_snapshot(specs, sname(i), ""); + for (int i = 3; i < 5; ++i) // -2 + vm.delete_snapshot(sname(i)); + for (int i = 6; i < 9; ++i) // +3 + vm.take_snapshot(specs, sname(i), ""); + for (int i : {1, 7}) // -2 + vm.delete_snapshot(sname(i)); + + ASSERT_EQ(vm.get_num_snapshots(), 4); + auto snapshots = vm.view_snapshots(); + + EXPECT_THAT(snapshots, SizeIs(4)); + + std::vector snapshot_indices{}; + std::transform(snapshots.begin(), snapshots.end(), std::back_inserter(snapshot_indices), [](const auto& snapshot) { + return snapshot->get_index(); + }); + + EXPECT_THAT(snapshot_indices, UnorderedElementsAre(2, 5, 6, 8)); +} + } // namespace From 4af7df98603938b64587187e75310233f3e52a92 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Thu, 7 Dec 2023 22:20:11 +0000 Subject: [PATCH 055/139] [tests] Remove repeated method mocking --- tests/test_base_virtual_machine.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 67b96b9c38..611dbe2e37 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -42,7 +42,6 @@ struct MockBaseVirtualMachine : public mpt::MockVirtualMachineT{std::forward(args)...} { auto& self = *this; - MP_DELEGATE_MOCK_CALLS_ON_BASE(self, take_snapshot, mp::BaseVirtualMachine); MP_DELEGATE_MOCK_CALLS_ON_BASE(self, view_snapshots, mp::BaseVirtualMachine); MP_DELEGATE_MOCK_CALLS_ON_BASE(self, get_num_snapshots, mp::BaseVirtualMachine); MP_DELEGATE_MOCK_CALLS_ON_BASE(self, take_snapshot, mp::BaseVirtualMachine); From 16f4ea06ba8afc7ab4df742485b34522684eb298 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Thu, 7 Dec 2023 22:22:17 +0000 Subject: [PATCH 056/139] [tests] Mock missing const method overloads --- tests/test_base_virtual_machine.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 611dbe2e37..f31e865515 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -42,6 +42,7 @@ struct MockBaseVirtualMachine : public mpt::MockVirtualMachineT{std::forward(args)...} { auto& self = *this; + const auto& const_self = self; MP_DELEGATE_MOCK_CALLS_ON_BASE(self, view_snapshots, mp::BaseVirtualMachine); MP_DELEGATE_MOCK_CALLS_ON_BASE(self, get_num_snapshots, mp::BaseVirtualMachine); MP_DELEGATE_MOCK_CALLS_ON_BASE(self, take_snapshot, mp::BaseVirtualMachine); @@ -52,10 +53,15 @@ struct MockBaseVirtualMachine : public mpt::MockVirtualMachineT())); + MP_DELEGATE_MOCK_CALLS_ON_BASE_WITH_MATCHERS(const_self, get_snapshot, mp::BaseVirtualMachine, (An())); MP_DELEGATE_MOCK_CALLS_ON_BASE_WITH_MATCHERS(self, get_snapshot, mp::BaseVirtualMachine, (A())); + MP_DELEGATE_MOCK_CALLS_ON_BASE_WITH_MATCHERS(const_self, + get_snapshot, + mp::BaseVirtualMachine, + (A())); } MOCK_METHOD(void, require_snapshots_support, (), (const, override)); From 386eb49bd2353d2ad894697773ca209aca855749 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Thu, 7 Dec 2023 22:52:24 +0000 Subject: [PATCH 057/139] [tests] Check BaseVM provides snapshot by name --- tests/test_base_virtual_machine.cpp | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index f31e865515..e902496860 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -424,4 +424,26 @@ TEST_F(BaseVM, providesSnapshotsView) EXPECT_THAT(snapshot_indices, UnorderedElementsAre(2, 5, 6, 8)); } +TEST_F(BaseVM, providesSnapshotsByName) +{ + EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillRepeatedly(WithArg<0>([](const std::string& name) { + auto ret = std::make_shared(); + EXPECT_CALL(*ret, get_name).WillRepeatedly(Return(name)); + EXPECT_CALL(*ret, capture).Times(1); + + return ret; + })); + + const mp::VMSpecs specs{}; + const std::string target_name = "pick"; + vm.take_snapshot(specs, "foo", "irrelevant"); + vm.take_snapshot(specs, target_name, "fetch me"); + vm.take_snapshot(specs, "bar", "whatever"); + vm.take_snapshot(specs, "baz", ""); + vm.delete_snapshot("bar"); + vm.take_snapshot(specs, "asdf", ""); + + EXPECT_THAT(vm.get_snapshot(target_name), Pointee(Property(&mp::Snapshot::get_name, Eq(target_name)))); +} + } // namespace From f32a2b24a6aac9cb73e294c01f77693ec1c22614 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Thu, 7 Dec 2023 22:35:08 +0000 Subject: [PATCH 058/139] [tests] Use nice snapshot mocks in BaseVM tests Simplify expectations accordingly. --- tests/test_base_virtual_machine.cpp | 31 ++++++++++------------------- 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index e902496860..6a3895ea31 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -206,7 +206,7 @@ struct BaseVM : public Test { mpt::MockSSHTestFixture mock_ssh_test_fixture; const mpt::DummyKeyProvider key_provider{"keeper of the seven keys"}; - MockBaseVirtualMachine vm{"mock-vm"}; + NiceMock vm{"mock-vm"}; }; TEST_F(BaseVM, get_all_ipv4_works_when_ssh_throws_opening_a_session) @@ -309,10 +309,10 @@ TEST_F(BaseVM, startsWithNoSnapshots) TEST_F(BaseVM, takesSnapshots) { - auto snapshot = std::make_shared(); - EXPECT_CALL(*snapshot, capture).Times(AnyNumber()); + auto snapshot = std::make_shared>(); + EXPECT_CALL(*snapshot, capture).Times(1); - EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillRepeatedly(Return(snapshot)); + EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillOnce(Return(snapshot)); vm.take_snapshot(mp::VMSpecs{}, "s1", ""); EXPECT_EQ(vm.get_num_snapshots(), 1); @@ -320,11 +320,10 @@ TEST_F(BaseVM, takesSnapshots) TEST_F(BaseVM, deletesSnapshots) { - auto snapshot = std::make_shared(); - EXPECT_CALL(*snapshot, capture).Times(AnyNumber()); - EXPECT_CALL(*snapshot, erase).Times(AnyNumber()); + auto snapshot = std::make_shared>(); + EXPECT_CALL(*snapshot, erase).Times(1); - EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillRepeatedly(Return(snapshot)); + EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillOnce(Return(snapshot)); vm.take_snapshot(mp::VMSpecs{}, "s1", ""); vm.delete_snapshot("s1"); @@ -336,10 +335,8 @@ TEST_F(BaseVM, countsCurrentSnapshots) const mp::VMSpecs specs{}; EXPECT_EQ(vm.get_num_snapshots(), 0); - auto snapshot = std::make_shared(); + auto snapshot = std::make_shared>(); EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillRepeatedly(Return(snapshot)); - EXPECT_CALL(*snapshot, capture).Times(AnyNumber()); - EXPECT_CALL(*snapshot, erase).Times(AnyNumber()); vm.take_snapshot(specs, "s1", ""); EXPECT_EQ(vm.get_num_snapshots(), 1); @@ -364,10 +361,8 @@ TEST_F(BaseVM, countsTotalSnapshots) const mp::VMSpecs specs{}; EXPECT_EQ(vm.get_num_snapshots(), 0); - auto snapshot = std::make_shared(); + auto snapshot = std::make_shared>(); EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillRepeatedly(Return(snapshot)); - EXPECT_CALL(*snapshot, capture).Times(AnyNumber()); - EXPECT_CALL(*snapshot, erase).Times(AnyNumber()); vm.take_snapshot(specs, "s1", ""); vm.take_snapshot(specs, "s2", ""); @@ -393,9 +388,7 @@ TEST_F(BaseVM, providesSnapshotsView) { const mp::VMSpecs specs{}; EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillRepeatedly([this](auto&&...) { - auto ret = std::make_shared(); - EXPECT_CALL(*ret, capture).Times(AnyNumber()); - EXPECT_CALL(*ret, erase).Times(AnyNumber()); + auto ret = std::make_shared>(); EXPECT_CALL(*ret, get_index).WillRepeatedly(Return(vm.get_snapshot_count() + 1)); return ret; @@ -427,10 +420,8 @@ TEST_F(BaseVM, providesSnapshotsView) TEST_F(BaseVM, providesSnapshotsByName) { EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillRepeatedly(WithArg<0>([](const std::string& name) { - auto ret = std::make_shared(); + auto ret = std::make_shared>(); EXPECT_CALL(*ret, get_name).WillRepeatedly(Return(name)); - EXPECT_CALL(*ret, capture).Times(1); - return ret; })); From 43e2f27464fcd47936df2c6f9d573ff338b8e455 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Thu, 7 Dec 2023 22:49:29 +0000 Subject: [PATCH 059/139] [tests] Check BaseVM provides snapshot by index --- tests/test_base_virtual_machine.cpp | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 6a3895ea31..e7f8f1d86c 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -417,6 +417,27 @@ TEST_F(BaseVM, providesSnapshotsView) EXPECT_THAT(snapshot_indices, UnorderedElementsAre(2, 5, 6, 8)); } +TEST_F(BaseVM, providesSnapshotsByIndex) +{ + EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillRepeatedly([this](auto&&...) { + auto ret = std::make_shared>(); + EXPECT_CALL(*ret, get_index).WillRepeatedly(Return(vm.get_snapshot_count() + 1)); + + return ret; + }); + + const mp::VMSpecs specs{}; + vm.take_snapshot(specs, "foo", ""); + vm.take_snapshot(specs, "bar", "this and that"); + vm.delete_snapshot("foo"); + vm.take_snapshot(specs, "baz", "this and that"); + + for (const auto i : {2, 3}) + { + EXPECT_THAT(vm.get_snapshot(i), Pointee(Property(&mp::Snapshot::get_index, Eq(i)))); + } +} + TEST_F(BaseVM, providesSnapshotsByName) { EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillRepeatedly(WithArg<0>([](const std::string& name) { From 51bea0694f6f85ed9032a60dfe8a28f17f56fe6d Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Tue, 12 Dec 2023 20:53:48 +0000 Subject: [PATCH 060/139] [tests] Fix mock method signature --- tests/mock_virtual_machine.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/mock_virtual_machine.h b/tests/mock_virtual_machine.h index a0c2f1bf8b..bda3ffac52 100644 --- a/tests/mock_virtual_machine.h +++ b/tests/mock_virtual_machine.h @@ -78,7 +78,7 @@ struct MockVirtualMachineT : public T make_native_mount_handler, (const SSHKeyProvider*, const std::string&, const VMMount&), (override)); - MOCK_METHOD(VirtualMachine::SnapshotVista, view_snapshots, (), (const, override, noexcept)); + MOCK_METHOD(VirtualMachine::SnapshotVista, view_snapshots, (), (const, override)); MOCK_METHOD(int, get_num_snapshots, (), (const, override)); MOCK_METHOD(std::shared_ptr, get_snapshot, (const std::string&), (const, override)); MOCK_METHOD(std::shared_ptr, get_snapshot, (int index), (const, override)); From 73fbae5ce58837a415aab4c2bbc2710fdfd4abd2 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Thu, 14 Dec 2023 18:46:43 +0000 Subject: [PATCH 061/139] [tests] Verify handling of missing snapshot index --- tests/test_base_virtual_machine.cpp | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index e7f8f1d86c..fccc63843a 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -458,4 +458,30 @@ TEST_F(BaseVM, providesSnapshotsByName) EXPECT_THAT(vm.get_snapshot(target_name), Pointee(Property(&mp::Snapshot::get_name, Eq(target_name)))); } +TEST_F(BaseVM, handlesMissingSnapshotByIndex) +{ + EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillRepeatedly([this](auto&&...) { + auto ret = std::make_shared>(); + EXPECT_CALL(*ret, get_index).WillRepeatedly(Return(vm.get_snapshot_count() + 1)); + + return ret; + }); + + auto expect_throw = [this](int i) { + MP_EXPECT_THROW_THAT(vm.get_snapshot(i), + std::runtime_error, + mpt::match_what(AllOf(HasSubstr(vm.vm_name), HasSubstr(std::to_string(i))))); + }; + + for (int i = -2; i < 4; ++i) + expect_throw(i); + + const mp::VMSpecs specs{}; + vm.take_snapshot(specs, "foo", "I know kung fu"); + vm.take_snapshot(specs, "bar", "blue pill"); + vm.take_snapshot(specs, "baz", "red pill"); + + for (int i : {-2, -1, 0, 4, 5}) + expect_throw(i); +} } // namespace From 6ae9773cced764e7b386dd044215da02a74b504c Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Thu, 14 Dec 2023 18:50:36 +0000 Subject: [PATCH 062/139] [tests] Refactor mocking for indexed snapshotting --- tests/test_base_virtual_machine.cpp | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index fccc63843a..b8109f2162 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -204,6 +204,16 @@ struct StubBaseVirtualMachine : public mp::BaseVirtualMachine struct BaseVM : public Test { + void mock_indexed_snapshotting() + { + EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillRepeatedly([this](auto&&...) { + auto ret = std::make_shared>(); + EXPECT_CALL(*ret, get_index).WillRepeatedly(Return(vm.get_snapshot_count() + 1)); + + return ret; + }); + } + mpt::MockSSHTestFixture mock_ssh_test_fixture; const mpt::DummyKeyProvider key_provider{"keeper of the seven keys"}; NiceMock vm{"mock-vm"}; @@ -419,12 +429,7 @@ TEST_F(BaseVM, providesSnapshotsView) TEST_F(BaseVM, providesSnapshotsByIndex) { - EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillRepeatedly([this](auto&&...) { - auto ret = std::make_shared>(); - EXPECT_CALL(*ret, get_index).WillRepeatedly(Return(vm.get_snapshot_count() + 1)); - - return ret; - }); + mock_indexed_snapshotting(); const mp::VMSpecs specs{}; vm.take_snapshot(specs, "foo", ""); @@ -460,12 +465,7 @@ TEST_F(BaseVM, providesSnapshotsByName) TEST_F(BaseVM, handlesMissingSnapshotByIndex) { - EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillRepeatedly([this](auto&&...) { - auto ret = std::make_shared>(); - EXPECT_CALL(*ret, get_index).WillRepeatedly(Return(vm.get_snapshot_count() + 1)); - - return ret; - }); + mock_indexed_snapshotting(); auto expect_throw = [this](int i) { MP_EXPECT_THROW_THAT(vm.get_snapshot(i), From 12cbd6fed57ae0e786d422dec4bba45334457a85 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Thu, 14 Dec 2023 19:04:45 +0000 Subject: [PATCH 063/139] [tests] Verify handling of missing snapshot names --- tests/test_base_virtual_machine.cpp | 44 +++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 6 deletions(-) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index b8109f2162..1c69fc54af 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -19,6 +19,7 @@ #include "dummy_ssh_key_provider.h" #include "mock_ssh_test_fixture.h" #include "mock_virtual_machine.h" +#include "multipass/exceptions/snapshot_exceptions.h" #include "temp_dir.h" #include @@ -214,6 +215,16 @@ struct BaseVM : public Test }); } + void mock_named_snapshotting() + { + EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillRepeatedly(WithArg<0>([](const std::string& name) { + auto ret = std::make_shared>(); + EXPECT_CALL(*ret, get_name).WillRepeatedly(Return(name)); + + return ret; + })); + } + mpt::MockSSHTestFixture mock_ssh_test_fixture; const mpt::DummyKeyProvider key_provider{"keeper of the seven keys"}; NiceMock vm{"mock-vm"}; @@ -445,11 +456,7 @@ TEST_F(BaseVM, providesSnapshotsByIndex) TEST_F(BaseVM, providesSnapshotsByName) { - EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillRepeatedly(WithArg<0>([](const std::string& name) { - auto ret = std::make_shared>(); - EXPECT_CALL(*ret, get_name).WillRepeatedly(Return(name)); - return ret; - })); + mock_named_snapshotting(); const mp::VMSpecs specs{}; const std::string target_name = "pick"; @@ -481,7 +488,32 @@ TEST_F(BaseVM, handlesMissingSnapshotByIndex) vm.take_snapshot(specs, "bar", "blue pill"); vm.take_snapshot(specs, "baz", "red pill"); - for (int i : {-2, -1, 0, 4, 5}) + for (int i : {-2, -1, 0, 4, 5, 100}) expect_throw(i); } + +TEST_F(BaseVM, handlesMissingSnapshotByName) +{ + mock_named_snapshotting(); + + auto expect_throws = [this]() { + std::array missing_names = {"neo", "morpheus", "trinity"}; + for (const auto& name : missing_names) + { + MP_EXPECT_THROW_THAT(vm.get_snapshot(name), + mp::NoSuchSnapshotException, + mpt::match_what(AllOf(HasSubstr(vm.vm_name), HasSubstr(name)))); + } + }; + + expect_throws(); + + const mp::VMSpecs specs{}; + vm.take_snapshot(specs, "smith", ""); + vm.take_snapshot(specs, "johnson", ""); + vm.take_snapshot(specs, "jones", ""); + + expect_throws(); +} + } // namespace From 27c7db993e0f515774422a7d8e41ba2c9dead3df Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Thu, 14 Dec 2023 19:10:37 +0000 Subject: [PATCH 064/139] [tests] Slight renaming --- tests/test_base_virtual_machine.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 1c69fc54af..40c30c04b8 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -470,7 +470,7 @@ TEST_F(BaseVM, providesSnapshotsByName) EXPECT_THAT(vm.get_snapshot(target_name), Pointee(Property(&mp::Snapshot::get_name, Eq(target_name)))); } -TEST_F(BaseVM, handlesMissingSnapshotByIndex) +TEST_F(BaseVM, throwsOnMissingSnapshotByIndex) { mock_indexed_snapshotting(); @@ -492,7 +492,7 @@ TEST_F(BaseVM, handlesMissingSnapshotByIndex) expect_throw(i); } -TEST_F(BaseVM, handlesMissingSnapshotByName) +TEST_F(BaseVM, throwsOnMissingSnapshotByName) { mock_named_snapshotting(); From 37d3a4214dadf6d5348f396987b99dd79ac15261 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Thu, 14 Dec 2023 19:17:28 +0000 Subject: [PATCH 065/139] [tests] Verify repeated snapshot names are refused --- tests/test_base_virtual_machine.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 40c30c04b8..2b5d1c124f 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -516,4 +516,22 @@ TEST_F(BaseVM, throwsOnMissingSnapshotByName) expect_throws(); } +TEST_F(BaseVM, throwsOnRepeatedSnapshotName) +{ + mock_named_snapshotting(); + + const mp::VMSpecs specs{}; + auto repeated_given_name = "asdf"; + auto repeated_derived_name = "snapshot3"; + vm.take_snapshot(specs, repeated_given_name, ""); + vm.take_snapshot(specs, repeated_derived_name, ""); + + MP_ASSERT_THROW_THAT(vm.take_snapshot(specs, repeated_given_name, ""), + mp::SnapshotNameTakenException, + mpt::match_what(HasSubstr(repeated_given_name))); + MP_EXPECT_THROW_THAT(vm.take_snapshot(specs, "", ""), // this would be the third snapshot + mp::SnapshotNameTakenException, + mpt::match_what(HasSubstr(repeated_derived_name))); +} + } // namespace From fb5fcf2f95688c4cbcd82e27adada3de3867bc46 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Thu, 14 Dec 2023 23:56:07 +0000 Subject: [PATCH 066/139] [tests] Verify snapshot deletion updates parents --- tests/test_base_virtual_machine.cpp | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 2b5d1c124f..b1ef8d29f1 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -225,6 +225,18 @@ struct BaseVM : public Test })); } + void mock_parented_named_snapshot() + { + EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)) + .WillRepeatedly(WithArgs<0, 3>([this](const std::string& name, std::shared_ptr parent) { + auto ret = std::make_shared>(); + EXPECT_CALL(*ret, get_parent()).WillRepeatedly(Return(parent)); + EXPECT_CALL(*ret, get_name).WillRepeatedly(Return(name)); + + return ret; + })); + } + mpt::MockSSHTestFixture mock_ssh_test_fixture; const mpt::DummyKeyProvider key_provider{"keeper of the seven keys"}; NiceMock vm{"mock-vm"}; @@ -534,4 +546,18 @@ TEST_F(BaseVM, throwsOnRepeatedSnapshotName) mpt::match_what(HasSubstr(repeated_derived_name))); } +TEST_F(BaseVM, snapshotDeletionUpdatesParents) +{ + mock_parented_named_snapshot(); + + const mp::VMSpecs specs{}; + auto snapshot1 = vm.take_snapshot(specs, "", ""); + auto snapshot2 = vm.take_snapshot(specs, "", ""); + auto snapshot3 = vm.take_snapshot(specs, "", ""); + auto& snapshot3_mock = dynamic_cast&>(*std::const_pointer_cast(snapshot3)); + + EXPECT_CALL(snapshot3_mock, set_parent(Eq(snapshot1))).Times(1); + vm.delete_snapshot(snapshot2->get_name()); +} + } // namespace From f743a6d1d1dcdba2711f81be64cd3489e3de1ec2 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Mon, 18 Dec 2023 19:06:43 +0000 Subject: [PATCH 067/139] [tests] Avoid unnecessary casts Modify test to save mock snapshots, rather than casting taken snapshots. --- tests/test_base_virtual_machine.cpp | 33 +++++++++++++++++------------ 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index b1ef8d29f1..8246ebc41a 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -225,16 +225,19 @@ struct BaseVM : public Test })); } - void mock_parented_named_snapshot() + void mock_parented_named_snapshots(std::vector>& snapshot_album) { EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)) - .WillRepeatedly(WithArgs<0, 3>([this](const std::string& name, std::shared_ptr parent) { - auto ret = std::make_shared>(); - EXPECT_CALL(*ret, get_parent()).WillRepeatedly(Return(parent)); - EXPECT_CALL(*ret, get_name).WillRepeatedly(Return(name)); + .WillRepeatedly( + WithArgs<0, 3>([this, &snapshot_album](const std::string& name, std::shared_ptr parent) { + auto ret = std::make_shared>(); + EXPECT_CALL(*ret, get_parent()).WillRepeatedly(Return(parent)); + EXPECT_CALL(*ret, get_name).WillRepeatedly(Return(name)); - return ret; - })); + snapshot_album.push_back(ret); + + return ret; + })); } mpt::MockSSHTestFixture mock_ssh_test_fixture; @@ -548,16 +551,18 @@ TEST_F(BaseVM, throwsOnRepeatedSnapshotName) TEST_F(BaseVM, snapshotDeletionUpdatesParents) { - mock_parented_named_snapshot(); + std::vector> snapshot_album{}; + mock_parented_named_snapshots(snapshot_album); const mp::VMSpecs specs{}; - auto snapshot1 = vm.take_snapshot(specs, "", ""); - auto snapshot2 = vm.take_snapshot(specs, "", ""); - auto snapshot3 = vm.take_snapshot(specs, "", ""); - auto& snapshot3_mock = dynamic_cast&>(*std::const_pointer_cast(snapshot3)); + vm.take_snapshot(specs, "", ""); + vm.take_snapshot(specs, "", ""); + vm.take_snapshot(specs, "", ""); + + ASSERT_EQ(snapshot_album.size(), 3); - EXPECT_CALL(snapshot3_mock, set_parent(Eq(snapshot1))).Times(1); - vm.delete_snapshot(snapshot2->get_name()); + EXPECT_CALL(*snapshot_album[2], set_parent(Eq(snapshot_album[0]))).Times(1); + vm.delete_snapshot(snapshot_album[1]->get_name()); } } // namespace From 45ce49cafb05bdc40a77335480126c694470d99f Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Mon, 18 Dec 2023 23:56:09 +0000 Subject: [PATCH 068/139] [tests] Avoid repeated calls --- tests/test_base_virtual_machine.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 8246ebc41a..741bc71bb2 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -554,12 +554,12 @@ TEST_F(BaseVM, snapshotDeletionUpdatesParents) std::vector> snapshot_album{}; mock_parented_named_snapshots(snapshot_album); + const auto num_snapshots = 3; const mp::VMSpecs specs{}; - vm.take_snapshot(specs, "", ""); - vm.take_snapshot(specs, "", ""); - vm.take_snapshot(specs, "", ""); + for (int i = 0; i < num_snapshots; ++i) + vm.take_snapshot(specs, "", ""); - ASSERT_EQ(snapshot_album.size(), 3); + ASSERT_EQ(snapshot_album.size(), num_snapshots); EXPECT_CALL(*snapshot_album[2], set_parent(Eq(snapshot_album[0]))).Times(1); vm.delete_snapshot(snapshot_album[1]->get_name()); From eeb6b06f44cc7955d917b62b47314e859f7064df Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Tue, 19 Dec 2023 00:39:45 +0000 Subject: [PATCH 069/139] [tests] Check that VM can get snapshots' children --- tests/test_base_virtual_machine.cpp | 52 +++++++++++++++++++++++------ 1 file changed, 42 insertions(+), 10 deletions(-) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 741bc71bb2..56867ecc0d 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -215,26 +215,31 @@ struct BaseVM : public Test }); } - void mock_named_snapshotting() + void mock_named_snapshotting(std::vector>* snapshot_album = nullptr) { - EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillRepeatedly(WithArg<0>([](const std::string& name) { - auto ret = std::make_shared>(); - EXPECT_CALL(*ret, get_name).WillRepeatedly(Return(name)); + EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)) + .WillRepeatedly(WithArg<0>([snapshot_album](const std::string& name) { + auto ret = std::make_shared>(); + EXPECT_CALL(*ret, get_name).WillRepeatedly(Return(name)); - return ret; - })); + if (snapshot_album) + snapshot_album->push_back(ret); + + return ret; + })); } - void mock_parented_named_snapshots(std::vector>& snapshot_album) + void mock_parented_named_snapshotting(std::vector>* snapshot_album = nullptr) { EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)) .WillRepeatedly( - WithArgs<0, 3>([this, &snapshot_album](const std::string& name, std::shared_ptr parent) { + WithArgs<0, 3>([this, snapshot_album](const std::string& name, std::shared_ptr parent) { auto ret = std::make_shared>(); EXPECT_CALL(*ret, get_parent()).WillRepeatedly(Return(parent)); EXPECT_CALL(*ret, get_name).WillRepeatedly(Return(name)); - snapshot_album.push_back(ret); + if (snapshot_album) + snapshot_album->push_back(ret); return ret; })); @@ -552,7 +557,7 @@ TEST_F(BaseVM, throwsOnRepeatedSnapshotName) TEST_F(BaseVM, snapshotDeletionUpdatesParents) { std::vector> snapshot_album{}; - mock_parented_named_snapshots(snapshot_album); + mock_parented_named_snapshotting(&snapshot_album); const auto num_snapshots = 3; const mp::VMSpecs specs{}; @@ -565,4 +570,31 @@ TEST_F(BaseVM, snapshotDeletionUpdatesParents) vm.delete_snapshot(snapshot_album[1]->get_name()); } +TEST_F(BaseVM, providesChildrenNames) +{ + std::vector> snapshot_album{}; + mock_named_snapshotting(&snapshot_album); + + const auto name_template = "s{}"; + const auto num_snapshots = 5; + const mp::VMSpecs specs{}; + for (int i = 0; i < num_snapshots; ++i) + vm.take_snapshot(specs, fmt::format(name_template, i), ""); + + ASSERT_EQ(snapshot_album.size(), num_snapshots); + + std::vector expected_children_names{}; + for (int i = 1; i < num_snapshots; ++i) + { + EXPECT_CALL(Const(*snapshot_album[i]), get_parent()).WillRepeatedly(Return(snapshot_album[0])); + expected_children_names.push_back(fmt::format(name_template, i)); + } + + EXPECT_THAT(vm.get_childrens_names(snapshot_album[0].get()), UnorderedElementsAreArray(expected_children_names)); + + for (int i = 1; i < num_snapshots; ++i) + { + EXPECT_THAT(vm.get_childrens_names(snapshot_album[i].get()), IsEmpty()); + } +} } // namespace From 58f5a1281a0d81527dcd630b6a253d246859cd30 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Tue, 19 Dec 2023 11:47:53 +0000 Subject: [PATCH 070/139] [tests] Move "snapshot album" to test-case class --- tests/test_base_virtual_machine.cpp | 41 +++++++++++++---------------- 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 56867ecc0d..94b12e762d 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -215,39 +215,36 @@ struct BaseVM : public Test }); } - void mock_named_snapshotting(std::vector>* snapshot_album = nullptr) + void mock_named_snapshotting() { - EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)) - .WillRepeatedly(WithArg<0>([snapshot_album](const std::string& name) { - auto ret = std::make_shared>(); - EXPECT_CALL(*ret, get_name).WillRepeatedly(Return(name)); + EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillRepeatedly(WithArg<0>([this](const std::string& name) { + auto ret = std::make_shared>(); + EXPECT_CALL(*ret, get_name).WillRepeatedly(Return(name)); - if (snapshot_album) - snapshot_album->push_back(ret); + snapshot_album.push_back(ret); - return ret; - })); + return ret; + })); } - void mock_parented_named_snapshotting(std::vector>* snapshot_album = nullptr) + void mock_parented_named_snapshotting() { EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)) - .WillRepeatedly( - WithArgs<0, 3>([this, snapshot_album](const std::string& name, std::shared_ptr parent) { - auto ret = std::make_shared>(); - EXPECT_CALL(*ret, get_parent()).WillRepeatedly(Return(parent)); - EXPECT_CALL(*ret, get_name).WillRepeatedly(Return(name)); + .WillRepeatedly(WithArgs<0, 3>([this](const std::string& name, std::shared_ptr parent) { + auto ret = std::make_shared>(); + EXPECT_CALL(*ret, get_parent()).WillRepeatedly(Return(parent)); + EXPECT_CALL(*ret, get_name).WillRepeatedly(Return(name)); - if (snapshot_album) - snapshot_album->push_back(ret); + snapshot_album.push_back(ret); - return ret; - })); + return ret; + })); } mpt::MockSSHTestFixture mock_ssh_test_fixture; const mpt::DummyKeyProvider key_provider{"keeper of the seven keys"}; NiceMock vm{"mock-vm"}; + std::vector> snapshot_album; }; TEST_F(BaseVM, get_all_ipv4_works_when_ssh_throws_opening_a_session) @@ -556,8 +553,7 @@ TEST_F(BaseVM, throwsOnRepeatedSnapshotName) TEST_F(BaseVM, snapshotDeletionUpdatesParents) { - std::vector> snapshot_album{}; - mock_parented_named_snapshotting(&snapshot_album); + mock_parented_named_snapshotting(); const auto num_snapshots = 3; const mp::VMSpecs specs{}; @@ -572,8 +568,7 @@ TEST_F(BaseVM, snapshotDeletionUpdatesParents) TEST_F(BaseVM, providesChildrenNames) { - std::vector> snapshot_album{}; - mock_named_snapshotting(&snapshot_album); + mock_named_snapshotting(); const auto name_template = "s{}"; const auto num_snapshots = 5; From df524da16b84cb432f027e6ff0c27749fc34e2b6 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 5 Jan 2024 22:49:46 +0000 Subject: [PATCH 071/139] [tests] Check renaming of snapshots by VM --- tests/test_base_virtual_machine.cpp | 74 +++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 94b12e762d..9c6264a4be 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -592,4 +592,78 @@ TEST_F(BaseVM, providesChildrenNames) EXPECT_THAT(vm.get_childrens_names(snapshot_album[i].get()), IsEmpty()); } } + +TEST_F(BaseVM, renamesSnapshot) +{ + const std::string old_name = "initial"; + const std::string new_name = "renamed"; + std::string current_name = old_name; + + auto snapshot = std::make_shared>(); + EXPECT_CALL(*snapshot, get_name()).WillRepeatedly(ReturnPointee(¤t_name)); + EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillOnce(Return(snapshot)); + + vm.take_snapshot({}, old_name, "as ;lklkh afa"); + + EXPECT_CALL(*snapshot, set_name(Eq(new_name))).WillOnce(Assign(¤t_name, new_name)); + vm.rename_snapshot(old_name, new_name); + + EXPECT_EQ(vm.get_snapshot(new_name), snapshot); +} + +TEST_F(BaseVM, skipsSnapshotRenamingWithIdenticalName) +{ + mock_named_snapshotting(); + + const auto* name = "fixed"; + vm.take_snapshot({}, name, "not changing"); + + ASSERT_EQ(snapshot_album.size(), 1); + EXPECT_CALL(*snapshot_album[0], set_name).Times(0); + + EXPECT_NO_THROW(vm.rename_snapshot(name, name)); + EXPECT_EQ(vm.get_snapshot(name), snapshot_album[0]); +} + +TEST_F(BaseVM, throwsOnRequestToRenameMissingSnapshot) +{ + mock_named_snapshotting(); + + const auto* good_name = "Mafalda"; + const auto* missing_name = "Gui"; + vm.take_snapshot({}, good_name, ""); + + ASSERT_EQ(snapshot_album.size(), 1); + EXPECT_CALL(*snapshot_album[0], set_name).Times(0); + + MP_EXPECT_THROW_THAT(vm.rename_snapshot(missing_name, "Filipe"), + mp::NoSuchSnapshotException, + mpt::match_what(AllOf(HasSubstr(vm.vm_name), HasSubstr(missing_name)))); + + EXPECT_EQ(vm.get_snapshot(good_name), snapshot_album[0]); +} + +TEST_F(BaseVM, throwsOnRequestToRenameSnapshotWithRepeatedName) +{ + mock_named_snapshotting(); + + const auto names = std::array{"Mafalda", "Gui"}; + + mp::VMSpecs specs{}; + vm.take_snapshot(specs, names[0], ""); + vm.take_snapshot(specs, names[1], ""); + + ASSERT_EQ(snapshot_album.size(), 2); + EXPECT_CALL(*snapshot_album[0], set_name).Times(0); + + MP_EXPECT_THROW_THAT(vm.rename_snapshot(names[0], names[1]), + mp::SnapshotNameTakenException, + mpt::match_what(AllOf(HasSubstr(vm.vm_name), HasSubstr(names[1])))); + MP_EXPECT_THROW_THAT(vm.rename_snapshot(names[1], names[0]), + mp::SnapshotNameTakenException, + mpt::match_what(AllOf(HasSubstr(vm.vm_name), HasSubstr(names[0])))); + + EXPECT_EQ(vm.get_snapshot(names[0]), snapshot_album[0]); + EXPECT_EQ(vm.get_snapshot(names[1]), snapshot_album[1]); +} } // namespace From 7c690615853c279199e28ebedbb0149a340c3d02 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 22 Dec 2023 16:03:38 +0000 Subject: [PATCH 072/139] [tests] Check deletion of missing snapshot throws --- tests/test_base_virtual_machine.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 9c6264a4be..e871131962 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -566,6 +566,14 @@ TEST_F(BaseVM, snapshotDeletionUpdatesParents) vm.delete_snapshot(snapshot_album[1]->get_name()); } +TEST_F(BaseVM, snapshotDeletionThrowsOnMissingSnapshot) +{ + const auto name = "missing"; + MP_EXPECT_THROW_THAT(vm.delete_snapshot(name), + mp::NoSuchSnapshotException, + mpt::match_what(AllOf(HasSubstr(vm.vm_name), HasSubstr(name)))); +} + TEST_F(BaseVM, providesChildrenNames) { mock_named_snapshotting(); From 2095fbbbae586a4ac5ecaf8c71a458a15626f897 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 22 Dec 2023 16:37:36 +0000 Subject: [PATCH 073/139] [tests] Confirm logging of head snapshot --- tests/test_base_virtual_machine.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index e871131962..917937dd6d 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -17,9 +17,11 @@ #include "common.h" #include "dummy_ssh_key_provider.h" +#include "mock_logger.h" #include "mock_ssh_test_fixture.h" #include "mock_virtual_machine.h" #include "multipass/exceptions/snapshot_exceptions.h" +#include "multipass/logging/level.h" #include "temp_dir.h" #include @@ -487,6 +489,17 @@ TEST_F(BaseVM, providesSnapshotsByName) EXPECT_THAT(vm.get_snapshot(target_name), Pointee(Property(&mp::Snapshot::get_name, Eq(target_name)))); } +TEST_F(BaseVM, logsSnapshotHead) +{ + mock_named_snapshotting(); + const auto name = "asdf"; + + auto logger_scope = mpt::MockLogger::inject(mpl::Level::debug); + logger_scope.mock_logger->expect_log(mpl::Level::debug, name); + + vm.take_snapshot({}, name, ""); +} + TEST_F(BaseVM, throwsOnMissingSnapshotByIndex) { mock_indexed_snapshotting(); From b23321a5aac7c1829853947e7e834163d39c7d63 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 5 Jan 2024 19:06:17 +0000 Subject: [PATCH 074/139] [tests] Check that BaseVM restores snapshots --- tests/test_base_virtual_machine.cpp | 49 +++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 917937dd6d..2159de25da 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -687,4 +687,53 @@ TEST_F(BaseVM, throwsOnRequestToRenameSnapshotWithRepeatedName) EXPECT_EQ(vm.get_snapshot(names[0]), snapshot_album[0]); EXPECT_EQ(vm.get_snapshot(names[1]), snapshot_album[1]); } + +TEST_F(BaseVM, restoresSnapshots) +{ + mock_named_snapshotting(); + + mp::VMMount mount{"src", {}, {}, mp::VMMount::MountType::Classic}; + + QJsonObject metadata{}; + metadata["meta"] = "data"; + + const mp::VMSpecs original_specs{2, + mp::MemorySize{"3.5G"}, + mp::MemorySize{"15G"}, + "12:12:12:12:12:12", + {}, + "user", + mp::VirtualMachine::State::off, + {{"dst", mount}}, + false, + metadata, + {}}; + + const auto* snapshot_name = "shoot"; + vm.take_snapshot(original_specs, snapshot_name, ""); + + ASSERT_EQ(snapshot_album.size(), 1); + auto& snapshot = *snapshot_album[0]; + + mp::VMSpecs changed_specs = original_specs; + changed_specs.num_cores = 3; + changed_specs.mem_size = mp::MemorySize{"5G"}; + changed_specs.disk_space = mp::MemorySize{"35G"}; + changed_specs.state = mp::VirtualMachine::State::stopped; + changed_specs.mounts.clear(); + changed_specs.metadata["data"] = "meta"; + changed_specs.metadata["meta"] = "toto"; + + EXPECT_CALL(snapshot, apply); + EXPECT_CALL(snapshot, get_state).WillRepeatedly(Return(original_specs.state)); + EXPECT_CALL(snapshot, get_num_cores).WillRepeatedly(Return(original_specs.num_cores)); + EXPECT_CALL(snapshot, get_mem_size).WillRepeatedly(Return(original_specs.mem_size)); + EXPECT_CALL(snapshot, get_disk_space).WillRepeatedly(Return(original_specs.disk_space)); + EXPECT_CALL(snapshot, get_mounts).WillRepeatedly(ReturnRef(original_specs.mounts)); + EXPECT_CALL(snapshot, get_metadata).WillRepeatedly(ReturnRef(original_specs.metadata)); + + vm.restore_snapshot(snapshot_name, changed_specs); + + EXPECT_EQ(original_specs, changed_specs); +} } // namespace From 8d4169ae7d3d744f594e30b45ef5bd47f50fd140 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 22 Dec 2023 21:05:51 +0000 Subject: [PATCH 075/139] [tests] Check restored snapshot used as parent --- tests/test_base_virtual_machine.cpp | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 2159de25da..f949d69a40 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -235,6 +235,7 @@ struct BaseVM : public Test .WillRepeatedly(WithArgs<0, 3>([this](const std::string& name, std::shared_ptr parent) { auto ret = std::make_shared>(); EXPECT_CALL(*ret, get_parent()).WillRepeatedly(Return(parent)); + EXPECT_CALL(Const(*ret), get_parent()).WillRepeatedly(Return(parent)); EXPECT_CALL(*ret, get_name).WillRepeatedly(Return(name)); snapshot_album.push_back(ret); @@ -736,4 +737,28 @@ TEST_F(BaseVM, restoresSnapshots) EXPECT_EQ(original_specs, changed_specs); } + +TEST_F(BaseVM, usesRestoredSnapshotAsParentForNewSnapshots) +{ + mock_parented_named_snapshotting(); + + mp::VMSpecs specs{}; + const std::string root_name{"first"}; + vm.take_snapshot(specs, root_name, ""); + auto root_snapshot = snapshot_album[0]; + + ASSERT_EQ(snapshot_album.size(), 1); + EXPECT_EQ(vm.take_snapshot(specs, "second", "")->get_parent().get(), root_snapshot.get()); + ASSERT_EQ(snapshot_album.size(), 2); + EXPECT_EQ(vm.take_snapshot(specs, "third", "")->get_parent().get(), snapshot_album[1].get()); + + std::unordered_map mounts; + EXPECT_CALL(*root_snapshot, get_mounts).WillRepeatedly(ReturnRef(mounts)); + + QJsonObject metadata{}; + EXPECT_CALL(*root_snapshot, get_metadata).WillRepeatedly(ReturnRef(metadata)); + + vm.restore_snapshot(root_name, specs); + EXPECT_EQ(vm.take_snapshot(specs, "fourth", "")->get_parent().get(), root_snapshot.get()); +} } // namespace From bd3a917aa125b63e279dc61cef54ba1df12d4dfb Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 22 Dec 2023 21:47:15 +0000 Subject: [PATCH 076/139] [tests] Fix dangling reference in stub VM --- tests/test_base_virtual_machine.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index f949d69a40..39b6d5112e 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -202,7 +202,7 @@ struct StubBaseVirtualMachine : public mp::BaseVirtualMachine return nullptr; } - std::unique_ptr&& tmp_dir; + std::unique_ptr tmp_dir; }; struct BaseVM : public Test From b1bbb34b39ecebaeaac5f30eec338b37906d6ce1 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 22 Dec 2023 21:48:03 +0000 Subject: [PATCH 077/139] [tests] Take unique_ptrs by value in stub VMs --- tests/stub_virtual_machine.h | 2 +- tests/stub_virtual_machine_factory.h | 2 +- tests/test_base_virtual_machine.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/stub_virtual_machine.h b/tests/stub_virtual_machine.h index 4c316b06b7..9a159135e5 100644 --- a/tests/stub_virtual_machine.h +++ b/tests/stub_virtual_machine.h @@ -38,7 +38,7 @@ struct StubVirtualMachine final : public multipass::VirtualMachine { } - StubVirtualMachine(const std::string& name, std::unique_ptr&& tmp_dir) + StubVirtualMachine(const std::string& name, std::unique_ptr tmp_dir) : VirtualMachine{name, tmp_dir->path()}, tmp_dir{std::move(tmp_dir)} { } diff --git a/tests/stub_virtual_machine_factory.h b/tests/stub_virtual_machine_factory.h index 93305b094d..756cf5e47e 100644 --- a/tests/stub_virtual_machine_factory.h +++ b/tests/stub_virtual_machine_factory.h @@ -34,8 +34,8 @@ struct StubVirtualMachineFactory : public multipass::BaseVirtualMachineFactory { } - StubVirtualMachineFactory(std::unique_ptr&& tmp_dir) : mp::BaseVirtualMachineFactory{tmp_dir->path()}, tmp_dir{std::move(tmp_dir)} + StubVirtualMachineFactory(std::unique_ptr tmp_dir) { } diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 39b6d5112e..75127ca836 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -109,7 +109,7 @@ struct StubBaseVirtualMachine : public mp::BaseVirtualMachine { } - StubBaseVirtualMachine(mp::VirtualMachine::State s, std::unique_ptr&& tmp_dir) + StubBaseVirtualMachine(mp::VirtualMachine::State s, std::unique_ptr tmp_dir) : mp::BaseVirtualMachine{s, "stub", tmp_dir->path()}, tmp_dir{std::move(tmp_dir)} { } From b3e735cff82fbd1abe6e96128d519cd45872b596 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 22 Dec 2023 21:48:34 +0000 Subject: [PATCH 078/139] [tests] Fix using mp-namespace shortcut in header --- tests/stub_virtual_machine_factory.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/stub_virtual_machine_factory.h b/tests/stub_virtual_machine_factory.h index 756cf5e47e..b4d3063ecd 100644 --- a/tests/stub_virtual_machine_factory.h +++ b/tests/stub_virtual_machine_factory.h @@ -34,8 +34,8 @@ struct StubVirtualMachineFactory : public multipass::BaseVirtualMachineFactory { } - : mp::BaseVirtualMachineFactory{tmp_dir->path()}, tmp_dir{std::move(tmp_dir)} StubVirtualMachineFactory(std::unique_ptr tmp_dir) + : multipass::BaseVirtualMachineFactory{tmp_dir->path()}, tmp_dir{std::move(tmp_dir)} { } From f3f9f79ffec254380c5354ac23bcc551b15eb8cd Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 22 Dec 2023 21:58:20 +0000 Subject: [PATCH 079/139] [tests] Check BaseVM refuses to take snapshot Check that BaseVM refuses to take snapshot, unless functions to make specific snapshots are overridden. (Tests still failing.) --- tests/test_base_virtual_machine.cpp | 31 +++++++++++++++++++---------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 75127ca836..ac6dbfaf11 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -17,6 +17,7 @@ #include "common.h" #include "dummy_ssh_key_provider.h" +#include "file_operations.h" #include "mock_logger.h" #include "mock_ssh_test_fixture.h" #include "mock_virtual_machine.h" @@ -188,18 +189,8 @@ struct StubBaseVirtualMachine : public mp::BaseVirtualMachine { } -protected: - std::shared_ptr make_specific_snapshot(const std::string& /*snapshot_name*/, - const std::string& /*comment*/, - const mp::VMSpecs& /*specs*/, - std::shared_ptr /*parent*/) override + void require_snapshots_support() const override // pretend we support it here { - return nullptr; - } - - virtual std::shared_ptr make_specific_snapshot(const QString& /*json*/) override - { - return nullptr; } std::unique_ptr tmp_dir; @@ -359,6 +350,14 @@ TEST_F(BaseVM, takesSnapshots) EXPECT_EQ(vm.get_num_snapshots(), 1); } +TEST_F(BaseVM, takeSnasphotthrowsIfSpecificSnapshotNotOverridden) +{ + StubBaseVirtualMachine stub{}; + MP_EXPECT_THROW_THAT(stub.take_snapshot({}, "stub-snap", ""), + mp::NotImplementedOnThisBackendException, + mpt::match_what(HasSubstr("snapshots"))); +} + TEST_F(BaseVM, deletesSnapshots) { auto snapshot = std::make_shared>(); @@ -761,4 +760,14 @@ TEST_F(BaseVM, usesRestoredSnapshotAsParentForNewSnapshots) vm.restore_snapshot(root_name, specs); EXPECT_EQ(vm.take_snapshot(specs, "fourth", "")->get_parent().get(), root_snapshot.get()); } + +TEST_F(BaseVM, loadSnasphotthrowsIfSpecificSnapshotNotOverridden) +{ + StubBaseVirtualMachine stub{}; + mpt::make_file_with_content(stub.tmp_dir->filePath("0001.snapshot.json"), "whatever-content"); + MP_EXPECT_THROW_THAT(stub.load_snapshots(), + mp::NotImplementedOnThisBackendException, + mpt::match_what(HasSubstr("snapshots"))); +} + } // namespace From 55cba854c7c4cfcd20c01e05f7c4b97dae498cc7 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 22 Dec 2023 21:59:15 +0000 Subject: [PATCH 080/139] [vm] Use smallcase "snapshots" feature name (Fulfills remaining BaseVM tests.) --- src/platform/backends/shared/base_virtual_machine.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/platform/backends/shared/base_virtual_machine.cpp b/src/platform/backends/shared/base_virtual_machine.cpp index 89006143fc..323218c980 100644 --- a/src/platform/backends/shared/base_virtual_machine.cpp +++ b/src/platform/backends/shared/base_virtual_machine.cpp @@ -548,12 +548,12 @@ std::shared_ptr BaseVirtualMachine::make_specific_snapshot(const std:: const VMSpecs& /*specs*/, std::shared_ptr /*parent*/) { - throw NotImplementedOnThisBackendException{"Snapshots"}; + throw NotImplementedOnThisBackendException{"snapshots"}; } std::shared_ptr BaseVirtualMachine::make_specific_snapshot(const QString& /*filename*/) { - throw NotImplementedOnThisBackendException{"Snapshots"}; + throw NotImplementedOnThisBackendException{"snapshots"}; } } // namespace multipass From 7be366c2b1e213df953dbbdc3af6979e84728bab Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 3 Jan 2024 17:45:39 +0000 Subject: [PATCH 081/139] [tests] Fix a couple of test names --- tests/test_base_virtual_machine.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index ac6dbfaf11..f6fc43dba7 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -350,7 +350,7 @@ TEST_F(BaseVM, takesSnapshots) EXPECT_EQ(vm.get_num_snapshots(), 1); } -TEST_F(BaseVM, takeSnasphotthrowsIfSpecificSnapshotNotOverridden) +TEST_F(BaseVM, takeSnasphotThrowsIfSpecificSnapshotNotOverridden) { StubBaseVirtualMachine stub{}; MP_EXPECT_THROW_THAT(stub.take_snapshot({}, "stub-snap", ""), @@ -761,7 +761,7 @@ TEST_F(BaseVM, usesRestoredSnapshotAsParentForNewSnapshots) EXPECT_EQ(vm.take_snapshot(specs, "fourth", "")->get_parent().get(), root_snapshot.get()); } -TEST_F(BaseVM, loadSnasphotthrowsIfSpecificSnapshotNotOverridden) +TEST_F(BaseVM, loadSnasphotThrowsIfSnapshotsNotImplemented) { StubBaseVirtualMachine stub{}; mpt::make_file_with_content(stub.tmp_dir->filePath("0001.snapshot.json"), "whatever-content"); From 7324ceb2559181bba6074da7fc02e478fdeb3610 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 3 Jan 2024 17:56:09 +0000 Subject: [PATCH 082/139] [tests] Check generated snapshot names --- tests/test_base_virtual_machine.cpp | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index f6fc43dba7..fb860fe99c 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -220,6 +220,19 @@ struct BaseVM : public Test })); } + void mock_indexed_named_snapshotting() // TODO@ricab should I have only one of these? + { + EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillRepeatedly(WithArg<0>([this](const std::string& name) { + auto ret = std::make_shared>(); + EXPECT_CALL(*ret, get_name).WillRepeatedly(Return(name)); + EXPECT_CALL(*ret, get_index).WillRepeatedly(Return(vm.get_snapshot_count() + 1)); + + snapshot_album.push_back(ret); + + return ret; + })); + } + void mock_parented_named_snapshotting() { EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)) @@ -500,6 +513,18 @@ TEST_F(BaseVM, logsSnapshotHead) vm.take_snapshot({}, name, ""); } +TEST_F(BaseVM, generatesSnapshotNameFromTotalCount) +{ + mock_indexed_named_snapshotting(); + + mp::VMSpecs specs{}; + for (int i = 1; i <= 5; ++i) + { + vm.take_snapshot(specs, "", ""); + EXPECT_EQ(vm.get_snapshot(i)->get_name(), fmt::format("snapshot{}", i)); + } +} + TEST_F(BaseVM, throwsOnMissingSnapshotByIndex) { mock_indexed_snapshotting(); From 899599974d47cb23f401dcf75a5ac368947de66c Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 3 Jan 2024 22:41:04 +0000 Subject: [PATCH 083/139] [tests] Check loading of snapshot-count --- tests/test_base_virtual_machine.cpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index fb860fe99c..61fef51131 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -795,4 +795,23 @@ TEST_F(BaseVM, loadSnasphotThrowsIfSnapshotsNotImplemented) mpt::match_what(HasSubstr("snapshots"))); } +TEST_F(BaseVM, loadsAndUsesTotalSnapshotCount) +{ + mock_indexed_named_snapshotting(); + + int initial_count = 42; + mpt::make_file_with_content(vm.tmp_dir->filePath("snapshot-count"), std::to_string(initial_count)); + + EXPECT_NO_THROW(vm.load_snapshots()); + + mp::VMSpecs specs{}; + for (int i = 1; i <= 5; ++i) + { + int expected_idx = initial_count + i; + vm.take_snapshot(specs, "", ""); + EXPECT_EQ(vm.get_snapshot(expected_idx)->get_name(), fmt::format("snapshot{}", expected_idx)); + } + +} // TODO@ricab test surrounding spaces + } // namespace From d51b2ef6a41508b6e20e721ee562f14318fbd0c6 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 3 Jan 2024 18:40:05 +0000 Subject: [PATCH 084/139] [tests] Verify loading of padded counts --- tests/test_base_virtual_machine.cpp | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 61fef51131..2a2bb6601c 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -795,12 +795,25 @@ TEST_F(BaseVM, loadSnasphotThrowsIfSnapshotsNotImplemented) mpt::match_what(HasSubstr("snapshots"))); } -TEST_F(BaseVM, loadsAndUsesTotalSnapshotCount) +using SpacePadding = std::tuple; +struct TestLoadingOfPaddedGenericSnapshotInfo : public BaseVM, WithParamInterface +{ +}; + +TEST_P(TestLoadingOfPaddedGenericSnapshotInfo, loadsAndUsesTotalSnapshotCount) { mock_indexed_named_snapshotting(); int initial_count = 42; - mpt::make_file_with_content(vm.tmp_dir->filePath("snapshot-count"), std::to_string(initial_count)); + const auto& [padding_left, padding_right] = GetParam(); + + for (const auto* item : {&padding_left, &padding_right}) + { + ASSERT_THAT(*item, MatchesRegex("\\s*")); + } + + auto count_text = fmt::format("{}{}{}", padding_left, initial_count, padding_right); + mpt::make_file_with_content(vm.tmp_dir->filePath("snapshot-count"), count_text); EXPECT_NO_THROW(vm.load_snapshots()); @@ -811,7 +824,11 @@ TEST_F(BaseVM, loadsAndUsesTotalSnapshotCount) vm.take_snapshot(specs, "", ""); EXPECT_EQ(vm.get_snapshot(expected_idx)->get_name(), fmt::format("snapshot{}", expected_idx)); } +} -} // TODO@ricab test surrounding spaces +std::vector space_paddings = {"", " ", " ", "\n", " \n", "\n\n\n", "\t", "\t\t\t", "\t \n \t "}; +INSTANTIATE_TEST_SUITE_P(BaseVM, + TestLoadingOfPaddedGenericSnapshotInfo, + Combine(ValuesIn(space_paddings), ValuesIn(space_paddings))); } // namespace From 26385880f0aca727f4396969ae1cf075bfcfade0 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 3 Jan 2024 18:52:41 +0000 Subject: [PATCH 085/139] [tests] Use a single helper to mock snapshotting --- tests/test_base_virtual_machine.cpp | 79 ++++++++--------------------- 1 file changed, 20 insertions(+), 59 deletions(-) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 2a2bb6601c..1b3d950df8 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -198,49 +198,15 @@ struct StubBaseVirtualMachine : public mp::BaseVirtualMachine struct BaseVM : public Test { - void mock_indexed_snapshotting() - { - EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillRepeatedly([this](auto&&...) { - auto ret = std::make_shared>(); - EXPECT_CALL(*ret, get_index).WillRepeatedly(Return(vm.get_snapshot_count() + 1)); - - return ret; - }); - } - - void mock_named_snapshotting() - { - EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillRepeatedly(WithArg<0>([this](const std::string& name) { - auto ret = std::make_shared>(); - EXPECT_CALL(*ret, get_name).WillRepeatedly(Return(name)); - - snapshot_album.push_back(ret); - - return ret; - })); - } - - void mock_indexed_named_snapshotting() // TODO@ricab should I have only one of these? - { - EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillRepeatedly(WithArg<0>([this](const std::string& name) { - auto ret = std::make_shared>(); - EXPECT_CALL(*ret, get_name).WillRepeatedly(Return(name)); - EXPECT_CALL(*ret, get_index).WillRepeatedly(Return(vm.get_snapshot_count() + 1)); - - snapshot_album.push_back(ret); - - return ret; - })); - } - - void mock_parented_named_snapshotting() + void mock_snapshotting() { EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)) .WillRepeatedly(WithArgs<0, 3>([this](const std::string& name, std::shared_ptr parent) { auto ret = std::make_shared>(); + EXPECT_CALL(*ret, get_name).WillRepeatedly(Return(name)); + EXPECT_CALL(*ret, get_index).WillRepeatedly(Return(vm.get_snapshot_count() + 1)); EXPECT_CALL(*ret, get_parent()).WillRepeatedly(Return(parent)); EXPECT_CALL(Const(*ret), get_parent()).WillRepeatedly(Return(parent)); - EXPECT_CALL(*ret, get_name).WillRepeatedly(Return(name)); snapshot_album.push_back(ret); @@ -439,13 +405,8 @@ TEST_F(BaseVM, countsTotalSnapshots) TEST_F(BaseVM, providesSnapshotsView) { + mock_snapshotting(); const mp::VMSpecs specs{}; - EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillRepeatedly([this](auto&&...) { - auto ret = std::make_shared>(); - EXPECT_CALL(*ret, get_index).WillRepeatedly(Return(vm.get_snapshot_count() + 1)); - - return ret; - }); auto sname = [](int i) { return fmt::format("s{}", i); }; for (int i = 1; i < 6; ++i) // +5 @@ -472,9 +433,9 @@ TEST_F(BaseVM, providesSnapshotsView) TEST_F(BaseVM, providesSnapshotsByIndex) { - mock_indexed_snapshotting(); - + mock_snapshotting(); const mp::VMSpecs specs{}; + vm.take_snapshot(specs, "foo", ""); vm.take_snapshot(specs, "bar", "this and that"); vm.delete_snapshot("foo"); @@ -488,7 +449,7 @@ TEST_F(BaseVM, providesSnapshotsByIndex) TEST_F(BaseVM, providesSnapshotsByName) { - mock_named_snapshotting(); + mock_snapshotting(); const mp::VMSpecs specs{}; const std::string target_name = "pick"; @@ -504,7 +465,7 @@ TEST_F(BaseVM, providesSnapshotsByName) TEST_F(BaseVM, logsSnapshotHead) { - mock_named_snapshotting(); + mock_snapshotting(); const auto name = "asdf"; auto logger_scope = mpt::MockLogger::inject(mpl::Level::debug); @@ -515,7 +476,7 @@ TEST_F(BaseVM, logsSnapshotHead) TEST_F(BaseVM, generatesSnapshotNameFromTotalCount) { - mock_indexed_named_snapshotting(); + mock_snapshotting(); mp::VMSpecs specs{}; for (int i = 1; i <= 5; ++i) @@ -527,7 +488,7 @@ TEST_F(BaseVM, generatesSnapshotNameFromTotalCount) TEST_F(BaseVM, throwsOnMissingSnapshotByIndex) { - mock_indexed_snapshotting(); + mock_snapshotting(); auto expect_throw = [this](int i) { MP_EXPECT_THROW_THAT(vm.get_snapshot(i), @@ -549,7 +510,7 @@ TEST_F(BaseVM, throwsOnMissingSnapshotByIndex) TEST_F(BaseVM, throwsOnMissingSnapshotByName) { - mock_named_snapshotting(); + mock_snapshotting(); auto expect_throws = [this]() { std::array missing_names = {"neo", "morpheus", "trinity"}; @@ -573,7 +534,7 @@ TEST_F(BaseVM, throwsOnMissingSnapshotByName) TEST_F(BaseVM, throwsOnRepeatedSnapshotName) { - mock_named_snapshotting(); + mock_snapshotting(); const mp::VMSpecs specs{}; auto repeated_given_name = "asdf"; @@ -591,7 +552,7 @@ TEST_F(BaseVM, throwsOnRepeatedSnapshotName) TEST_F(BaseVM, snapshotDeletionUpdatesParents) { - mock_parented_named_snapshotting(); + mock_snapshotting(); const auto num_snapshots = 3; const mp::VMSpecs specs{}; @@ -614,7 +575,7 @@ TEST_F(BaseVM, snapshotDeletionThrowsOnMissingSnapshot) TEST_F(BaseVM, providesChildrenNames) { - mock_named_snapshotting(); + mock_snapshotting(); const auto name_template = "s{}"; const auto num_snapshots = 5; @@ -659,7 +620,7 @@ TEST_F(BaseVM, renamesSnapshot) TEST_F(BaseVM, skipsSnapshotRenamingWithIdenticalName) { - mock_named_snapshotting(); + mock_snapshotting(); const auto* name = "fixed"; vm.take_snapshot({}, name, "not changing"); @@ -673,7 +634,7 @@ TEST_F(BaseVM, skipsSnapshotRenamingWithIdenticalName) TEST_F(BaseVM, throwsOnRequestToRenameMissingSnapshot) { - mock_named_snapshotting(); + mock_snapshotting(); const auto* good_name = "Mafalda"; const auto* missing_name = "Gui"; @@ -691,7 +652,7 @@ TEST_F(BaseVM, throwsOnRequestToRenameMissingSnapshot) TEST_F(BaseVM, throwsOnRequestToRenameSnapshotWithRepeatedName) { - mock_named_snapshotting(); + mock_snapshotting(); const auto names = std::array{"Mafalda", "Gui"}; @@ -715,7 +676,7 @@ TEST_F(BaseVM, throwsOnRequestToRenameSnapshotWithRepeatedName) TEST_F(BaseVM, restoresSnapshots) { - mock_named_snapshotting(); + mock_snapshotting(); mp::VMMount mount{"src", {}, {}, mp::VMMount::MountType::Classic}; @@ -764,7 +725,7 @@ TEST_F(BaseVM, restoresSnapshots) TEST_F(BaseVM, usesRestoredSnapshotAsParentForNewSnapshots) { - mock_parented_named_snapshotting(); + mock_snapshotting(); mp::VMSpecs specs{}; const std::string root_name{"first"}; @@ -802,7 +763,7 @@ struct TestLoadingOfPaddedGenericSnapshotInfo : public BaseVM, WithParamInterfac TEST_P(TestLoadingOfPaddedGenericSnapshotInfo, loadsAndUsesTotalSnapshotCount) { - mock_indexed_named_snapshotting(); + mock_snapshotting(); int initial_count = 42; const auto& [padding_left, padding_right] = GetParam(); From d08698ca8541f39b4bf5e2e6229f6b9a8954099c Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 3 Jan 2024 20:33:28 +0000 Subject: [PATCH 086/139] [tests] Extract verification of test params --- tests/test_base_virtual_machine.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 1b3d950df8..30083fd76e 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -759,6 +759,15 @@ TEST_F(BaseVM, loadSnasphotThrowsIfSnapshotsNotImplemented) using SpacePadding = std::tuple; struct TestLoadingOfPaddedGenericSnapshotInfo : public BaseVM, WithParamInterface { + void SetUp() override + { + static const auto space_matcher = MatchesRegex("\\s*"); + ASSERT_THAT(padding_left, space_matcher); + ASSERT_THAT(padding_right, space_matcher); + } + + const std::string& padding_left = std::get<0>(GetParam()); + const std::string& padding_right = std::get<1>(GetParam()); }; TEST_P(TestLoadingOfPaddedGenericSnapshotInfo, loadsAndUsesTotalSnapshotCount) @@ -766,13 +775,6 @@ TEST_P(TestLoadingOfPaddedGenericSnapshotInfo, loadsAndUsesTotalSnapshotCount) mock_snapshotting(); int initial_count = 42; - const auto& [padding_left, padding_right] = GetParam(); - - for (const auto* item : {&padding_left, &padding_right}) - { - ASSERT_THAT(*item, MatchesRegex("\\s*")); - } - auto count_text = fmt::format("{}{}{}", padding_left, initial_count, padding_right); mpt::make_file_with_content(vm.tmp_dir->filePath("snapshot-count"), count_text); From 40b1972cee95d071a63a5356749217cabb22a856 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 3 Jan 2024 22:39:42 +0000 Subject: [PATCH 087/139] [tests] Verify loading of head index --- tests/test_base_virtual_machine.cpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 30083fd76e..d037b4c8e2 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -789,6 +789,25 @@ TEST_P(TestLoadingOfPaddedGenericSnapshotInfo, loadsAndUsesTotalSnapshotCount) } } +TEST_P(TestLoadingOfPaddedGenericSnapshotInfo, loadsAndUsesSnapshotHeadIndex) +{ + mock_snapshotting(); + + int head_index = 13; + auto snapshot = std::make_shared>(); + EXPECT_CALL(vm, get_snapshot(head_index)).WillOnce(Return(snapshot)); + + auto head_text = fmt::format("{}{}{}", padding_left, head_index, padding_right); + mpt::make_file_with_content(vm.tmp_dir->filePath("snapshot-head"), head_text); + mpt::make_file_with_content(vm.tmp_dir->filePath("snapshot-count"), "31"); + + EXPECT_NO_THROW(vm.load_snapshots()); + + auto name = "julius"; + vm.take_snapshot({}, name, ""); + EXPECT_EQ(vm.get_snapshot(name)->get_parent().get(), snapshot.get()); +} + std::vector space_paddings = {"", " ", " ", "\n", " \n", "\n\n\n", "\t", "\t\t\t", "\t \n \t "}; INSTANTIATE_TEST_SUITE_P(BaseVM, TestLoadingOfPaddedGenericSnapshotInfo, From f21fd9bd2963f66bb2a120a1a5d73b8e98f5249e Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Thu, 4 Jan 2024 19:54:05 +0000 Subject: [PATCH 088/139] [tests] Check loading of snapshots by VM --- tests/test_base_virtual_machine.cpp | 44 +++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index d037b4c8e2..f3d29bd233 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -32,6 +32,8 @@ #include #include +#include + namespace mp = multipass; namespace mpl = multipass::logging; namespace mpt = multipass::test; @@ -813,4 +815,46 @@ INSTANTIATE_TEST_SUITE_P(BaseVM, TestLoadingOfPaddedGenericSnapshotInfo, Combine(ValuesIn(space_paddings), ValuesIn(space_paddings))); +TEST_F(BaseVM, loadsSnasphots) +{ + static constexpr auto num_snapshots = 5; + static constexpr auto name_prefix = "blankpage"; + static constexpr auto generate_snapshot_name = [](int count) { return fmt::format("{}{}", name_prefix, count); }; + static constexpr auto index_digits_regex = +#ifdef MULTIPASS_PLATFORM_WINDOWS + R"(\d\d\d\d)"; +#else + "[[:digit:]]{4}"; +#endif + static const auto file_regex = fmt::format(R"(.*{}\.snapshot\.json)", index_digits_regex); + + auto& expectation = EXPECT_CALL(vm, make_specific_snapshot(mpt::match_qstring(MatchesRegex(file_regex)))); + + using NiceMockSnapshot = NiceMock; + std::array, num_snapshots> snapshot_bag{}; + generate(snapshot_bag.begin(), snapshot_bag.end(), [this, &expectation] { + static int idx = 1; + + const auto path = vm.tmp_dir->filePath(QString::fromStdString(fmt::format("{:04}.snapshot.json", idx))); + mpt::make_file_with_content(path, "stub"); + + auto ret = std::make_shared(); + EXPECT_CALL(*ret, get_index).WillRepeatedly(Return(idx)); + EXPECT_CALL(*ret, get_name).WillRepeatedly(Return(generate_snapshot_name(idx++))); + expectation.WillOnce(Return(ret)); + + return ret; + }); + + mpt::make_file_with_content(vm.tmp_dir->filePath("snapshot-head"), fmt::format("{}", num_snapshots)); + mpt::make_file_with_content(vm.tmp_dir->filePath("snapshot-count"), fmt::format("{}", num_snapshots)); + + EXPECT_NO_THROW(vm.load_snapshots()); + + for (int i = 0; i < num_snapshots; ++i) + { + const auto idx = i + 1; + EXPECT_EQ(vm.get_snapshot(idx)->get_name(), generate_snapshot_name(idx)); + } +} } // namespace From 1708cf9355337253f47804318b7f35915895c09c Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Thu, 4 Jan 2024 20:58:55 +0000 Subject: [PATCH 089/139] [tests] Check missing generic-snapshot-info files --- tests/test_base_virtual_machine.cpp | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index f3d29bd233..23570f5da5 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -21,6 +21,7 @@ #include "mock_logger.h" #include "mock_ssh_test_fixture.h" #include "mock_virtual_machine.h" +#include "multipass/exceptions/file_open_failed_exception.h" #include "multipass/exceptions/snapshot_exceptions.h" #include "multipass/logging/level.h" #include "temp_dir.h" @@ -857,4 +858,24 @@ TEST_F(BaseVM, loadsSnasphots) EXPECT_EQ(vm.get_snapshot(idx)->get_name(), generate_snapshot_name(idx)); } } + +TEST_F(BaseVM, throwsIfThereAreSnapshotsToLoadButNoGenericInfo) +{ + auto snapshot = std::make_shared>(); + + const auto name = "snapshot1"; + EXPECT_CALL(*snapshot, get_name).WillRepeatedly(Return(name)); + EXPECT_CALL(*snapshot, get_index).WillRepeatedly(Return(1)); + EXPECT_CALL(vm, make_specific_snapshot(_)).Times(2).WillRepeatedly(Return(snapshot)); + + mpt::make_file_with_content(vm.tmp_dir->filePath("0001.snapshot.json"), "stub"); + MP_EXPECT_THROW_THAT(vm.load_snapshots(), + mp::FileOpenFailedException, + mpt::match_what(HasSubstr("snapshot-count"))); + + vm.delete_snapshot(name); + mpt::make_file_with_content(vm.tmp_dir->filePath("snapshot-count"), "1"); + MP_EXPECT_THROW_THAT(vm.load_snapshots(), mp::FileOpenFailedException, mpt::match_what(HasSubstr("snapshot-head"))); +} + } // namespace From bceaeb82b25c9d634f40094a816316370742d4ee Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 5 Jan 2024 18:28:18 +0000 Subject: [PATCH 090/139] [tests] Fix platform-dependent regex --- tests/test_base_virtual_machine.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 23570f5da5..06d19a70e4 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -764,7 +764,13 @@ struct TestLoadingOfPaddedGenericSnapshotInfo : public BaseVM, WithParamInterfac { void SetUp() override { - static const auto space_matcher = MatchesRegex("\\s*"); + static constexpr auto space_char_class = +#ifdef MULTIPASS_PLATFORM_WINDOWS + "\\s"; +#else + "[[:space:]]"; +#endif + static const auto space_matcher = MatchesRegex(fmt::format("{}*", space_char_class)); ASSERT_THAT(padding_left, space_matcher); ASSERT_THAT(padding_right, space_matcher); } From d92cd2c33e89479eeb8b825fb0ecc73d0137b35c Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 5 Jan 2024 18:54:32 +0000 Subject: [PATCH 091/139] [tests] Check handling of repeated snapshot names Check handling of repeated snapshot names when loading snapshots. --- tests/test_base_virtual_machine.cpp | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 06d19a70e4..5be7e8a8f7 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -884,4 +884,26 @@ TEST_F(BaseVM, throwsIfThereAreSnapshotsToLoadButNoGenericInfo) MP_EXPECT_THROW_THAT(vm.load_snapshots(), mp::FileOpenFailedException, mpt::match_what(HasSubstr("snapshot-head"))); } +TEST_F(BaseVM, throwsIfLoadedSnapshotsNameIsTaken) +{ + const auto common_name = "common"; + auto snapshot1 = std::make_shared>(); + auto snapshot2 = std::make_shared>(); + + EXPECT_CALL(*snapshot1, get_name).WillRepeatedly(Return(common_name)); + EXPECT_CALL(*snapshot1, get_index).WillRepeatedly(Return(1)); + + EXPECT_CALL(*snapshot2, get_name).WillRepeatedly(Return(common_name)); + EXPECT_CALL(*snapshot2, get_index).WillRepeatedly(Return(2)); + + EXPECT_CALL(vm, make_specific_snapshot(_)).WillOnce(Return(snapshot1)).WillOnce(Return(snapshot2)); + + mpt::make_file_with_content(vm.tmp_dir->filePath("0001.snapshot.json"), "stub"); + mpt::make_file_with_content(vm.tmp_dir->filePath("0002.snapshot.json"), "stub"); + mpt::make_file_with_content(vm.tmp_dir->filePath("snapshot-head"), "1"); + mpt::make_file_with_content(vm.tmp_dir->filePath("snapshot-count"), "2"); + + MP_EXPECT_THROW_THAT(vm.load_snapshots(), mp::SnapshotNameTakenException, mpt::match_what(HasSubstr(common_name))); +} + } // namespace From 8b76be73273a22d6ba7a82790fa79a20822c7569 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 5 Jan 2024 22:06:27 +0000 Subject: [PATCH 092/139] [test] Check parent rollback on erase failure --- tests/test_base_virtual_machine.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 5be7e8a8f7..b91f001909 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -906,4 +906,22 @@ TEST_F(BaseVM, throwsIfLoadedSnapshotsNameIsTaken) MP_EXPECT_THROW_THAT(vm.load_snapshots(), mp::SnapshotNameTakenException, mpt::match_what(HasSubstr(common_name))); } +TEST_F(BaseVM, snapshotDeletionRestoresParentsOnFailure) +{ + mock_snapshotting(); + + const auto num_snapshots = 3; + const mp::VMSpecs specs{}; + for (int i = 0; i < num_snapshots; ++i) + vm.take_snapshot(specs, "", ""); + + ASSERT_EQ(snapshot_album.size(), num_snapshots); + + EXPECT_CALL(*snapshot_album[2], set_parent(Eq(snapshot_album[0]))).Times(1); + EXPECT_CALL(*snapshot_album[2], set_parent(Eq(snapshot_album[1]))).Times(1); // rollback + + EXPECT_CALL(*snapshot_album[1], erase).WillOnce(Throw(std::runtime_error{"intentional"})); + EXPECT_ANY_THROW(vm.delete_snapshot(snapshot_album[1]->get_name())); +} + } // namespace From 2f8464fabc8056e83038459786fdf0296626add8 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 5 Jan 2024 22:06:55 +0000 Subject: [PATCH 093/139] [tests] Check head rollback on deletion --- tests/test_base_virtual_machine.cpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index b91f001909..615b75d3b1 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -924,4 +924,20 @@ TEST_F(BaseVM, snapshotDeletionRestoresParentsOnFailure) EXPECT_ANY_THROW(vm.delete_snapshot(snapshot_album[1]->get_name())); } +TEST_F(BaseVM, snapshotDeletionKeepsHeadOnFailure) +{ + mock_snapshotting(); + + mp::VMSpecs specs{}; + vm.take_snapshot(specs, "", ""); + vm.take_snapshot(specs, "", ""); + + ASSERT_EQ(snapshot_album.size(), 2); + + EXPECT_CALL(*snapshot_album[1], erase).WillOnce(Throw(std::runtime_error{"intentional"})); + EXPECT_ANY_THROW(vm.delete_snapshot(snapshot_album[1]->get_name())); + + EXPECT_EQ(vm.take_snapshot(specs, "", "")->get_parent().get(), snapshot_album[1].get()); +} + } // namespace From dd0cb9f3acce95ea34ed1c786beef2de389bbcec Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 5 Jan 2024 22:22:14 +0000 Subject: [PATCH 094/139] [tests] Check null head after capture failure --- tests/test_base_virtual_machine.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 615b75d3b1..ef22018864 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -940,4 +940,17 @@ TEST_F(BaseVM, snapshotDeletionKeepsHeadOnFailure) EXPECT_EQ(vm.take_snapshot(specs, "", "")->get_parent().get(), snapshot_album[1].get()); } +TEST_F(BaseVM, takeSnapshotRevertsToNullHeadOnFirstFailure) +{ + auto snapshot = std::make_shared>(); + EXPECT_CALL(*snapshot, capture).WillOnce(Throw(std::runtime_error{"intentional"})); + EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillOnce(Return(snapshot)).RetiresOnSaturation(); + + mp::VMSpecs specs{}; + EXPECT_ANY_THROW(vm.take_snapshot(specs, "", "")); + + mock_snapshotting(); + EXPECT_EQ(vm.take_snapshot(specs, "", "")->get_parent().get(), nullptr); +} + } // namespace From 53dca774d941131b07dbf9029e56984f069fd4e6 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 5 Jan 2024 22:21:55 +0000 Subject: [PATCH 095/139] [tests] Check rollback to previous head on failure --- tests/test_base_virtual_machine.cpp | 34 +++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index ef22018864..829508a62c 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -953,4 +953,38 @@ TEST_F(BaseVM, takeSnapshotRevertsToNullHeadOnFirstFailure) EXPECT_EQ(vm.take_snapshot(specs, "", "")->get_parent().get(), nullptr); } +TEST_F(BaseVM, takeSnapshotRevertsHeadAndCount) +{ + auto early_snapshot = std::make_shared>(); + EXPECT_CALL(*early_snapshot, get_name).WillRepeatedly(Return("asdf")); + EXPECT_CALL(*early_snapshot, get_index).WillRepeatedly(Return(1)); + + EXPECT_CALL(vm, make_specific_snapshot(_)).WillOnce(Return(early_snapshot)); + + mpt::make_file_with_content(vm.tmp_dir->filePath("0001.snapshot.json"), "stub"); + mpt::make_file_with_content(vm.tmp_dir->filePath("snapshot-head"), "1"); + mpt::make_file_with_content(vm.tmp_dir->filePath("snapshot-count"), "1"); + + vm.load_snapshots(); + + constexpr auto attempted_name = "fdsa"; + auto failing_snapshot = std::make_shared>(); + + EXPECT_CALL(*failing_snapshot, get_name).WillRepeatedly(Return(attempted_name)); + EXPECT_CALL(*failing_snapshot, get_index).WillRepeatedly(Return(2)); + EXPECT_CALL(*failing_snapshot, get_parents_index) + .WillOnce(Throw(std::runtime_error{"intentional"})) // causes persisting to break, after successful capture + .RetiresOnSaturation(); + + EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillOnce(Return(failing_snapshot)).RetiresOnSaturation(); + + mp::VMSpecs specs{}; + EXPECT_ANY_THROW(vm.take_snapshot(specs, attempted_name, "")); + + mock_snapshotting(); + auto new_snapshot = vm.take_snapshot(specs, attempted_name, ""); + EXPECT_EQ(new_snapshot->get_parent().get(), early_snapshot.get()); + EXPECT_EQ(new_snapshot->get_index(), 2); // snapshot count not increased by failed snapshot +} + } // namespace From 1b100d5552f1d763c586f7a8b388c8e48fda5e3b Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 5 Jan 2024 22:46:08 +0000 Subject: [PATCH 096/139] [tests] Check handling of rename failure --- tests/test_base_virtual_machine.cpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 829508a62c..acac717003 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -987,4 +987,20 @@ TEST_F(BaseVM, takeSnapshotRevertsHeadAndCount) EXPECT_EQ(new_snapshot->get_index(), 2); // snapshot count not increased by failed snapshot } +TEST_F(BaseVM, renameFailureIsReverted) +{ + std::string current_name = "before"; + std::string attempted_name = "after"; + auto snapshot = std::make_shared>(); + EXPECT_CALL(*snapshot, get_name()).WillRepeatedly(Return(current_name)); + EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillOnce(Return(snapshot)); + + vm.take_snapshot({}, current_name, ""); + + EXPECT_CALL(*snapshot, set_name(Eq(attempted_name))).WillOnce(Throw(std::runtime_error{"intentional"})); + EXPECT_ANY_THROW(vm.rename_snapshot(current_name, attempted_name)); + + EXPECT_EQ(vm.get_snapshot(current_name).get(), snapshot.get()); +} + } // namespace From d77e0afa69db48362344fb03cc98a3f355e70853 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Mon, 8 Jan 2024 17:23:07 +0000 Subject: [PATCH 097/139] [tests] Refactor platform-dependent regex elements --- tests/test_base_virtual_machine.cpp | 36 +++++++++++++++++++---------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index acac717003..3bbdc0b5be 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -221,6 +221,29 @@ struct BaseVM : public Test const mpt::DummyKeyProvider key_provider{"keeper of the seven keys"}; NiceMock vm{"mock-vm"}; std::vector> snapshot_album; + + static constexpr auto space_char_class = +#ifdef MULTIPASS_PLATFORM_WINDOWS + "\\s"; +#else + "[[:space:]]"; +#endif + static constexpr auto digit_char_class = +#ifdef MULTIPASS_PLATFORM_WINDOWS + "\\d"; +#else + "[[:digit:]]"; +#endif + static std::string n_occurrences(const std::string& regex, int n) + { + assert(n > 0 && "need positive n"); + return +#ifdef MULTIPASS_PLATFORM_WINDOWS + fmt::to_string(fmt::join(std::vector(n, regex), "")); +#else + fmt::format("{}{{{}}}", regex, n); +#endif + } }; TEST_F(BaseVM, get_all_ipv4_works_when_ssh_throws_opening_a_session) @@ -764,12 +787,6 @@ struct TestLoadingOfPaddedGenericSnapshotInfo : public BaseVM, WithParamInterfac { void SetUp() override { - static constexpr auto space_char_class = -#ifdef MULTIPASS_PLATFORM_WINDOWS - "\\s"; -#else - "[[:space:]]"; -#endif static const auto space_matcher = MatchesRegex(fmt::format("{}*", space_char_class)); ASSERT_THAT(padding_left, space_matcher); ASSERT_THAT(padding_right, space_matcher); @@ -827,12 +844,7 @@ TEST_F(BaseVM, loadsSnasphots) static constexpr auto num_snapshots = 5; static constexpr auto name_prefix = "blankpage"; static constexpr auto generate_snapshot_name = [](int count) { return fmt::format("{}{}", name_prefix, count); }; - static constexpr auto index_digits_regex = -#ifdef MULTIPASS_PLATFORM_WINDOWS - R"(\d\d\d\d)"; -#else - "[[:digit:]]{4}"; -#endif + static const auto index_digits_regex = n_occurrences(digit_char_class, 4); static const auto file_regex = fmt::format(R"(.*{}\.snapshot\.json)", index_digits_regex); auto& expectation = EXPECT_CALL(vm, make_specific_snapshot(mpt::match_qstring(MatchesRegex(file_regex)))); From 4081c0fde62cfbfe085b0779b9842e4fd159fd47 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Mon, 8 Jan 2024 17:32:17 +0000 Subject: [PATCH 098/139] [tests] Check persistence of generic snapshot info --- tests/test_base_virtual_machine.cpp | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 3bbdc0b5be..5d966a41b4 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -1015,4 +1015,29 @@ TEST_F(BaseVM, renameFailureIsReverted) EXPECT_EQ(vm.get_snapshot(current_name).get(), snapshot.get()); } +TEST_F(BaseVM, persistsGenericSnapshotInfoWhenTakingSnapshot) +{ + mock_snapshotting(); + + ASSERT_EQ(vm.get_snapshot_count(), 0); + + const QString& head_path = vm.tmp_dir->filePath("snapshot-head"); + const QString& count_path = vm.tmp_dir->filePath("snapshot-count"); + ASSERT_FALSE(QFileInfo{head_path}.exists()); + ASSERT_FALSE(QFileInfo{count_path}.exists()); + + auto make_regex_matcher = [](int n) { return MatchesRegex(fmt::format("{0}*{1}{0}*", space_char_class, n)); }; + + mp::VMSpecs specs{}; + for (int i = 1; i < 5; ++i) + { + vm.take_snapshot(specs, "", ""); + ASSERT_TRUE(QFileInfo{head_path}.exists()); + ASSERT_TRUE(QFileInfo{count_path}.exists()); + + auto regex_matcher = make_regex_matcher(i); + EXPECT_THAT(mpt::load(head_path).toStdString(), regex_matcher); + EXPECT_THAT(mpt::load(count_path).toStdString(), regex_matcher); + } +} } // namespace From 53444183ccd6438360145d52dc86b3f6de88a063 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Mon, 8 Jan 2024 19:22:58 +0000 Subject: [PATCH 099/139] [tests] Fix include order --- tests/test_base_virtual_machine.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 5d966a41b4..a06e6396ba 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -21,14 +21,14 @@ #include "mock_logger.h" #include "mock_ssh_test_fixture.h" #include "mock_virtual_machine.h" -#include "multipass/exceptions/file_open_failed_exception.h" -#include "multipass/exceptions/snapshot_exceptions.h" -#include "multipass/logging/level.h" #include "temp_dir.h" #include +#include +#include #include +#include #include #include #include From 991fbd6dae8652641e82cd5afdef2d5f01512de7 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Mon, 8 Jan 2024 19:24:50 +0000 Subject: [PATCH 100/139] [tests] Check new info files removed on failure Check that generic snapshot info files are removed if there is a failure to take the snapshot and they were not there before. --- tests/test_base_virtual_machine.cpp | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index a06e6396ba..1316089ed2 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -20,6 +20,7 @@ #include "file_operations.h" #include "mock_logger.h" #include "mock_ssh_test_fixture.h" +#include "mock_utils.h" #include "mock_virtual_machine.h" #include "temp_dir.h" @@ -1040,4 +1041,32 @@ TEST_F(BaseVM, persistsGenericSnapshotInfoWhenTakingSnapshot) EXPECT_THAT(mpt::load(count_path).toStdString(), regex_matcher); } } + +TEST_F(BaseVM, removesGenericSnapshotInfoFilesOnFirstFailure) +{ + auto [mock_utils_ptr, guard] = mpt::MockUtils::inject(); + auto& mock_utils = *mock_utils_ptr; + mock_snapshotting(); + + const char* head_filename = "snapshot-head"; + const char* count_filename = "snapshot-count"; + const QString& head_path = vm.tmp_dir->filePath(head_filename); + const QString& count_path = vm.tmp_dir->filePath(count_filename); + + ASSERT_FALSE(QFileInfo{head_path}.exists()); + ASSERT_FALSE(QFileInfo{count_path}.exists()); + + MP_DELEGATE_MOCK_CALLS_ON_BASE_WITH_MATCHERS(mock_utils, + make_file_with_content, + mp::Utils, + (EndsWith(head_filename), _, Eq(true))); + EXPECT_CALL(mock_utils, make_file_with_content(EndsWith(head_filename), _, Eq(true))); + EXPECT_CALL(mock_utils, make_file_with_content(EndsWith(count_filename), _, Eq(true))) + .WillOnce(Throw(std::runtime_error{"intentional"})); + + EXPECT_ANY_THROW(vm.take_snapshot({}, "", "")); + + EXPECT_FALSE(QFileInfo{head_path}.exists()); + EXPECT_FALSE(QFileInfo{count_path}.exists()); +} } // namespace From 17af3e36c5070d0d4949063df5969666bcde2ebf Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Mon, 8 Jan 2024 21:52:24 +0000 Subject: [PATCH 101/139] [tests] Factor out a couple of constants --- tests/test_base_virtual_machine.cpp | 32 ++++++++++++++--------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 1316089ed2..d907d94eed 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -223,6 +223,8 @@ struct BaseVM : public Test NiceMock vm{"mock-vm"}; std::vector> snapshot_album; + static constexpr auto* head_filename = "snapshot-head"; + static constexpr auto* count_filename = "snapshot-count"; static constexpr auto space_char_class = #ifdef MULTIPASS_PLATFORM_WINDOWS "\\s"; @@ -825,8 +827,8 @@ TEST_P(TestLoadingOfPaddedGenericSnapshotInfo, loadsAndUsesSnapshotHeadIndex) EXPECT_CALL(vm, get_snapshot(head_index)).WillOnce(Return(snapshot)); auto head_text = fmt::format("{}{}{}", padding_left, head_index, padding_right); - mpt::make_file_with_content(vm.tmp_dir->filePath("snapshot-head"), head_text); - mpt::make_file_with_content(vm.tmp_dir->filePath("snapshot-count"), "31"); + mpt::make_file_with_content(vm.tmp_dir->filePath(head_filename), head_text); + mpt::make_file_with_content(vm.tmp_dir->filePath(count_filename), "31"); EXPECT_NO_THROW(vm.load_snapshots()); @@ -866,8 +868,8 @@ TEST_F(BaseVM, loadsSnasphots) return ret; }); - mpt::make_file_with_content(vm.tmp_dir->filePath("snapshot-head"), fmt::format("{}", num_snapshots)); - mpt::make_file_with_content(vm.tmp_dir->filePath("snapshot-count"), fmt::format("{}", num_snapshots)); + mpt::make_file_with_content(vm.tmp_dir->filePath(head_filename), fmt::format("{}", num_snapshots)); + mpt::make_file_with_content(vm.tmp_dir->filePath(count_filename), fmt::format("{}", num_snapshots)); EXPECT_NO_THROW(vm.load_snapshots()); @@ -888,13 +890,11 @@ TEST_F(BaseVM, throwsIfThereAreSnapshotsToLoadButNoGenericInfo) EXPECT_CALL(vm, make_specific_snapshot(_)).Times(2).WillRepeatedly(Return(snapshot)); mpt::make_file_with_content(vm.tmp_dir->filePath("0001.snapshot.json"), "stub"); - MP_EXPECT_THROW_THAT(vm.load_snapshots(), - mp::FileOpenFailedException, - mpt::match_what(HasSubstr("snapshot-count"))); + MP_EXPECT_THROW_THAT(vm.load_snapshots(), mp::FileOpenFailedException, mpt::match_what(HasSubstr(count_filename))); vm.delete_snapshot(name); - mpt::make_file_with_content(vm.tmp_dir->filePath("snapshot-count"), "1"); - MP_EXPECT_THROW_THAT(vm.load_snapshots(), mp::FileOpenFailedException, mpt::match_what(HasSubstr("snapshot-head"))); + mpt::make_file_with_content(vm.tmp_dir->filePath(count_filename), "1"); + MP_EXPECT_THROW_THAT(vm.load_snapshots(), mp::FileOpenFailedException, mpt::match_what(HasSubstr(head_filename))); } TEST_F(BaseVM, throwsIfLoadedSnapshotsNameIsTaken) @@ -913,8 +913,8 @@ TEST_F(BaseVM, throwsIfLoadedSnapshotsNameIsTaken) mpt::make_file_with_content(vm.tmp_dir->filePath("0001.snapshot.json"), "stub"); mpt::make_file_with_content(vm.tmp_dir->filePath("0002.snapshot.json"), "stub"); - mpt::make_file_with_content(vm.tmp_dir->filePath("snapshot-head"), "1"); - mpt::make_file_with_content(vm.tmp_dir->filePath("snapshot-count"), "2"); + mpt::make_file_with_content(vm.tmp_dir->filePath(head_filename), "1"); + mpt::make_file_with_content(vm.tmp_dir->filePath(count_filename), "2"); MP_EXPECT_THROW_THAT(vm.load_snapshots(), mp::SnapshotNameTakenException, mpt::match_what(HasSubstr(common_name))); } @@ -975,8 +975,8 @@ TEST_F(BaseVM, takeSnapshotRevertsHeadAndCount) EXPECT_CALL(vm, make_specific_snapshot(_)).WillOnce(Return(early_snapshot)); mpt::make_file_with_content(vm.tmp_dir->filePath("0001.snapshot.json"), "stub"); - mpt::make_file_with_content(vm.tmp_dir->filePath("snapshot-head"), "1"); - mpt::make_file_with_content(vm.tmp_dir->filePath("snapshot-count"), "1"); + mpt::make_file_with_content(vm.tmp_dir->filePath(head_filename), "1"); + mpt::make_file_with_content(vm.tmp_dir->filePath(count_filename), "1"); vm.load_snapshots(); @@ -1022,8 +1022,8 @@ TEST_F(BaseVM, persistsGenericSnapshotInfoWhenTakingSnapshot) ASSERT_EQ(vm.get_snapshot_count(), 0); - const QString& head_path = vm.tmp_dir->filePath("snapshot-head"); - const QString& count_path = vm.tmp_dir->filePath("snapshot-count"); + const QString& head_path = vm.tmp_dir->filePath(head_filename); + const QString& count_path = vm.tmp_dir->filePath(count_filename); ASSERT_FALSE(QFileInfo{head_path}.exists()); ASSERT_FALSE(QFileInfo{count_path}.exists()); @@ -1048,8 +1048,6 @@ TEST_F(BaseVM, removesGenericSnapshotInfoFilesOnFirstFailure) auto& mock_utils = *mock_utils_ptr; mock_snapshotting(); - const char* head_filename = "snapshot-head"; - const char* count_filename = "snapshot-count"; const QString& head_path = vm.tmp_dir->filePath(head_filename); const QString& count_path = vm.tmp_dir->filePath(count_filename); From c51882f0530dd7a8eabfc08bd51c0a21f0f5bc1d Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Mon, 8 Jan 2024 21:49:09 +0000 Subject: [PATCH 102/139] [tests] Reorder test-case class contents slightly --- tests/test_base_virtual_machine.cpp | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index d907d94eed..d29c3c7fe5 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -218,11 +218,21 @@ struct BaseVM : public Test })); } + static std::string n_occurrences(const std::string& regex, int n) + { + assert(n > 0 && "need positive n"); + return +#ifdef MULTIPASS_PLATFORM_WINDOWS + fmt::to_string(fmt::join(std::vector(n, regex), "")); +#else + fmt::format("{}{{{}}}", regex, n); +#endif + } + mpt::MockSSHTestFixture mock_ssh_test_fixture; const mpt::DummyKeyProvider key_provider{"keeper of the seven keys"}; NiceMock vm{"mock-vm"}; std::vector> snapshot_album; - static constexpr auto* head_filename = "snapshot-head"; static constexpr auto* count_filename = "snapshot-count"; static constexpr auto space_char_class = @@ -237,16 +247,6 @@ struct BaseVM : public Test #else "[[:digit:]]"; #endif - static std::string n_occurrences(const std::string& regex, int n) - { - assert(n > 0 && "need positive n"); - return -#ifdef MULTIPASS_PLATFORM_WINDOWS - fmt::to_string(fmt::join(std::vector(n, regex), "")); -#else - fmt::format("{}{{{}}}", regex, n); -#endif - } }; TEST_F(BaseVM, get_all_ipv4_works_when_ssh_throws_opening_a_session) From 7fd4c84ed6c573bfdcb41e081f9d9d34750f9327 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Mon, 8 Jan 2024 22:03:04 +0000 Subject: [PATCH 103/139] [tests] Factor out a couple common vars in tests --- tests/test_base_virtual_machine.cpp | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index d29c3c7fe5..3f86620cae 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -233,6 +233,8 @@ struct BaseVM : public Test const mpt::DummyKeyProvider key_provider{"keeper of the seven keys"}; NiceMock vm{"mock-vm"}; std::vector> snapshot_album; + QString head_path = vm.tmp_dir->filePath(head_filename); + QString count_path = vm.tmp_dir->filePath(count_filename); static constexpr auto* head_filename = "snapshot-head"; static constexpr auto* count_filename = "snapshot-count"; static constexpr auto space_char_class = @@ -805,7 +807,7 @@ TEST_P(TestLoadingOfPaddedGenericSnapshotInfo, loadsAndUsesTotalSnapshotCount) int initial_count = 42; auto count_text = fmt::format("{}{}{}", padding_left, initial_count, padding_right); - mpt::make_file_with_content(vm.tmp_dir->filePath("snapshot-count"), count_text); + mpt::make_file_with_content(count_path, count_text); EXPECT_NO_THROW(vm.load_snapshots()); @@ -827,8 +829,8 @@ TEST_P(TestLoadingOfPaddedGenericSnapshotInfo, loadsAndUsesSnapshotHeadIndex) EXPECT_CALL(vm, get_snapshot(head_index)).WillOnce(Return(snapshot)); auto head_text = fmt::format("{}{}{}", padding_left, head_index, padding_right); - mpt::make_file_with_content(vm.tmp_dir->filePath(head_filename), head_text); - mpt::make_file_with_content(vm.tmp_dir->filePath(count_filename), "31"); + mpt::make_file_with_content(head_path, head_text); + mpt::make_file_with_content(count_path, "31"); EXPECT_NO_THROW(vm.load_snapshots()); @@ -868,8 +870,8 @@ TEST_F(BaseVM, loadsSnasphots) return ret; }); - mpt::make_file_with_content(vm.tmp_dir->filePath(head_filename), fmt::format("{}", num_snapshots)); - mpt::make_file_with_content(vm.tmp_dir->filePath(count_filename), fmt::format("{}", num_snapshots)); + mpt::make_file_with_content(head_path, fmt::format("{}", num_snapshots)); + mpt::make_file_with_content(count_path, fmt::format("{}", num_snapshots)); EXPECT_NO_THROW(vm.load_snapshots()); @@ -893,7 +895,7 @@ TEST_F(BaseVM, throwsIfThereAreSnapshotsToLoadButNoGenericInfo) MP_EXPECT_THROW_THAT(vm.load_snapshots(), mp::FileOpenFailedException, mpt::match_what(HasSubstr(count_filename))); vm.delete_snapshot(name); - mpt::make_file_with_content(vm.tmp_dir->filePath(count_filename), "1"); + mpt::make_file_with_content(count_path, "1"); MP_EXPECT_THROW_THAT(vm.load_snapshots(), mp::FileOpenFailedException, mpt::match_what(HasSubstr(head_filename))); } @@ -913,8 +915,8 @@ TEST_F(BaseVM, throwsIfLoadedSnapshotsNameIsTaken) mpt::make_file_with_content(vm.tmp_dir->filePath("0001.snapshot.json"), "stub"); mpt::make_file_with_content(vm.tmp_dir->filePath("0002.snapshot.json"), "stub"); - mpt::make_file_with_content(vm.tmp_dir->filePath(head_filename), "1"); - mpt::make_file_with_content(vm.tmp_dir->filePath(count_filename), "2"); + mpt::make_file_with_content(head_path, "1"); + mpt::make_file_with_content(count_path, "2"); MP_EXPECT_THROW_THAT(vm.load_snapshots(), mp::SnapshotNameTakenException, mpt::match_what(HasSubstr(common_name))); } @@ -975,8 +977,8 @@ TEST_F(BaseVM, takeSnapshotRevertsHeadAndCount) EXPECT_CALL(vm, make_specific_snapshot(_)).WillOnce(Return(early_snapshot)); mpt::make_file_with_content(vm.tmp_dir->filePath("0001.snapshot.json"), "stub"); - mpt::make_file_with_content(vm.tmp_dir->filePath(head_filename), "1"); - mpt::make_file_with_content(vm.tmp_dir->filePath(count_filename), "1"); + mpt::make_file_with_content(head_path, "1"); + mpt::make_file_with_content(count_path, "1"); vm.load_snapshots(); @@ -1022,8 +1024,6 @@ TEST_F(BaseVM, persistsGenericSnapshotInfoWhenTakingSnapshot) ASSERT_EQ(vm.get_snapshot_count(), 0); - const QString& head_path = vm.tmp_dir->filePath(head_filename); - const QString& count_path = vm.tmp_dir->filePath(count_filename); ASSERT_FALSE(QFileInfo{head_path}.exists()); ASSERT_FALSE(QFileInfo{count_path}.exists()); @@ -1048,9 +1048,6 @@ TEST_F(BaseVM, removesGenericSnapshotInfoFilesOnFirstFailure) auto& mock_utils = *mock_utils_ptr; mock_snapshotting(); - const QString& head_path = vm.tmp_dir->filePath(head_filename); - const QString& count_path = vm.tmp_dir->filePath(count_filename); - ASSERT_FALSE(QFileInfo{head_path}.exists()); ASSERT_FALSE(QFileInfo{count_path}.exists()); From bdbdc8611fbe2c2cc5388e8b5dc07c6cd2977afc Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Mon, 8 Jan 2024 22:09:14 +0000 Subject: [PATCH 104/139] [tests] Further refactor common snapshot file vars --- tests/test_base_virtual_machine.cpp | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 3f86620cae..c4141ebb83 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -218,6 +218,13 @@ struct BaseVM : public Test })); } + QString get_snapshot_file_path(int idx) const + { + assert(idx > 0 && "need positive index"); + + return vm.tmp_dir->filePath(QString::fromStdString(fmt::format("{:04}.snapshot.json", idx))); + } + static std::string n_occurrences(const std::string& regex, int n) { assert(n > 0 && "need positive n"); @@ -859,8 +866,7 @@ TEST_F(BaseVM, loadsSnasphots) generate(snapshot_bag.begin(), snapshot_bag.end(), [this, &expectation] { static int idx = 1; - const auto path = vm.tmp_dir->filePath(QString::fromStdString(fmt::format("{:04}.snapshot.json", idx))); - mpt::make_file_with_content(path, "stub"); + mpt::make_file_with_content(get_snapshot_file_path(idx), "stub"); auto ret = std::make_shared(); EXPECT_CALL(*ret, get_index).WillRepeatedly(Return(idx)); @@ -891,7 +897,7 @@ TEST_F(BaseVM, throwsIfThereAreSnapshotsToLoadButNoGenericInfo) EXPECT_CALL(*snapshot, get_index).WillRepeatedly(Return(1)); EXPECT_CALL(vm, make_specific_snapshot(_)).Times(2).WillRepeatedly(Return(snapshot)); - mpt::make_file_with_content(vm.tmp_dir->filePath("0001.snapshot.json"), "stub"); + mpt::make_file_with_content(get_snapshot_file_path(1), "stub"); MP_EXPECT_THROW_THAT(vm.load_snapshots(), mp::FileOpenFailedException, mpt::match_what(HasSubstr(count_filename))); vm.delete_snapshot(name); @@ -913,8 +919,8 @@ TEST_F(BaseVM, throwsIfLoadedSnapshotsNameIsTaken) EXPECT_CALL(vm, make_specific_snapshot(_)).WillOnce(Return(snapshot1)).WillOnce(Return(snapshot2)); - mpt::make_file_with_content(vm.tmp_dir->filePath("0001.snapshot.json"), "stub"); - mpt::make_file_with_content(vm.tmp_dir->filePath("0002.snapshot.json"), "stub"); + mpt::make_file_with_content(get_snapshot_file_path(1), "stub"); + mpt::make_file_with_content(get_snapshot_file_path(2), "stub"); mpt::make_file_with_content(head_path, "1"); mpt::make_file_with_content(count_path, "2"); @@ -976,7 +982,7 @@ TEST_F(BaseVM, takeSnapshotRevertsHeadAndCount) EXPECT_CALL(vm, make_specific_snapshot(_)).WillOnce(Return(early_snapshot)); - mpt::make_file_with_content(vm.tmp_dir->filePath("0001.snapshot.json"), "stub"); + mpt::make_file_with_content(get_snapshot_file_path(1), "stub"); mpt::make_file_with_content(head_path, "1"); mpt::make_file_with_content(count_path, "1"); From 23313a32928595cf5865522337782dd30a6c9b83 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Mon, 8 Jan 2024 22:33:50 +0000 Subject: [PATCH 105/139] [tests] Check rollback of existing info files Check that snapshot info files are reverted to their previous contents if there is a failure to take a snapshot. --- tests/test_base_virtual_machine.cpp | 34 +++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index c4141ebb83..63081d8caa 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -211,6 +211,7 @@ struct BaseVM : public Test EXPECT_CALL(*ret, get_index).WillRepeatedly(Return(vm.get_snapshot_count() + 1)); EXPECT_CALL(*ret, get_parent()).WillRepeatedly(Return(parent)); EXPECT_CALL(Const(*ret), get_parent()).WillRepeatedly(Return(parent)); + EXPECT_CALL(*ret, get_parents_index).WillRepeatedly(Return(parent ? parent->get_index() : 0)); snapshot_album.push_back(ret); @@ -1070,4 +1071,37 @@ TEST_F(BaseVM, removesGenericSnapshotInfoFilesOnFirstFailure) EXPECT_FALSE(QFileInfo{head_path}.exists()); EXPECT_FALSE(QFileInfo{count_path}.exists()); } + +TEST_F(BaseVM, restoresGenericSnapshotInfoFileContents) +{ + mock_snapshotting(); + + mp::VMSpecs specs{}; + vm.take_snapshot(specs, "", ""); + + ASSERT_TRUE(QFileInfo{head_path}.exists()); + ASSERT_TRUE(QFileInfo{count_path}.exists()); + + // TODO@ricab refactor + auto make_regex_matcher = [](int n) { return MatchesRegex(fmt::format("{0}*{1}{0}*", space_char_class, n)); }; + auto regex_matcher = make_regex_matcher(1); + EXPECT_THAT(mpt::load(head_path).toStdString(), regex_matcher); + EXPECT_THAT(mpt::load(count_path).toStdString(), regex_matcher); + + auto [mock_utils_ptr, guard] = mpt::MockUtils::inject(); + auto& mock_utils = *mock_utils_ptr; + + MP_DELEGATE_MOCK_CALLS_ON_BASE_WITH_MATCHERS(mock_utils, make_file_with_content, mp::Utils, (_, _, Eq(true))); + EXPECT_CALL(mock_utils, make_file_with_content(EndsWith(head_filename), _, Eq(true))).Times(2); + EXPECT_CALL(mock_utils, make_file_with_content(EndsWith(count_filename), _, Eq(true))) + .Times(2) + .WillOnce(Throw(std::runtime_error{"intentional"})); + + EXPECT_ANY_THROW(vm.take_snapshot({}, "", "")); + + EXPECT_TRUE(QFileInfo{head_path}.exists()); + EXPECT_TRUE(QFileInfo{count_path}.exists()); + EXPECT_THAT(mpt::load(head_path).toStdString(), regex_matcher); + EXPECT_THAT(mpt::load(count_path).toStdString(), regex_matcher); +} } // namespace From c05083312f1f7d1c5c48d880fb2ccb36e38d3dc5 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Mon, 8 Jan 2024 22:44:14 +0000 Subject: [PATCH 106/139] [tests] Check head persisted on restore --- tests/test_base_virtual_machine.cpp | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 63081d8caa..aad830621b 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -1104,4 +1104,29 @@ TEST_F(BaseVM, restoresGenericSnapshotInfoFileContents) EXPECT_THAT(mpt::load(head_path).toStdString(), regex_matcher); EXPECT_THAT(mpt::load(count_path).toStdString(), regex_matcher); } + +TEST_F(BaseVM, persistsHeadIndexOnRestore) +{ + mock_snapshotting(); + + mp::VMSpecs specs{}; + const auto intended_snapshot = "this-one"; + vm.take_snapshot(specs, "foo", ""); + vm.take_snapshot(specs, intended_snapshot, ""); + vm.take_snapshot(specs, "bar", ""); + + std::unordered_map mounts; + EXPECT_CALL(*snapshot_album[1], get_mounts).WillRepeatedly(ReturnRef(mounts)); + + QJsonObject metadata{}; + EXPECT_CALL(*snapshot_album[1], get_metadata).WillRepeatedly(ReturnRef(metadata)); + + vm.restore_snapshot(intended_snapshot, specs); + EXPECT_TRUE(QFileInfo{head_path}.exists()); + + // TODO@ricab refactor + auto make_regex_matcher = [](int n) { return MatchesRegex(fmt::format("{0}*{1}{0}*", space_char_class, n)); }; + auto regex_matcher = make_regex_matcher(snapshot_album[1]->get_index()); + EXPECT_THAT(mpt::load(head_path).toStdString(), regex_matcher); +} } // namespace From acb8fcb459f9c8bea0798ce625c5b5bc24fa8c5d Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Mon, 8 Jan 2024 22:56:11 +0000 Subject: [PATCH 107/139] [tests] Refactor derivation of regex matcher --- tests/test_base_virtual_machine.cpp | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index aad830621b..443455f243 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -237,6 +237,13 @@ struct BaseVM : public Test #endif } + static auto make_index_file_contents_matcher(int idx) + { + assert(idx > 0 && "need positive index"); + + return MatchesRegex(fmt::format("{0}*{1}{0}*", space_char_class, idx)); + } + mpt::MockSSHTestFixture mock_ssh_test_fixture; const mpt::DummyKeyProvider key_provider{"keeper of the seven keys"}; NiceMock vm{"mock-vm"}; @@ -1034,8 +1041,6 @@ TEST_F(BaseVM, persistsGenericSnapshotInfoWhenTakingSnapshot) ASSERT_FALSE(QFileInfo{head_path}.exists()); ASSERT_FALSE(QFileInfo{count_path}.exists()); - auto make_regex_matcher = [](int n) { return MatchesRegex(fmt::format("{0}*{1}{0}*", space_char_class, n)); }; - mp::VMSpecs specs{}; for (int i = 1; i < 5; ++i) { @@ -1043,7 +1048,7 @@ TEST_F(BaseVM, persistsGenericSnapshotInfoWhenTakingSnapshot) ASSERT_TRUE(QFileInfo{head_path}.exists()); ASSERT_TRUE(QFileInfo{count_path}.exists()); - auto regex_matcher = make_regex_matcher(i); + auto regex_matcher = make_index_file_contents_matcher(i); EXPECT_THAT(mpt::load(head_path).toStdString(), regex_matcher); EXPECT_THAT(mpt::load(count_path).toStdString(), regex_matcher); } @@ -1082,9 +1087,7 @@ TEST_F(BaseVM, restoresGenericSnapshotInfoFileContents) ASSERT_TRUE(QFileInfo{head_path}.exists()); ASSERT_TRUE(QFileInfo{count_path}.exists()); - // TODO@ricab refactor - auto make_regex_matcher = [](int n) { return MatchesRegex(fmt::format("{0}*{1}{0}*", space_char_class, n)); }; - auto regex_matcher = make_regex_matcher(1); + auto regex_matcher = make_index_file_contents_matcher(1); EXPECT_THAT(mpt::load(head_path).toStdString(), regex_matcher); EXPECT_THAT(mpt::load(count_path).toStdString(), regex_matcher); @@ -1124,9 +1127,7 @@ TEST_F(BaseVM, persistsHeadIndexOnRestore) vm.restore_snapshot(intended_snapshot, specs); EXPECT_TRUE(QFileInfo{head_path}.exists()); - // TODO@ricab refactor - auto make_regex_matcher = [](int n) { return MatchesRegex(fmt::format("{0}*{1}{0}*", space_char_class, n)); }; - auto regex_matcher = make_regex_matcher(snapshot_album[1]->get_index()); + auto regex_matcher = make_index_file_contents_matcher(snapshot_album[1]->get_index()); EXPECT_THAT(mpt::load(head_path).toStdString(), regex_matcher); } } // namespace From 99a1e077c2ad4ddf6439f04508d57bb47c04dda0 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Mon, 8 Jan 2024 22:53:01 +0000 Subject: [PATCH 108/139] [tests] Refactor platform dependent constants --- tests/test_base_virtual_machine.cpp | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 443455f243..5721f96ea8 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -250,20 +250,17 @@ struct BaseVM : public Test std::vector> snapshot_album; QString head_path = vm.tmp_dir->filePath(head_filename); QString count_path = vm.tmp_dir->filePath(count_filename); - static constexpr auto* head_filename = "snapshot-head"; - static constexpr auto* count_filename = "snapshot-count"; - static constexpr auto space_char_class = -#ifdef MULTIPASS_PLATFORM_WINDOWS - "\\s"; -#else - "[[:space:]]"; -#endif - static constexpr auto digit_char_class = + + static constexpr bool on_windows = #ifdef MULTIPASS_PLATFORM_WINDOWS - "\\d"; + true; #else - "[[:digit:]]"; + false; #endif + static constexpr auto* head_filename = "snapshot-head"; + static constexpr auto* count_filename = "snapshot-count"; + static constexpr auto space_char_class = on_windows ? "\\s" : "[[:space:]]"; + static constexpr auto digit_char_class = on_windows ? "\\d" : "[[:digit:]]"; }; TEST_F(BaseVM, get_all_ipv4_works_when_ssh_throws_opening_a_session) From c3b6d135df19d521289201eddcd90f8e6b905819 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Tue, 9 Jan 2024 17:37:12 +0000 Subject: [PATCH 109/139] [tests] Fix gmock warning --- tests/test_base_virtual_machine.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 5721f96ea8..f2d0c79bf1 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -1094,8 +1094,8 @@ TEST_F(BaseVM, restoresGenericSnapshotInfoFileContents) MP_DELEGATE_MOCK_CALLS_ON_BASE_WITH_MATCHERS(mock_utils, make_file_with_content, mp::Utils, (_, _, Eq(true))); EXPECT_CALL(mock_utils, make_file_with_content(EndsWith(head_filename), _, Eq(true))).Times(2); EXPECT_CALL(mock_utils, make_file_with_content(EndsWith(count_filename), _, Eq(true))) - .Times(2) - .WillOnce(Throw(std::runtime_error{"intentional"})); + .WillOnce(Throw(std::runtime_error{"intentional"})) + .WillOnce(DoDefault()); EXPECT_ANY_THROW(vm.take_snapshot({}, "", "")); From 97a8e5b238a205d39122becfb9657044f8ee090c Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Tue, 9 Jan 2024 20:36:06 +0000 Subject: [PATCH 110/139] [tests] Test rollback of failed snapshot restore --- tests/test_base_virtual_machine.cpp | 56 +++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index f2d0c79bf1..6dd36dd4e7 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -1127,4 +1127,60 @@ TEST_F(BaseVM, persistsHeadIndexOnRestore) auto regex_matcher = make_index_file_contents_matcher(snapshot_album[1]->get_index()); EXPECT_THAT(mpt::load(head_path).toStdString(), regex_matcher); } + +TEST_F(BaseVM, rollsbackFailedRestore) +{ + mock_snapshotting(); + + const mp::VMSpecs original_specs{1, + mp::MemorySize{"1.5G"}, + mp::MemorySize{"4G"}, + "ab:ab:ab:ab:ab:ab", + {}, + "me", + mp::VirtualMachine::State::off, + {}, + false, + {}, + {}}; + + vm.take_snapshot(original_specs, "", ""); + + auto target_snapshot_name = "this one"; + vm.take_snapshot(original_specs, target_snapshot_name, ""); + vm.take_snapshot(original_specs, "", ""); + + ASSERT_EQ(snapshot_album.size(), 3); + auto& target_snapshot = *snapshot_album[1]; + auto& last_snapshot = *snapshot_album[2]; + + auto changed_specs = original_specs; + changed_specs.num_cores = 4; + changed_specs.mem_size = mp::MemorySize{"2G"}; + changed_specs.state = multipass::VirtualMachine::State::running; + changed_specs.mounts["dst"].source_path = "src"; + changed_specs.metadata["blah"] = "this and that"; + + EXPECT_CALL(target_snapshot, get_state).WillRepeatedly(Return(original_specs.state)); + EXPECT_CALL(target_snapshot, get_num_cores).WillRepeatedly(Return(original_specs.num_cores)); + EXPECT_CALL(target_snapshot, get_mem_size).WillRepeatedly(Return(original_specs.mem_size)); + EXPECT_CALL(target_snapshot, get_disk_space).WillRepeatedly(Return(original_specs.disk_space)); + EXPECT_CALL(target_snapshot, get_mounts).WillRepeatedly(ReturnRef(original_specs.mounts)); + EXPECT_CALL(target_snapshot, get_metadata).WillRepeatedly(ReturnRef(original_specs.metadata)); + + auto [mock_utils_ptr, guard] = mpt::MockUtils::inject(); + EXPECT_CALL(*mock_utils_ptr, make_file_with_content(_, _, _)) + .WillOnce(Throw(std::runtime_error{"intentional"})) + .WillRepeatedly(DoDefault()); + + auto current_specs = changed_specs; + EXPECT_ANY_THROW(vm.restore_snapshot(target_snapshot_name, current_specs)); + EXPECT_EQ(changed_specs, current_specs); + + auto regex_matcher = make_index_file_contents_matcher(last_snapshot.get_index()); + EXPECT_THAT(mpt::load(head_path).toStdString(), regex_matcher); + + EXPECT_EQ(vm.take_snapshot(current_specs, "", "")->get_parent().get(), &last_snapshot); +} + } // namespace From e8d1364505c40410aaacfab7fa341d7cf5e21318 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Tue, 9 Jan 2024 20:31:48 +0000 Subject: [PATCH 111/139] [tests] Add file to test QemuSnapshot --- tests/CMakeLists.txt | 1 + tests/test_qemu_snapshot.cpp | 29 +++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 tests/test_qemu_snapshot.cpp diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 96f733e42b..31e42c727e 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -89,6 +89,7 @@ add_executable(multipass_tests test_petname.cpp test_platform_shared.cpp test_private_pass_provider.cpp + test_qemu_snapshot.cpp test_qemuimg_process_spec.cpp test_remote_settings_handler.cpp test_setting_specs.cpp diff --git a/tests/test_qemu_snapshot.cpp b/tests/test_qemu_snapshot.cpp new file mode 100644 index 0000000000..dc5c80db88 --- /dev/null +++ b/tests/test_qemu_snapshot.cpp @@ -0,0 +1,29 @@ +/* + * Copyright (C) Canonical, Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 3. + * + * This program 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 . + * + */ + +#include "common.h" + +namespace mp = multipass; +namespace mpt = multipass::test; +using namespace testing; + +namespace +{ +struct TestQemuSnapshot : public Test +{ +}; +} // namespace From 8f19a06b1e480362316d8ddae2de607c2dfb7bc4 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Tue, 9 Jan 2024 19:44:55 +0000 Subject: [PATCH 112/139] [tests] Add a PublicQemuSnapshot class for testing Declare a descendant of QemuSnapshot that promotes visibility of impl methods, so that they can be easily called in tests. --- tests/test_qemu_snapshot.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/test_qemu_snapshot.cpp b/tests/test_qemu_snapshot.cpp index dc5c80db88..714f660da3 100644 --- a/tests/test_qemu_snapshot.cpp +++ b/tests/test_qemu_snapshot.cpp @@ -17,12 +17,26 @@ #include "common.h" +#include + namespace mp = multipass; namespace mpt = multipass::test; using namespace testing; namespace { + +struct PublicQemuSnapshot : public mp::QemuSnapshot +{ + // clang-format off + // (keeping original declaration order) + using mp::QemuSnapshot::QemuSnapshot; + using mp::QemuSnapshot::capture_impl; + using mp::QemuSnapshot::erase_impl; + using mp::QemuSnapshot::apply_impl; + // clang-format on +}; + struct TestQemuSnapshot : public Test { }; From afa628233f744ad6f3bfaf68cf04569659293f7b Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Tue, 9 Jan 2024 19:59:38 +0000 Subject: [PATCH 113/139] [tests] Extract MockSnapshot For reuse in other tests. --- tests/mock_snapshot.h | 56 ++++++++++++++++++++++++++++ tests/test_base_virtual_machine.cpp | 57 +++++++++-------------------- 2 files changed, 73 insertions(+), 40 deletions(-) create mode 100644 tests/mock_snapshot.h diff --git a/tests/mock_snapshot.h b/tests/mock_snapshot.h new file mode 100644 index 0000000000..a2cbc95785 --- /dev/null +++ b/tests/mock_snapshot.h @@ -0,0 +1,56 @@ +/* + * Copyright (C) Canonical, Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 3. + * + * This program 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 . + * + */ + +#ifndef MULTIPASS_MOCK_SNAPSHOT_H +#define MULTIPASS_MOCK_SNAPSHOT_H + +#include "common.h" + +#include +#include +#include + +namespace mp = multipass; + +namespace multipass::test +{ +struct MockSnapshot : public mp::Snapshot +{ + MOCK_METHOD(int, get_index, (), (const, noexcept, override)); + MOCK_METHOD(std::string, get_name, (), (const, override)); + MOCK_METHOD(std::string, get_comment, (), (const, override)); + MOCK_METHOD(QDateTime, get_creation_timestamp, (), (const, noexcept, override)); + MOCK_METHOD(int, get_num_cores, (), (const, noexcept, override)); + MOCK_METHOD(mp::MemorySize, get_mem_size, (), (const, noexcept, override)); + MOCK_METHOD(mp::MemorySize, get_disk_space, (), (const, noexcept, override)); + MOCK_METHOD(mp::VirtualMachine::State, get_state, (), (const, noexcept, override)); + MOCK_METHOD((const std::unordered_map&), get_mounts, (), (const, noexcept, override)); + MOCK_METHOD(const QJsonObject&, get_metadata, (), (const, noexcept, override)); + MOCK_METHOD(std::shared_ptr, get_parent, (), (const, override)); + MOCK_METHOD(std::shared_ptr, get_parent, (), (override)); + MOCK_METHOD(std::string, get_parents_name, (), (const, override)); + MOCK_METHOD(int, get_parents_index, (), (const, override)); + MOCK_METHOD(void, set_name, (const std::string&), (override)); + MOCK_METHOD(void, set_comment, (const std::string&), (override)); + MOCK_METHOD(void, set_parent, (std::shared_ptr), (override)); + MOCK_METHOD(void, capture, (), (override)); + MOCK_METHOD(void, erase, (), (override)); + MOCK_METHOD(void, apply, (), (override)); +}; +} // namespace multipass::test + +#endif // MULTIPASS_MOCK_SNAPSHOT_H diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 6dd36dd4e7..9ada126bbb 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -19,6 +19,7 @@ #include "dummy_ssh_key_provider.h" #include "file_operations.h" #include "mock_logger.h" +#include "mock_snapshot.h" #include "mock_ssh_test_fixture.h" #include "mock_utils.h" #include "mock_virtual_machine.h" @@ -83,30 +84,6 @@ struct MockBaseVirtualMachine : public mpt::MockVirtualMachineT&), get_mounts, (), (const, noexcept, override)); - MOCK_METHOD(const QJsonObject&, get_metadata, (), (const, noexcept, override)); - MOCK_METHOD(std::shared_ptr, get_parent, (), (const, override)); - MOCK_METHOD(std::shared_ptr, get_parent, (), (override)); - MOCK_METHOD(std::string, get_parents_name, (), (const, override)); - MOCK_METHOD(int, get_parents_index, (), (const, override)); - MOCK_METHOD(void, set_name, (const std::string&), (override)); - MOCK_METHOD(void, set_comment, (const std::string&), (override)); - MOCK_METHOD(void, set_parent, (std::shared_ptr), (override)); - MOCK_METHOD(void, capture, (), (override)); - MOCK_METHOD(void, erase, (), (override)); - MOCK_METHOD(void, apply, (), (override)); -}; - struct StubBaseVirtualMachine : public mp::BaseVirtualMachine { StubBaseVirtualMachine(mp::VirtualMachine::State s = mp::VirtualMachine::State::off) @@ -206,7 +183,7 @@ struct BaseVM : public Test { EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)) .WillRepeatedly(WithArgs<0, 3>([this](const std::string& name, std::shared_ptr parent) { - auto ret = std::make_shared>(); + auto ret = std::make_shared>(); EXPECT_CALL(*ret, get_name).WillRepeatedly(Return(name)); EXPECT_CALL(*ret, get_index).WillRepeatedly(Return(vm.get_snapshot_count() + 1)); EXPECT_CALL(*ret, get_parent()).WillRepeatedly(Return(parent)); @@ -247,7 +224,7 @@ struct BaseVM : public Test mpt::MockSSHTestFixture mock_ssh_test_fixture; const mpt::DummyKeyProvider key_provider{"keeper of the seven keys"}; NiceMock vm{"mock-vm"}; - std::vector> snapshot_album; + std::vector> snapshot_album; QString head_path = vm.tmp_dir->filePath(head_filename); QString count_path = vm.tmp_dir->filePath(count_filename); @@ -363,7 +340,7 @@ TEST_F(BaseVM, startsWithNoSnapshots) TEST_F(BaseVM, takesSnapshots) { - auto snapshot = std::make_shared>(); + auto snapshot = std::make_shared>(); EXPECT_CALL(*snapshot, capture).Times(1); EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillOnce(Return(snapshot)); @@ -382,7 +359,7 @@ TEST_F(BaseVM, takeSnasphotThrowsIfSpecificSnapshotNotOverridden) TEST_F(BaseVM, deletesSnapshots) { - auto snapshot = std::make_shared>(); + auto snapshot = std::make_shared>(); EXPECT_CALL(*snapshot, erase).Times(1); EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillOnce(Return(snapshot)); @@ -397,7 +374,7 @@ TEST_F(BaseVM, countsCurrentSnapshots) const mp::VMSpecs specs{}; EXPECT_EQ(vm.get_num_snapshots(), 0); - auto snapshot = std::make_shared>(); + auto snapshot = std::make_shared>(); EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillRepeatedly(Return(snapshot)); vm.take_snapshot(specs, "s1", ""); @@ -423,7 +400,7 @@ TEST_F(BaseVM, countsTotalSnapshots) const mp::VMSpecs specs{}; EXPECT_EQ(vm.get_num_snapshots(), 0); - auto snapshot = std::make_shared>(); + auto snapshot = std::make_shared>(); EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillRepeatedly(Return(snapshot)); vm.take_snapshot(specs, "s1", ""); @@ -649,7 +626,7 @@ TEST_F(BaseVM, renamesSnapshot) const std::string new_name = "renamed"; std::string current_name = old_name; - auto snapshot = std::make_shared>(); + auto snapshot = std::make_shared>(); EXPECT_CALL(*snapshot, get_name()).WillRepeatedly(ReturnPointee(¤t_name)); EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillOnce(Return(snapshot)); @@ -837,7 +814,7 @@ TEST_P(TestLoadingOfPaddedGenericSnapshotInfo, loadsAndUsesSnapshotHeadIndex) mock_snapshotting(); int head_index = 13; - auto snapshot = std::make_shared>(); + auto snapshot = std::make_shared>(); EXPECT_CALL(vm, get_snapshot(head_index)).WillOnce(Return(snapshot)); auto head_text = fmt::format("{}{}{}", padding_left, head_index, padding_right); @@ -866,7 +843,7 @@ TEST_F(BaseVM, loadsSnasphots) auto& expectation = EXPECT_CALL(vm, make_specific_snapshot(mpt::match_qstring(MatchesRegex(file_regex)))); - using NiceMockSnapshot = NiceMock; + using NiceMockSnapshot = NiceMock; std::array, num_snapshots> snapshot_bag{}; generate(snapshot_bag.begin(), snapshot_bag.end(), [this, &expectation] { static int idx = 1; @@ -895,7 +872,7 @@ TEST_F(BaseVM, loadsSnasphots) TEST_F(BaseVM, throwsIfThereAreSnapshotsToLoadButNoGenericInfo) { - auto snapshot = std::make_shared>(); + auto snapshot = std::make_shared>(); const auto name = "snapshot1"; EXPECT_CALL(*snapshot, get_name).WillRepeatedly(Return(name)); @@ -913,8 +890,8 @@ TEST_F(BaseVM, throwsIfThereAreSnapshotsToLoadButNoGenericInfo) TEST_F(BaseVM, throwsIfLoadedSnapshotsNameIsTaken) { const auto common_name = "common"; - auto snapshot1 = std::make_shared>(); - auto snapshot2 = std::make_shared>(); + auto snapshot1 = std::make_shared>(); + auto snapshot2 = std::make_shared>(); EXPECT_CALL(*snapshot1, get_name).WillRepeatedly(Return(common_name)); EXPECT_CALL(*snapshot1, get_index).WillRepeatedly(Return(1)); @@ -968,7 +945,7 @@ TEST_F(BaseVM, snapshotDeletionKeepsHeadOnFailure) TEST_F(BaseVM, takeSnapshotRevertsToNullHeadOnFirstFailure) { - auto snapshot = std::make_shared>(); + auto snapshot = std::make_shared>(); EXPECT_CALL(*snapshot, capture).WillOnce(Throw(std::runtime_error{"intentional"})); EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillOnce(Return(snapshot)).RetiresOnSaturation(); @@ -981,7 +958,7 @@ TEST_F(BaseVM, takeSnapshotRevertsToNullHeadOnFirstFailure) TEST_F(BaseVM, takeSnapshotRevertsHeadAndCount) { - auto early_snapshot = std::make_shared>(); + auto early_snapshot = std::make_shared>(); EXPECT_CALL(*early_snapshot, get_name).WillRepeatedly(Return("asdf")); EXPECT_CALL(*early_snapshot, get_index).WillRepeatedly(Return(1)); @@ -994,7 +971,7 @@ TEST_F(BaseVM, takeSnapshotRevertsHeadAndCount) vm.load_snapshots(); constexpr auto attempted_name = "fdsa"; - auto failing_snapshot = std::make_shared>(); + auto failing_snapshot = std::make_shared>(); EXPECT_CALL(*failing_snapshot, get_name).WillRepeatedly(Return(attempted_name)); EXPECT_CALL(*failing_snapshot, get_index).WillRepeatedly(Return(2)); @@ -1017,7 +994,7 @@ TEST_F(BaseVM, renameFailureIsReverted) { std::string current_name = "before"; std::string attempted_name = "after"; - auto snapshot = std::make_shared>(); + auto snapshot = std::make_shared>(); EXPECT_CALL(*snapshot, get_name()).WillRepeatedly(Return(current_name)); EXPECT_CALL(vm, make_specific_snapshot(_, _, _, _)).WillOnce(Return(snapshot)); From 18c6122b36f4872bc1311e6ef3f4e70f54aaf4fb Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Tue, 9 Jan 2024 20:31:31 +0000 Subject: [PATCH 114/139] [tests] Check init of base fields in QemuSnapshot --- tests/test_qemu_snapshot.cpp | 44 ++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/tests/test_qemu_snapshot.cpp b/tests/test_qemu_snapshot.cpp index 714f660da3..6b5994338e 100644 --- a/tests/test_qemu_snapshot.cpp +++ b/tests/test_qemu_snapshot.cpp @@ -16,9 +16,18 @@ */ #include "common.h" +#include "mock_snapshot.h" +#include "mock_virtual_machine.h" +#include +#include #include +#include + +#include +#include + namespace mp = multipass; namespace mpt = multipass::test; using namespace testing; @@ -40,4 +49,39 @@ struct PublicQemuSnapshot : public mp::QemuSnapshot struct TestQemuSnapshot : public Test { }; + +TEST_F(TestQemuSnapshot, initializesBaseProperties) +{ + const auto name = "name"; + const auto comment = "comment"; + const auto parent = std::make_shared(); + + const auto cpus = 3; + const auto mem_size = mp::MemorySize{"1.23G"}; + const auto disk_space = mp::MemorySize{"3.21M"}; + const auto state = mp::VirtualMachine::State::off; + const auto mounts = + std::unordered_map{{"asdf", {"fdsa", {}, {}, mp::VMMount::MountType::Classic}}}; + const auto metadata = [] { + auto metadata = QJsonObject{}; + metadata["meta"] = "data"; + return metadata; + }(); + const auto specs = mp::VMSpecs{cpus, mem_size, disk_space, "mac", {}, "", state, mounts, false, metadata, {}}; + + auto desc = mp::VirtualMachineDescription{}; + auto vm = NiceMock>{"qemu-vm"}; + + const auto snapshot = mp::QemuSnapshot{name, comment, parent, specs, vm, desc}; + EXPECT_EQ(snapshot.get_name(), name); + EXPECT_EQ(snapshot.get_comment(), comment); + EXPECT_EQ(snapshot.get_parent(), parent); + EXPECT_EQ(snapshot.get_num_cores(), cpus); + EXPECT_EQ(snapshot.get_mem_size(), mem_size); + EXPECT_EQ(snapshot.get_disk_space(), disk_space); + EXPECT_EQ(snapshot.get_state(), state); + EXPECT_EQ(snapshot.get_mounts(), mounts); + EXPECT_EQ(snapshot.get_metadata(), metadata); +} + } // namespace From 4328bbd3bd8cb0f837161560e0e38f2e269c1bea Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Tue, 9 Jan 2024 23:51:35 +0000 Subject: [PATCH 115/139] [tests] Check init of QemuSnapshot from JSON --- tests/test_qemu_snapshot.cpp | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/test_qemu_snapshot.cpp b/tests/test_qemu_snapshot.cpp index 6b5994338e..9009701191 100644 --- a/tests/test_qemu_snapshot.cpp +++ b/tests/test_qemu_snapshot.cpp @@ -18,11 +18,13 @@ #include "common.h" #include "mock_snapshot.h" #include "mock_virtual_machine.h" +#include "path.h" #include #include #include +#include #include #include @@ -48,6 +50,8 @@ struct PublicQemuSnapshot : public mp::QemuSnapshot struct TestQemuSnapshot : public Test { + mp::VirtualMachineDescription desc{}; + NiceMock> vm{"qemu-vm"}; }; TEST_F(TestQemuSnapshot, initializesBaseProperties) @@ -84,4 +88,27 @@ TEST_F(TestQemuSnapshot, initializesBaseProperties) EXPECT_EQ(snapshot.get_metadata(), metadata); } +TEST_F(TestQemuSnapshot, initializesBasePropertiesFromJson) +{ + const auto parent = std::make_shared(); + EXPECT_CALL(vm, get_snapshot(2)).WillOnce(Return(parent)); + + const mp::QemuSnapshot snapshot{mpt::test_data_path_for("test_snapshot.json"), vm, desc}; + EXPECT_EQ(snapshot.get_name(), "snapshot3"); + EXPECT_EQ(snapshot.get_comment(), "A comment"); + EXPECT_EQ(snapshot.get_parent(), parent); + EXPECT_EQ(snapshot.get_num_cores(), 1); + EXPECT_EQ(snapshot.get_mem_size(), mp::MemorySize{"1G"}); + EXPECT_EQ(snapshot.get_disk_space(), mp::MemorySize{"5G"}); + EXPECT_EQ(snapshot.get_state(), mp::VirtualMachine::State::off); + + auto mount_matcher1 = Pair(Eq("guybrush"), Field(&mp::VMMount::mount_type, mp::VMMount::MountType::Classic)); + auto mount_matcher2 = Pair(Eq("murray"), Field(&mp::VMMount::mount_type, mp::VMMount::MountType::Native)); + EXPECT_THAT(snapshot.get_mounts(), UnorderedElementsAre(mount_matcher1, mount_matcher2)); + + EXPECT_THAT( + snapshot.get_metadata(), + ResultOf([](const QJsonObject& metadata) { return metadata["arguments"].toArray(); }, Contains("-qmp"))); +} + } // namespace From 3fa389b6e1728b1ee3a60b49062456bec6c34ae9 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 10 Jan 2024 12:29:19 +0000 Subject: [PATCH 116/139] [tests] Extract vm specs for reuse --- tests/test_qemu_snapshot.cpp | 41 +++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/tests/test_qemu_snapshot.cpp b/tests/test_qemu_snapshot.cpp index 9009701191..fe91d3294b 100644 --- a/tests/test_qemu_snapshot.cpp +++ b/tests/test_qemu_snapshot.cpp @@ -52,6 +52,22 @@ struct TestQemuSnapshot : public Test { mp::VirtualMachineDescription desc{}; NiceMock> vm{"qemu-vm"}; + inline static const auto success = mp::ProcessState{0, std::nullopt}; + inline static const auto specs = [] { + const auto cpus = 3; + const auto mem_size = mp::MemorySize{"1.23G"}; + const auto disk_space = mp::MemorySize{"3.21M"}; + const auto state = mp::VirtualMachine::State::off; + const auto mounts = + std::unordered_map{{"asdf", {"fdsa", {}, {}, mp::VMMount::MountType::Classic}}}; + const auto metadata = [] { + auto metadata = QJsonObject{}; + metadata["meta"] = "data"; + return metadata; + }(); + + return mp::VMSpecs{cpus, mem_size, disk_space, "mac", {}, "", state, mounts, false, metadata, {}}; + }(); }; TEST_F(TestQemuSnapshot, initializesBaseProperties) @@ -60,19 +76,6 @@ TEST_F(TestQemuSnapshot, initializesBaseProperties) const auto comment = "comment"; const auto parent = std::make_shared(); - const auto cpus = 3; - const auto mem_size = mp::MemorySize{"1.23G"}; - const auto disk_space = mp::MemorySize{"3.21M"}; - const auto state = mp::VirtualMachine::State::off; - const auto mounts = - std::unordered_map{{"asdf", {"fdsa", {}, {}, mp::VMMount::MountType::Classic}}}; - const auto metadata = [] { - auto metadata = QJsonObject{}; - metadata["meta"] = "data"; - return metadata; - }(); - const auto specs = mp::VMSpecs{cpus, mem_size, disk_space, "mac", {}, "", state, mounts, false, metadata, {}}; - auto desc = mp::VirtualMachineDescription{}; auto vm = NiceMock>{"qemu-vm"}; @@ -80,12 +83,12 @@ TEST_F(TestQemuSnapshot, initializesBaseProperties) EXPECT_EQ(snapshot.get_name(), name); EXPECT_EQ(snapshot.get_comment(), comment); EXPECT_EQ(snapshot.get_parent(), parent); - EXPECT_EQ(snapshot.get_num_cores(), cpus); - EXPECT_EQ(snapshot.get_mem_size(), mem_size); - EXPECT_EQ(snapshot.get_disk_space(), disk_space); - EXPECT_EQ(snapshot.get_state(), state); - EXPECT_EQ(snapshot.get_mounts(), mounts); - EXPECT_EQ(snapshot.get_metadata(), metadata); + EXPECT_EQ(snapshot.get_num_cores(), specs.num_cores); + EXPECT_EQ(snapshot.get_mem_size(), specs.mem_size); + EXPECT_EQ(snapshot.get_disk_space(), specs.disk_space); + EXPECT_EQ(snapshot.get_state(), specs.state); + EXPECT_EQ(snapshot.get_mounts(), specs.mounts); + EXPECT_EQ(snapshot.get_metadata(), specs.metadata); } TEST_F(TestQemuSnapshot, initializesBasePropertiesFromJson) From e54b44c973b2c401e52c6f5b27918e9199890422 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 10 Jan 2024 21:43:55 +0000 Subject: [PATCH 117/139] [tests] Check capture of QEMU snapshots --- tests/test_qemu_snapshot.cpp | 37 ++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/test_qemu_snapshot.cpp b/tests/test_qemu_snapshot.cpp index fe91d3294b..83af6e12f0 100644 --- a/tests/test_qemu_snapshot.cpp +++ b/tests/test_qemu_snapshot.cpp @@ -16,10 +16,12 @@ */ #include "common.h" +#include "mock_process_factory.h" #include "mock_snapshot.h" #include "mock_virtual_machine.h" #include "path.h" +#include #include #include #include @@ -50,6 +52,11 @@ struct PublicQemuSnapshot : public mp::QemuSnapshot struct TestQemuSnapshot : public Test { + TestQemuSnapshot() + { + desc.image.image_path = "raniunotuiroleh"; + } + mp::VirtualMachineDescription desc{}; NiceMock> vm{"qemu-vm"}; inline static const auto success = mp::ProcessState{0, std::nullopt}; @@ -114,4 +121,34 @@ TEST_F(TestQemuSnapshot, initializesBasePropertiesFromJson) ResultOf([](const QJsonObject& metadata) { return metadata["arguments"].toArray(); }, Contains("-qmp"))); } +TEST_F(TestQemuSnapshot, capturesSnapshot) +{ + auto snapshot_index = 3; + auto snapshot_tag = fmt::format("@s{}", snapshot_index); + EXPECT_CALL(vm, get_snapshot_count).WillOnce(Return(snapshot_index - 1)); + + auto proc_count = 0; + auto list_args_matcher = ElementsAre("snapshot", "-l", desc.image.image_path); + auto capture_args_matcher = + ElementsAre("snapshot", "-c", QString::fromStdString(snapshot_tag), desc.image.image_path); + + auto mock_factory_scope = mpt::MockProcessFactory::Inject(); + mock_factory_scope->register_callback([&](mpt::MockProcess* process) { + ASSERT_LE(++proc_count, 2); + + EXPECT_EQ(process->program(), "qemu-img"); + + using ArgsMatcher = Matcher; + const auto args_matcher = proc_count == 1 ? ArgsMatcher{list_args_matcher} : ArgsMatcher{capture_args_matcher}; + EXPECT_THAT(process->arguments(), args_matcher); + + EXPECT_CALL(*process, execute).WillOnce(Return(success)); + }); + + mp::QemuSnapshot snapshot{"asdf", "", nullptr, specs, vm, desc}; + snapshot.capture(); + + EXPECT_EQ(proc_count, 2); +} + } // namespace From 657d98b811c7b1bfbb003b7f894b38d2030e5d9c Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 10 Jan 2024 22:01:40 +0000 Subject: [PATCH 118/139] [tests] Refactor bits for reuse --- tests/test_qemu_snapshot.cpp | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/tests/test_qemu_snapshot.cpp b/tests/test_qemu_snapshot.cpp index 83af6e12f0..14a8b85521 100644 --- a/tests/test_qemu_snapshot.cpp +++ b/tests/test_qemu_snapshot.cpp @@ -52,13 +52,20 @@ struct PublicQemuSnapshot : public mp::QemuSnapshot struct TestQemuSnapshot : public Test { - TestQemuSnapshot() + using ArgsMatcher = Matcher; + + mp::QemuSnapshot quick_snapshot(const std::string& name = "asdf") { - desc.image.image_path = "raniunotuiroleh"; + return mp::QemuSnapshot{name, "", nullptr, specs, vm, desc}; } - mp::VirtualMachineDescription desc{}; + mp::VirtualMachineDescription desc = [] { + mp::VirtualMachineDescription ret{}; + ret.image.image_path = "raniunotuiroleh"; + return ret; + }(); NiceMock> vm{"qemu-vm"}; + ArgsMatcher list_args_matcher = ElementsAre("snapshot", "-l", desc.image.image_path); inline static const auto success = mp::ProcessState{0, std::nullopt}; inline static const auto specs = [] { const auto cpus = 3; @@ -128,9 +135,9 @@ TEST_F(TestQemuSnapshot, capturesSnapshot) EXPECT_CALL(vm, get_snapshot_count).WillOnce(Return(snapshot_index - 1)); auto proc_count = 0; - auto list_args_matcher = ElementsAre("snapshot", "-l", desc.image.image_path); - auto capture_args_matcher = - ElementsAre("snapshot", "-c", QString::fromStdString(snapshot_tag), desc.image.image_path); + + ArgsMatcher capture_args_matcher{ + ElementsAre("snapshot", "-c", QString::fromStdString(snapshot_tag), desc.image.image_path)}; auto mock_factory_scope = mpt::MockProcessFactory::Inject(); mock_factory_scope->register_callback([&](mpt::MockProcess* process) { @@ -138,16 +145,13 @@ TEST_F(TestQemuSnapshot, capturesSnapshot) EXPECT_EQ(process->program(), "qemu-img"); - using ArgsMatcher = Matcher; - const auto args_matcher = proc_count == 1 ? ArgsMatcher{list_args_matcher} : ArgsMatcher{capture_args_matcher}; + const auto& args_matcher = proc_count == 1 ? list_args_matcher : capture_args_matcher; EXPECT_THAT(process->arguments(), args_matcher); EXPECT_CALL(*process, execute).WillOnce(Return(success)); }); - mp::QemuSnapshot snapshot{"asdf", "", nullptr, specs, vm, desc}; - snapshot.capture(); - + quick_snapshot().capture(); EXPECT_EQ(proc_count, 2); } From 3a86969dd850ec6ca8ac9fd08b4418f2c99719cd Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 10 Jan 2024 22:02:54 +0000 Subject: [PATCH 119/139] [tests] Check QemuSnapshot capture throws Verify that QemuSnapshot capture throws on repeated snapshot tag. --- tests/test_qemu_snapshot.cpp | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/test_qemu_snapshot.cpp b/tests/test_qemu_snapshot.cpp index 14a8b85521..f87aabdca2 100644 --- a/tests/test_qemu_snapshot.cpp +++ b/tests/test_qemu_snapshot.cpp @@ -155,4 +155,29 @@ TEST_F(TestQemuSnapshot, capturesSnapshot) EXPECT_EQ(proc_count, 2); } +TEST_F(TestQemuSnapshot, captureThrowsOnRepeatedTag) +{ + auto snapshot_index = 22; + auto snapshot_tag = fmt::format("@s{}", snapshot_index); + EXPECT_CALL(vm, get_snapshot_count).WillOnce(Return(snapshot_index - 1)); + + auto proc_count = 0; + auto mock_factory_scope = mpt::MockProcessFactory::Inject(); + mock_factory_scope->register_callback([&](mpt::MockProcess* process) { + ASSERT_EQ(++proc_count, 1); + + EXPECT_EQ(process->program(), "qemu-img"); + EXPECT_THAT(process->arguments(), list_args_matcher); + + EXPECT_CALL(*process, execute).WillOnce(Return(success)); + EXPECT_CALL(*process, read_all_standard_output).WillOnce(Return(QByteArray::fromStdString(snapshot_tag + ' '))); + }); + + MP_EXPECT_THROW_THAT(quick_snapshot("whatever").capture(), + std::runtime_error, + mpt::match_what(AllOf(HasSubstr("already exists"), + HasSubstr(snapshot_tag), + HasSubstr(desc.image.image_path.toStdString())))); +} + } // namespace From 99b6ff6887f9e58c3b6e4556a056a93c59c5ed09 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 10 Jan 2024 22:48:03 +0000 Subject: [PATCH 120/139] [tests] Refactor common test components --- tests/test_qemu_snapshot.cpp | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/tests/test_qemu_snapshot.cpp b/tests/test_qemu_snapshot.cpp index f87aabdca2..ca0aefebf0 100644 --- a/tests/test_qemu_snapshot.cpp +++ b/tests/test_qemu_snapshot.cpp @@ -59,13 +59,32 @@ struct TestQemuSnapshot : public Test return mp::QemuSnapshot{name, "", nullptr, specs, vm, desc}; } + template + static std::string derive_tag(T&& index) + { + return fmt::format("@s{}", std::forward(index)); + } + + static void set_common_expectations_on(mpt::MockProcess* process) + { + EXPECT_EQ(process->program(), "qemu-img"); + EXPECT_CALL(*process, execute).WillOnce(Return(success)); + } + + static void set_tag_output(mpt::MockProcess* process, std::string tag) + { + EXPECT_CALL(*process, read_all_standard_output).WillOnce(Return(QByteArray::fromStdString(tag + ' '))); + } + mp::VirtualMachineDescription desc = [] { mp::VirtualMachineDescription ret{}; ret.image.image_path = "raniunotuiroleh"; return ret; }(); + NiceMock> vm{"qemu-vm"}; ArgsMatcher list_args_matcher = ElementsAre("snapshot", "-l", desc.image.image_path); + inline static const auto success = mp::ProcessState{0, std::nullopt}; inline static const auto specs = [] { const auto cpus = 3; @@ -131,7 +150,7 @@ TEST_F(TestQemuSnapshot, initializesBasePropertiesFromJson) TEST_F(TestQemuSnapshot, capturesSnapshot) { auto snapshot_index = 3; - auto snapshot_tag = fmt::format("@s{}", snapshot_index); + auto snapshot_tag = derive_tag(snapshot_index); EXPECT_CALL(vm, get_snapshot_count).WillOnce(Return(snapshot_index - 1)); auto proc_count = 0; @@ -143,12 +162,10 @@ TEST_F(TestQemuSnapshot, capturesSnapshot) mock_factory_scope->register_callback([&](mpt::MockProcess* process) { ASSERT_LE(++proc_count, 2); - EXPECT_EQ(process->program(), "qemu-img"); + set_common_expectations_on(process); const auto& args_matcher = proc_count == 1 ? list_args_matcher : capture_args_matcher; EXPECT_THAT(process->arguments(), args_matcher); - - EXPECT_CALL(*process, execute).WillOnce(Return(success)); }); quick_snapshot().capture(); @@ -158,7 +175,7 @@ TEST_F(TestQemuSnapshot, capturesSnapshot) TEST_F(TestQemuSnapshot, captureThrowsOnRepeatedTag) { auto snapshot_index = 22; - auto snapshot_tag = fmt::format("@s{}", snapshot_index); + auto snapshot_tag = derive_tag(snapshot_index); EXPECT_CALL(vm, get_snapshot_count).WillOnce(Return(snapshot_index - 1)); auto proc_count = 0; @@ -166,11 +183,10 @@ TEST_F(TestQemuSnapshot, captureThrowsOnRepeatedTag) mock_factory_scope->register_callback([&](mpt::MockProcess* process) { ASSERT_EQ(++proc_count, 1); - EXPECT_EQ(process->program(), "qemu-img"); - EXPECT_THAT(process->arguments(), list_args_matcher); + set_common_expectations_on(process); - EXPECT_CALL(*process, execute).WillOnce(Return(success)); - EXPECT_CALL(*process, read_all_standard_output).WillOnce(Return(QByteArray::fromStdString(snapshot_tag + ' '))); + EXPECT_THAT(process->arguments(), list_args_matcher); + set_tag_output(process, snapshot_tag); }); MP_EXPECT_THROW_THAT(quick_snapshot("whatever").capture(), From be5fe2e08dfeab98211d384de1deff156fb53b4c Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 10 Jan 2024 22:46:04 +0000 Subject: [PATCH 121/139] [snapshot] Be robust to missing file on erase Avoids failing to erase a snapshot because its JSON file is not available. --- src/platform/backends/shared/base_snapshot.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/platform/backends/shared/base_snapshot.cpp b/src/platform/backends/shared/base_snapshot.cpp index 7b42cd4e2b..b0cfd18a93 100644 --- a/src/platform/backends/shared/base_snapshot.cpp +++ b/src/platform/backends/shared/base_snapshot.cpp @@ -238,7 +238,7 @@ auto mp::BaseSnapshot::erase_helper() auto deleting_filepath = tmp_dir->filePath(snapshot_filename); QFile snapshot_file{snapshot_filepath}; - if (!MP_FILEOPS.rename(snapshot_file, deleting_filepath)) + if (!MP_FILEOPS.rename(snapshot_file, deleting_filepath) && snapshot_file.exists()) throw std::runtime_error{ fmt::format("Failed to move snapshot file to temporary destination: {}", deleting_filepath)}; From 1dbdeae3c9713d4fc048fce5e1b4e8906bdcd6c4 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 10 Jan 2024 22:53:02 +0000 Subject: [PATCH 122/139] [tests] Check erasure of QEMU snapshots --- tests/test_qemu_snapshot.cpp | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/test_qemu_snapshot.cpp b/tests/test_qemu_snapshot.cpp index ca0aefebf0..3203e4f0f6 100644 --- a/tests/test_qemu_snapshot.cpp +++ b/tests/test_qemu_snapshot.cpp @@ -59,6 +59,11 @@ struct TestQemuSnapshot : public Test return mp::QemuSnapshot{name, "", nullptr, specs, vm, desc}; } + mp::QemuSnapshot loaded_snapshot() + { + return mp::QemuSnapshot{mpt::test_data_path_for("test_snapshot.json"), vm, desc}; + } + template static std::string derive_tag(T&& index) { @@ -196,4 +201,32 @@ TEST_F(TestQemuSnapshot, captureThrowsOnRepeatedTag) HasSubstr(desc.image.image_path.toStdString())))); } +TEST_F(TestQemuSnapshot, erasesSnapshot) +{ + auto snapshot = loaded_snapshot(); + auto proc_count = 0; + + auto mock_factory_scope = mpt::MockProcessFactory::Inject(); + mock_factory_scope->register_callback([&](mpt::MockProcess* process) { + ASSERT_LE(++proc_count, 2); + + set_common_expectations_on(process); + + auto tag = derive_tag(snapshot.get_index()); + if (proc_count == 1) + { + EXPECT_THAT(process->arguments(), list_args_matcher); + set_tag_output(process, tag); + } + else + { + EXPECT_THAT(process->arguments(), + ElementsAre("snapshot", "-d", QString::fromStdString(tag), desc.image.image_path)); + } + }); + + snapshot.erase(); + EXPECT_EQ(proc_count, 2); +} + } // namespace From 738344f193d5b4dc5cacf4cf2719fa7e4b2cd1d0 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 10 Jan 2024 23:04:26 +0000 Subject: [PATCH 123/139] [tests] Check erase on missing QEMU snapshot --- tests/test_qemu_snapshot.cpp | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/test_qemu_snapshot.cpp b/tests/test_qemu_snapshot.cpp index 3203e4f0f6..42df315875 100644 --- a/tests/test_qemu_snapshot.cpp +++ b/tests/test_qemu_snapshot.cpp @@ -16,6 +16,7 @@ */ #include "common.h" +#include "mock_logger.h" #include "mock_process_factory.h" #include "mock_snapshot.h" #include "mock_virtual_machine.h" @@ -229,4 +230,25 @@ TEST_F(TestQemuSnapshot, erasesSnapshot) EXPECT_EQ(proc_count, 2); } +TEST_F(TestQemuSnapshot, eraseLogsOnMissingTag) +{ + auto snapshot = loaded_snapshot(); + auto proc_count = 0; + + auto mock_factory_scope = mpt::MockProcessFactory::Inject(); + mock_factory_scope->register_callback([&](mpt::MockProcess* process) { + ASSERT_EQ(++proc_count, 1); + + set_common_expectations_on(process); + EXPECT_THAT(process->arguments(), list_args_matcher); + set_tag_output(process, "some-tag-other-than-the-one-we-are-looking-for"); + }); + + auto expected_log_level = mpl::Level::warning; + auto logger_scope = mpt::MockLogger::inject(expected_log_level); + logger_scope.mock_logger->expect_log(expected_log_level, "Could not find"); + + snapshot.erase(); +} + } // namespace From d31af84ec2e56d6ee42752f81618f37ef35fdb7d Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 10 Jan 2024 23:16:09 +0000 Subject: [PATCH 124/139] [tests] Check applying QEMU snapshots --- tests/test_qemu_snapshot.cpp | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/test_qemu_snapshot.cpp b/tests/test_qemu_snapshot.cpp index 42df315875..c91b6ffcd3 100644 --- a/tests/test_qemu_snapshot.cpp +++ b/tests/test_qemu_snapshot.cpp @@ -251,4 +251,31 @@ TEST_F(TestQemuSnapshot, eraseLogsOnMissingTag) snapshot.erase(); } +TEST_F(TestQemuSnapshot, appliesSnapshot) +{ + auto snapshot = loaded_snapshot(); + auto proc_count = 0; + + auto mock_factory_scope = mpt::MockProcessFactory::Inject(); + mock_factory_scope->register_callback([&](mpt::MockProcess* process) { + ASSERT_EQ(++proc_count, 1); + + set_common_expectations_on(process); + EXPECT_THAT(process->arguments(), + ElementsAre("snapshot", + "-a", + QString::fromStdString(derive_tag(snapshot.get_index())), + desc.image.image_path)); + }); + + desc.num_cores = 8598; + desc.mem_size = mp::MemorySize{"49"}; + desc.disk_space = mp::MemorySize{"328"}; + + snapshot.apply(); + + EXPECT_EQ(desc.num_cores, snapshot.get_num_cores()); + EXPECT_EQ(desc.mem_size, snapshot.get_mem_size()); + EXPECT_EQ(desc.disk_space, snapshot.get_disk_space()); +} } // namespace From 5e84c28cfb6401d335860151a611e871a262e598 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Wed, 10 Jan 2024 23:28:20 +0000 Subject: [PATCH 125/139] [tests] Check rollback on failure to apply --- tests/test_qemu_snapshot.cpp | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/test_qemu_snapshot.cpp b/tests/test_qemu_snapshot.cpp index c91b6ffcd3..7794ba7e8b 100644 --- a/tests/test_qemu_snapshot.cpp +++ b/tests/test_qemu_snapshot.cpp @@ -92,6 +92,7 @@ struct TestQemuSnapshot : public Test ArgsMatcher list_args_matcher = ElementsAre("snapshot", "-l", desc.image.image_path); inline static const auto success = mp::ProcessState{0, std::nullopt}; + inline static const auto failure = mp::ProcessState{1, std::nullopt}; inline static const auto specs = [] { const auto cpus = 3; const auto mem_size = mp::MemorySize{"1.23G"}; @@ -278,4 +279,28 @@ TEST_F(TestQemuSnapshot, appliesSnapshot) EXPECT_EQ(desc.mem_size, snapshot.get_mem_size()); EXPECT_EQ(desc.disk_space, snapshot.get_disk_space()); } + +TEST_F(TestQemuSnapshot, keepsDescOnFailure) +{ + auto snapshot = loaded_snapshot(); + auto proc_count = 0; + + auto mock_factory_scope = mpt::MockProcessFactory::Inject(); + mock_factory_scope->register_callback([&](mpt::MockProcess* process) { + ASSERT_EQ(++proc_count, 1); + EXPECT_CALL(*process, execute).WillOnce(Return(failure)); + }); + + desc.num_cores = 123; + desc.mem_size = mp::MemorySize{"321"}; + desc.disk_space = mp::MemorySize{"56K"}; + + const auto orig_desc = desc; + MP_EXPECT_THROW_THAT(snapshot.apply(), std::runtime_error, mpt::match_what(HasSubstr("qemu-img failed"))); + + EXPECT_EQ(orig_desc.num_cores, desc.num_cores); + EXPECT_EQ(orig_desc.mem_size, desc.mem_size); + EXPECT_EQ(orig_desc.disk_space, desc.disk_space); +} + } // namespace From 1712a0e22ab4c800ab8d0401a5ef3c50a52fd982 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 12 Jan 2024 20:01:50 +0000 Subject: [PATCH 126/139] [tests] Check throw on unsupported snapshots --- tests/test_base_virtual_machine.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 9ada126bbb..15c9a44571 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -82,6 +82,12 @@ struct MockBaseVirtualMachine : public mpt::MockVirtualMachineT parent), (override)); + + void simulate_no_snapshots_support() const // doing this here to access protected method on the base + { + auto& self = *this; + MP_DELEGATE_MOCK_CALLS_ON_BASE(self, require_snapshots_support, mp::BaseVirtualMachine); + } }; struct StubBaseVirtualMachine : public mp::BaseVirtualMachine @@ -338,6 +344,14 @@ TEST_F(BaseVM, startsWithNoSnapshots) EXPECT_EQ(vm.get_num_snapshots(), 0); } +TEST_F(BaseVM, throwsOnSnapshotsRequestIfNotSupported) +{ + vm.simulate_no_snapshots_support(); + MP_EXPECT_THROW_THAT(vm.get_num_snapshots(), + mp::NotImplementedOnThisBackendException, + mpt::match_what(HasSubstr("snapshots"))); +} + TEST_F(BaseVM, takesSnapshots) { auto snapshot = std::make_shared>(); From cb3f61d0f492aabf32a90d16eef0eb90182c1ea4 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 12 Jan 2024 20:23:49 +0000 Subject: [PATCH 127/139] [tests] Improve test for trim_end utility --- tests/test_utils.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/test_utils.cpp b/tests/test_utils.cpp index 4bb28b32cb..c4ccf42c95 100644 --- a/tests/test_utils.cpp +++ b/tests/test_utils.cpp @@ -390,12 +390,13 @@ TEST(Utils, to_cmd_arguments_with_double_quotes_are_escaped) EXPECT_THAT(output, ::testing::StrEq("they said \\\"please\\\"")); } -TEST(Utils, trim_end_actually_trims_end) +TEST(Utils, trimEndActuallyTrimsEnd) { - std::string s{"I'm a great\n\t string \n \f \n \r \t \v"}; + std::string s{"\n \f \n \r \t \vI'm a great\n\t string \n \f \n \r \t \v"}; mp::utils::trim_end(s); - EXPECT_THAT(s, ::testing::StrEq("I'm a great\n\t string")); + EXPECT_THAT(s, ::testing::StrEq("\n \f \n \r \t \vI'm a great\n\t string")); +} } TEST(Utils, trim_newline_works) From 31056e639a38b247ac4d7e996d0779d6a6de1fd4 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 12 Jan 2024 20:24:07 +0000 Subject: [PATCH 128/139] [test] Check trim_begin utility --- tests/test_utils.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/test_utils.cpp b/tests/test_utils.cpp index c4ccf42c95..697c8c8fce 100644 --- a/tests/test_utils.cpp +++ b/tests/test_utils.cpp @@ -397,6 +397,13 @@ TEST(Utils, trimEndActuallyTrimsEnd) EXPECT_THAT(s, ::testing::StrEq("\n \f \n \r \t \vI'm a great\n\t string")); } + +TEST(Utils, trimBeginActuallyTrimsTheBeginning) +{ + std::string s{"\n \f \n \r \t \vI'm a great\n\t string \n \f \n \r \t \v"}; + mp::utils::trim_begin(s); + + EXPECT_EQ(s, "I'm a great\n\t string \n \f \n \r \t \v"); } TEST(Utils, trim_newline_works) From 4612ec92b0caa867ec45b844e5635b85eb2f4711 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 12 Jan 2024 20:29:33 +0000 Subject: [PATCH 129/139] [test] Check trim utility --- tests/test_utils.cpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/test_utils.cpp b/tests/test_utils.cpp index 697c8c8fce..0f5a50e83b 100644 --- a/tests/test_utils.cpp +++ b/tests/test_utils.cpp @@ -406,6 +406,22 @@ TEST(Utils, trimBeginActuallyTrimsTheBeginning) EXPECT_EQ(s, "I'm a great\n\t string \n \f \n \r \t \v"); } +TEST(Utils, trimActuallyTrims) +{ + std::string s{"\n \f \n \r \t \vI'm a great\n\t string \n \f \n \r \t \v"}; + mp::utils::trim(s); + + EXPECT_EQ(s, "I'm a great\n\t string"); +} + +TEST(Utils, trimAcceptsCustomFilter) +{ + std::string s{"\n \f \n \r \t \vI'm a great\n\t string \n \f \n \r \t \v"}; + mp::utils::trim(s, [](unsigned char c) { return c == '\n' || c == '\v'; }); + + EXPECT_EQ(s, " \f \n \r \t \vI'm a great\n\t string \n \f \n \r \t "); +} + TEST(Utils, trim_newline_works) { std::string s{"correct\n"}; From 0a790c446afe6a1cf23bb4ed51798479a4637e7e Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 12 Jan 2024 20:35:13 +0000 Subject: [PATCH 130/139] [tests] Refactor sample string out of trim tests --- tests/test_utils.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/tests/test_utils.cpp b/tests/test_utils.cpp index 0f5a50e83b..a77b8232f1 100644 --- a/tests/test_utils.cpp +++ b/tests/test_utils.cpp @@ -390,33 +390,34 @@ TEST(Utils, to_cmd_arguments_with_double_quotes_are_escaped) EXPECT_THAT(output, ::testing::StrEq("they said \\\"please\\\"")); } -TEST(Utils, trimEndActuallyTrimsEnd) +struct TestTrimUtilities : public Test { std::string s{"\n \f \n \r \t \vI'm a great\n\t string \n \f \n \r \t \v"}; +}; + +TEST_F(TestTrimUtilities, trimEndActuallyTrimsEnd) +{ mp::utils::trim_end(s); EXPECT_THAT(s, ::testing::StrEq("\n \f \n \r \t \vI'm a great\n\t string")); } -TEST(Utils, trimBeginActuallyTrimsTheBeginning) +TEST_F(TestTrimUtilities, trimBeginActuallyTrimsTheBeginning) { - std::string s{"\n \f \n \r \t \vI'm a great\n\t string \n \f \n \r \t \v"}; mp::utils::trim_begin(s); EXPECT_EQ(s, "I'm a great\n\t string \n \f \n \r \t \v"); } -TEST(Utils, trimActuallyTrims) +TEST_F(TestTrimUtilities, trimActuallyTrims) { - std::string s{"\n \f \n \r \t \vI'm a great\n\t string \n \f \n \r \t \v"}; mp::utils::trim(s); EXPECT_EQ(s, "I'm a great\n\t string"); } -TEST(Utils, trimAcceptsCustomFilter) +TEST_F(TestTrimUtilities, trimAcceptsCustomFilter) { - std::string s{"\n \f \n \r \t \vI'm a great\n\t string \n \f \n \r \t \v"}; mp::utils::trim(s, [](unsigned char c) { return c == '\n' || c == '\v'; }); EXPECT_EQ(s, " \f \n \r \t \vI'm a great\n\t string \n \f \n \r \t "); From 7aab0c688ecf05d9a165f9a5d2ed2c3cc33325f1 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 12 Jan 2024 21:07:41 +0000 Subject: [PATCH 131/139] [tests] Check creation of QEMU-specific snapshots --- tests/qemu/test_qemu_backend.cpp | 39 ++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/tests/qemu/test_qemu_backend.cpp b/tests/qemu/test_qemu_backend.cpp index bc582ed9d2..9ae779bf74 100644 --- a/tests/qemu/test_qemu_backend.cpp +++ b/tests/qemu/test_qemu_backend.cpp @@ -35,8 +35,10 @@ #include #include #include +#include #include #include +#include #include #include @@ -675,6 +677,43 @@ TEST_F(QemuBackend, ssh_hostname_timeout_throws_and_sets_unknown_state) EXPECT_EQ(machine.state, mp::VirtualMachine::State::unknown); } +struct PublicSnapshotMakingQemuVM : public mp::QemuVirtualMachine +{ + using mp::QemuVirtualMachine::make_specific_snapshot; + using mp::QemuVirtualMachine::QemuVirtualMachine; +}; + +TEST_F(QemuBackend, createsQemuSnapshotsFromSpecs) +{ + mpt::StubVMStatusMonitor stub_monitor; + PublicSnapshotMakingQemuVM machine{default_description, + mock_qemu_platform.get(), + stub_monitor, + instance_dir.path()}; + + auto snapshot_name = "elvis"; + auto snapshot_comment = "has left the building"; + const mp::VMSpecs specs{2, + mp::MemorySize{"3.21G"}, + mp::MemorySize{"4.32M"}, + "00:00:00:00:00:00", + {}, + "asdf", + mp::VirtualMachine::State::stopped, + {}, + false, + {}, + {}}; + auto snapshot = machine.make_specific_snapshot(snapshot_name, snapshot_comment, specs, nullptr); + EXPECT_EQ(snapshot->get_name(), snapshot_name); + EXPECT_EQ(snapshot->get_comment(), snapshot_comment); + EXPECT_EQ(snapshot->get_num_cores(), specs.num_cores); + EXPECT_EQ(snapshot->get_mem_size(), specs.mem_size); + EXPECT_EQ(snapshot->get_disk_space(), specs.disk_space); + EXPECT_EQ(snapshot->get_state(), specs.state); + EXPECT_EQ(snapshot->get_parent(), nullptr); +} + TEST_F(QemuBackend, lists_no_networks) { EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { From 32c063fdb076c381b085604e215674439ae9ba49 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Mon, 15 Jan 2024 19:12:57 +0000 Subject: [PATCH 132/139] [tests] Use a mock vm to test QEMU snapshots --- tests/qemu/test_qemu_backend.cpp | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/tests/qemu/test_qemu_backend.cpp b/tests/qemu/test_qemu_backend.cpp index 9ae779bf74..1d5410ce82 100644 --- a/tests/qemu/test_qemu_backend.cpp +++ b/tests/qemu/test_qemu_backend.cpp @@ -21,6 +21,7 @@ #include "tests/mock_environment_helpers.h" #include "tests/mock_process_factory.h" #include "tests/mock_status_monitor.h" +#include "tests/mock_virtual_machine.h" #include "tests/stub_process_factory.h" #include "tests/stub_ssh_key_provider.h" #include "tests/stub_status_monitor.h" @@ -677,19 +678,15 @@ TEST_F(QemuBackend, ssh_hostname_timeout_throws_and_sets_unknown_state) EXPECT_EQ(machine.state, mp::VirtualMachine::State::unknown); } -struct PublicSnapshotMakingQemuVM : public mp::QemuVirtualMachine +struct PublicSnapshotMakingQemuVM : public mpt::MockVirtualMachineT { + using mpt::MockVirtualMachineT::MockVirtualMachineT; using mp::QemuVirtualMachine::make_specific_snapshot; - using mp::QemuVirtualMachine::QemuVirtualMachine; }; TEST_F(QemuBackend, createsQemuSnapshotsFromSpecs) { - mpt::StubVMStatusMonitor stub_monitor; - PublicSnapshotMakingQemuVM machine{default_description, - mock_qemu_platform.get(), - stub_monitor, - instance_dir.path()}; + PublicSnapshotMakingQemuVM machine{"mock-qemu-vm"}; auto snapshot_name = "elvis"; auto snapshot_comment = "has left the building"; From 9020fa2f583efc631144dbbc6a537c8fb77ef083 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Mon, 15 Jan 2024 19:19:04 +0000 Subject: [PATCH 133/139] [tests] Snapshot creation from file by QEMU VM --- tests/qemu/test_qemu_backend.cpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/qemu/test_qemu_backend.cpp b/tests/qemu/test_qemu_backend.cpp index 1d5410ce82..bb512fd196 100644 --- a/tests/qemu/test_qemu_backend.cpp +++ b/tests/qemu/test_qemu_backend.cpp @@ -20,8 +20,10 @@ #include "tests/common.h" #include "tests/mock_environment_helpers.h" #include "tests/mock_process_factory.h" +#include "tests/mock_snapshot.h" #include "tests/mock_status_monitor.h" #include "tests/mock_virtual_machine.h" +#include "tests/path.h" #include "tests/stub_process_factory.h" #include "tests/stub_ssh_key_provider.h" #include "tests/stub_status_monitor.h" @@ -711,6 +713,23 @@ TEST_F(QemuBackend, createsQemuSnapshotsFromSpecs) EXPECT_EQ(snapshot->get_parent(), nullptr); } +TEST_F(QemuBackend, createsQemuSnapshotsFromJsonFile) +{ + PublicSnapshotMakingQemuVM machine{"mock-qemu-vm"}; + + const auto parent = std::make_shared(); + EXPECT_CALL(machine, get_snapshot(2)).WillOnce(Return(parent)); + + auto snapshot = machine.make_specific_snapshot(mpt::test_data_path_for("test_snapshot.json")); + EXPECT_EQ(snapshot->get_name(), "snapshot3"); + EXPECT_EQ(snapshot->get_comment(), "A comment"); + EXPECT_EQ(snapshot->get_num_cores(), 1); + EXPECT_EQ(snapshot->get_mem_size(), mp::MemorySize{"1G"}); + EXPECT_EQ(snapshot->get_disk_space(), mp::MemorySize{"5G"}); + EXPECT_EQ(snapshot->get_state(), mp::VirtualMachine::State::off); + EXPECT_EQ(snapshot->get_parent(), parent); +} + TEST_F(QemuBackend, lists_no_networks) { EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { From fe4f7f162983a9ccf0691a5dcd3e0ae0c6dd634d Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Mon, 15 Jan 2024 20:56:13 +0000 Subject: [PATCH 134/139] [tests] Compare shared_ptr directly where possible --- tests/test_base_virtual_machine.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 15c9a44571..affcd47f96 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -767,7 +767,7 @@ TEST_F(BaseVM, usesRestoredSnapshotAsParentForNewSnapshots) auto root_snapshot = snapshot_album[0]; ASSERT_EQ(snapshot_album.size(), 1); - EXPECT_EQ(vm.take_snapshot(specs, "second", "")->get_parent().get(), root_snapshot.get()); + EXPECT_EQ(vm.take_snapshot(specs, "second", "")->get_parent(), root_snapshot); ASSERT_EQ(snapshot_album.size(), 2); EXPECT_EQ(vm.take_snapshot(specs, "third", "")->get_parent().get(), snapshot_album[1].get()); @@ -778,7 +778,7 @@ TEST_F(BaseVM, usesRestoredSnapshotAsParentForNewSnapshots) EXPECT_CALL(*root_snapshot, get_metadata).WillRepeatedly(ReturnRef(metadata)); vm.restore_snapshot(root_name, specs); - EXPECT_EQ(vm.take_snapshot(specs, "fourth", "")->get_parent().get(), root_snapshot.get()); + EXPECT_EQ(vm.take_snapshot(specs, "fourth", "")->get_parent(), root_snapshot); } TEST_F(BaseVM, loadSnasphotThrowsIfSnapshotsNotImplemented) @@ -839,7 +839,7 @@ TEST_P(TestLoadingOfPaddedGenericSnapshotInfo, loadsAndUsesSnapshotHeadIndex) auto name = "julius"; vm.take_snapshot({}, name, ""); - EXPECT_EQ(vm.get_snapshot(name)->get_parent().get(), snapshot.get()); + EXPECT_EQ(vm.get_snapshot(name)->get_parent(), snapshot); } std::vector space_paddings = {"", " ", " ", "\n", " \n", "\n\n\n", "\t", "\t\t\t", "\t \n \t "}; @@ -1000,7 +1000,7 @@ TEST_F(BaseVM, takeSnapshotRevertsHeadAndCount) mock_snapshotting(); auto new_snapshot = vm.take_snapshot(specs, attempted_name, ""); - EXPECT_EQ(new_snapshot->get_parent().get(), early_snapshot.get()); + EXPECT_EQ(new_snapshot->get_parent(), early_snapshot); EXPECT_EQ(new_snapshot->get_index(), 2); // snapshot count not increased by failed snapshot } @@ -1017,7 +1017,7 @@ TEST_F(BaseVM, renameFailureIsReverted) EXPECT_CALL(*snapshot, set_name(Eq(attempted_name))).WillOnce(Throw(std::runtime_error{"intentional"})); EXPECT_ANY_THROW(vm.rename_snapshot(current_name, attempted_name)); - EXPECT_EQ(vm.get_snapshot(current_name).get(), snapshot.get()); + EXPECT_EQ(vm.get_snapshot(current_name), snapshot); } TEST_F(BaseVM, persistsGenericSnapshotInfoWhenTakingSnapshot) From 1cdd0448af07a7ce0bf09e1750b5a4971c86713d Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Mon, 15 Jan 2024 21:16:53 +0000 Subject: [PATCH 135/139] [tests] Verify support for snapshots in QEMU --- tests/qemu/test_qemu_backend.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/qemu/test_qemu_backend.cpp b/tests/qemu/test_qemu_backend.cpp index bb512fd196..9e27039a62 100644 --- a/tests/qemu/test_qemu_backend.cpp +++ b/tests/qemu/test_qemu_backend.cpp @@ -684,8 +684,15 @@ struct PublicSnapshotMakingQemuVM : public mpt::MockVirtualMachineT::MockVirtualMachineT; using mp::QemuVirtualMachine::make_specific_snapshot; + using mp::QemuVirtualMachine::require_snapshots_support; }; +TEST_F(QemuBackend, supportsSnapshots) +{ + PublicSnapshotMakingQemuVM vm{"asdf"}; + EXPECT_NO_THROW(vm.require_snapshots_support()); +} + TEST_F(QemuBackend, createsQemuSnapshotsFromSpecs) { PublicSnapshotMakingQemuVM machine{"mock-qemu-vm"}; From 1665ae45f3dd9bbab9606ec2a18001007e9c261b Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Mon, 15 Jan 2024 22:05:30 +0000 Subject: [PATCH 136/139] [tests] Check writeJson creates parent directory --- tests/CMakeLists.txt | 1 + tests/test_json_utils.cpp | 52 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) create mode 100644 tests/test_json_utils.cpp diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 31e42c727e..a2ca034862 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -81,6 +81,7 @@ add_executable(multipass_tests test_image_vault.cpp test_instance_settings_handler.cpp test_ip_address.cpp + test_json_utils.cpp test_memory_size.cpp test_mock_standard_paths.cpp test_new_release_monitor.cpp diff --git a/tests/test_json_utils.cpp b/tests/test_json_utils.cpp new file mode 100644 index 0000000000..1f795f0682 --- /dev/null +++ b/tests/test_json_utils.cpp @@ -0,0 +1,52 @@ +/* + * Copyright (C) Canonical, Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 3. + * + * This program 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 . + * + */ + +#include "common.h" +#include "mock_file_ops.h" + +#include + +#include +#include +#include +#include + +namespace mpt = multipass::test; +using namespace testing; + +namespace +{ +struct TestJsonUtils : public Test +{ + mpt::MockFileOps::GuardedMock guarded_mock_file_ops = mpt::MockFileOps::inject(); + mpt::MockFileOps& mock_file_ops = *guarded_mock_file_ops.first; +}; + +TEST_F(TestJsonUtils, writeJsonCreatesDirectory) +{ + const auto separator = QDir::separator(); + auto dir = QString{"a%1b%1c"}.arg(separator); + auto file_name = "asd.blag"; + auto file_path = QString{"%1%2%3"}.arg(dir, separator, file_name); + QJsonObject json{}; + + EXPECT_CALL(mock_file_ops, mkpath(Eq(dir), Eq("."))).WillOnce(Return(true)); + EXPECT_CALL(mock_file_ops, open(A(), _)).WillOnce(Throw(std::runtime_error{"intentional"})); + EXPECT_ANY_THROW(MP_JSONUTILS.write_json(json, file_path)); +} + +} // namespace From 7ec8b45d94e6126cc2e43cca3382e09c0ab39c72 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Mon, 15 Jan 2024 22:09:19 +0000 Subject: [PATCH 137/139] [tests] Check handling of failure to create dir Check handling of failure to create parent directory in `JsonUtils::writeJson` --- tests/test_json_utils.cpp | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/tests/test_json_utils.cpp b/tests/test_json_utils.cpp index 1f795f0682..a77edc958a 100644 --- a/tests/test_json_utils.cpp +++ b/tests/test_json_utils.cpp @@ -34,19 +34,26 @@ struct TestJsonUtils : public Test { mpt::MockFileOps::GuardedMock guarded_mock_file_ops = mpt::MockFileOps::inject(); mpt::MockFileOps& mock_file_ops = *guarded_mock_file_ops.first; + inline static const QChar separator = QDir::separator(); + inline static const QString dir = QStringLiteral("a%1b%1c").arg(separator); + inline static const QString file_name = QStringLiteral("asd.blag"); + inline static const QString file_path = QStringLiteral("%1%2%3").arg(dir, separator, file_name); + inline static const QJsonObject json{}; }; TEST_F(TestJsonUtils, writeJsonCreatesDirectory) { - const auto separator = QDir::separator(); - auto dir = QString{"a%1b%1c"}.arg(separator); - auto file_name = "asd.blag"; - auto file_path = QString{"%1%2%3"}.arg(dir, separator, file_name); - QJsonObject json{}; - EXPECT_CALL(mock_file_ops, mkpath(Eq(dir), Eq("."))).WillOnce(Return(true)); EXPECT_CALL(mock_file_ops, open(A(), _)).WillOnce(Throw(std::runtime_error{"intentional"})); EXPECT_ANY_THROW(MP_JSONUTILS.write_json(json, file_path)); } +TEST_F(TestJsonUtils, writeJsonThrowsOnFailureToCreateDirectory) +{ + EXPECT_CALL(mock_file_ops, mkpath(Eq(dir), Eq("."))).WillOnce(Return(false)); + MP_EXPECT_THROW_THAT(MP_JSONUTILS.write_json(json, file_path), + std::runtime_error, + mpt::match_what(AllOf(HasSubstr("Could not create"), HasSubstr(dir.toStdString())))); +} + } // namespace From 0f5ff289a381c0ae83c033426a7eb3a6096ba33c Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Mon, 15 Jan 2024 22:31:48 +0000 Subject: [PATCH 138/139] [tests] Check full sequence to write JSON --- tests/test_json_utils.cpp | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/tests/test_json_utils.cpp b/tests/test_json_utils.cpp index a77edc958a..5bc4ea2ad9 100644 --- a/tests/test_json_utils.cpp +++ b/tests/test_json_utils.cpp @@ -22,6 +22,7 @@ #include #include +#include #include #include @@ -38,19 +39,27 @@ struct TestJsonUtils : public Test inline static const QString dir = QStringLiteral("a%1b%1c").arg(separator); inline static const QString file_name = QStringLiteral("asd.blag"); inline static const QString file_path = QStringLiteral("%1%2%3").arg(dir, separator, file_name); - inline static const QJsonObject json{}; + inline static const char* json_text = R"({"a": [1,2,3]})"; + inline static const QJsonObject json = QJsonDocument::fromJson(json_text).object(); + template + inline static Matcher file_matcher = Property(&T::fileName, Eq(file_path)); }; -TEST_F(TestJsonUtils, writeJsonCreatesDirectory) +TEST_F(TestJsonUtils, writesJsonTransactionally) { + auto json_matcher = + ResultOf([](auto&& text) { return QJsonDocument::fromJson(std::forward(text), nullptr); }, + Property(&QJsonDocument::object, Eq(json))); EXPECT_CALL(mock_file_ops, mkpath(Eq(dir), Eq("."))).WillOnce(Return(true)); - EXPECT_CALL(mock_file_ops, open(A(), _)).WillOnce(Throw(std::runtime_error{"intentional"})); - EXPECT_ANY_THROW(MP_JSONUTILS.write_json(json, file_path)); + EXPECT_CALL(mock_file_ops, open(file_matcher, _)).WillOnce(Return(true)); + EXPECT_CALL(mock_file_ops, write(file_matcher, json_matcher)).WillOnce(Return(14)); + EXPECT_CALL(mock_file_ops, commit(file_matcher)).WillOnce(Return(true)); + EXPECT_NO_THROW(MP_JSONUTILS.write_json(json, file_path)); } TEST_F(TestJsonUtils, writeJsonThrowsOnFailureToCreateDirectory) { - EXPECT_CALL(mock_file_ops, mkpath(Eq(dir), Eq("."))).WillOnce(Return(false)); + EXPECT_CALL(mock_file_ops, mkpath).WillOnce(Return(false)); MP_EXPECT_THROW_THAT(MP_JSONUTILS.write_json(json, file_path), std::runtime_error, mpt::match_what(AllOf(HasSubstr("Could not create"), HasSubstr(dir.toStdString())))); From f6bfb4887e93a96acd5843b6815daa13cda6f7b6 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Mon, 15 Jan 2024 22:41:02 +0000 Subject: [PATCH 139/139] [tests] Check remaining failure cases of writeJson --- tests/test_json_utils.cpp | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/test_json_utils.cpp b/tests/test_json_utils.cpp index 5bc4ea2ad9..e33b44add7 100644 --- a/tests/test_json_utils.cpp +++ b/tests/test_json_utils.cpp @@ -26,6 +26,8 @@ #include #include +#include + namespace mpt = multipass::test; using namespace testing; @@ -65,4 +67,34 @@ TEST_F(TestJsonUtils, writeJsonThrowsOnFailureToCreateDirectory) mpt::match_what(AllOf(HasSubstr("Could not create"), HasSubstr(dir.toStdString())))); } +TEST_F(TestJsonUtils, writeJsonThrowsOnFailureToOpenFile) +{ + EXPECT_CALL(mock_file_ops, mkpath).WillOnce(Return(true)); + EXPECT_CALL(mock_file_ops, open(_, _)).WillOnce(Return(false)); + MP_EXPECT_THROW_THAT(MP_JSONUTILS.write_json(json, file_path), + std::runtime_error, + mpt::match_what(AllOf(HasSubstr("Could not open"), HasSubstr(file_path.toStdString())))); +} + +TEST_F(TestJsonUtils, writeJsonThrowsOnFailureToWriteFile) +{ + EXPECT_CALL(mock_file_ops, mkpath).WillOnce(Return(true)); + EXPECT_CALL(mock_file_ops, open(_, _)).WillOnce(Return(true)); + EXPECT_CALL(mock_file_ops, write(_, _)).WillOnce(Return(-1)); + MP_EXPECT_THROW_THAT(MP_JSONUTILS.write_json(json, file_path), + std::runtime_error, + mpt::match_what(AllOf(HasSubstr("Could not write"), HasSubstr(file_path.toStdString())))); +} + +TEST_F(TestJsonUtils, writeJsonThrowsOnFailureToCommit) +{ + EXPECT_CALL(mock_file_ops, mkpath).WillOnce(Return(true)); + EXPECT_CALL(mock_file_ops, open(_, _)).WillOnce(Return(true)); + EXPECT_CALL(mock_file_ops, write(_, _)).WillOnce(Return(1234)); + EXPECT_CALL(mock_file_ops, commit).WillOnce(Return(false)); + MP_EXPECT_THROW_THAT(MP_JSONUTILS.write_json(json, file_path), + std::runtime_error, + mpt::match_what(AllOf(HasSubstr("Could not commit"), HasSubstr(file_path.toStdString())))); +} + } // namespace