refactor create_install_plan tests

4 files changed, 64 insertions, 74 deletions

diff --git a/toolsrc/src/commands_ci.cpp b/toolsrc/src/commands_ci.cpp
index 5f6fc95b5..95d722302 100644
--- a/toolsrc/src/commands_ci.cpp
+++ b/toolsrc/src/commands_ci.cpp
@@ -40,8 +40,9 @@ namespace vcpkg::Commands::CI
         const std::vector<PackageSpec> specs = load_all_package_specs(paths.get_filesystem(), paths.ports, triplet);
 
         StatusParagraphs status_db = database_load_check(paths);
+        auto paths_port_file = Dependencies::PathsPortFile(paths);
         const std::vector<InstallPlanAction> install_plan =
-            Dependencies::create_install_plan(Dependencies::PathsPortFile(paths), specs, status_db);
+            Dependencies::create_install_plan(paths_port_file, specs, status_db);
         Checks::check_exit(VCPKG_LINE_INFO, !install_plan.empty(), "Install plan cannot be empty");
 
         std::vector<BuildResult> results;
diff --git a/toolsrc/src/commands_install.cpp b/toolsrc/src/commands_install.cpp
index a58432eef..d340a9cf8 100644
--- a/toolsrc/src/commands_install.cpp
+++ b/toolsrc/src/commands_install.cpp
@@ -359,8 +359,9 @@ namespace vcpkg::Commands::Install
 
         // create the plan
         StatusParagraphs status_db = database_load_check(paths);
+        auto paths_port_file = Dependencies::PathsPortFile(paths);
         std::vector<InstallPlanAction> install_plan =
-            Dependencies::create_install_plan(Dependencies::PathsPortFile(paths), specs, status_db);
+            Dependencies::create_install_plan(paths_port_file, specs, status_db);
         Checks::check_exit(VCPKG_LINE_INFO, !install_plan.empty(), "Install plan cannot be empty");
 
         // log the plan
diff --git a/toolsrc/src/test_install_plan.cpp b/toolsrc/src/test_install_plan.cpp
index 2404e30f7..93a1436d8 100644
--- a/toolsrc/src/test_install_plan.cpp
+++ b/toolsrc/src/test_install_plan.cpp
@@ -9,12 +9,11 @@ namespace UnitTest1
 {
     class InstallPlanTests : public TestClass<InstallPlanTests>
     {
-        TEST_METHOD(basic_install_scheme)
+        struct PackageSpecMap
         {
-            std::vector<std::unique_ptr<StatusParagraph>> status_paragraphs;
-
             std::unordered_map<PackageSpec, SourceControlFile> map;
-            auto add_scf = [&](std::vector<std::unordered_map<std::string, std::string>>&& fields) -> PackageSpec {
+            PackageSpec get_package_spec(std::vector<std::unordered_map<std::string, std::string>>&& fields)
+            {
                 auto m_pgh = vcpkg::SourceControlFile::parse_control_file(std::move(fields));
                 Assert::IsTrue(m_pgh.has_value());
                 auto& scf = *m_pgh.get();
@@ -23,13 +22,19 @@ namespace UnitTest1
                 Assert::IsTrue(spec.has_value());
                 map.emplace(*spec.get(), std::move(*scf.get()));
                 return PackageSpec{*spec.get()};
-            };
+            }
+        };
+        TEST_METHOD(basic_install_scheme)
+        {
+            std::vector<std::unique_ptr<StatusParagraph>> status_paragraphs;
+
+            PackageSpecMap spec_map;
 
-            auto spec_a = add_scf({{{"Source", "a"}, {"Version", "1.2.8"}, {"Build-Depends", "b"}}});
-            auto spec_b = add_scf({{{"Source", "b"}, {"Version", "1.3"}, {"Build-Depends", "c"}}});
-            auto spec_c = add_scf({{{"Source", "c"}, {"Version", "2.5.3"}, {"Build-Depends", ""}}});
+            auto spec_a = spec_map.get_package_spec({{{"Source", "a"}, {"Version", "1.2.8"}, {"Build-Depends", "b"}}});
+            auto spec_b = spec_map.get_package_spec({{{"Source", "b"}, {"Version", "1.3"}, {"Build-Depends", "c"}}});
+            auto spec_c = spec_map.get_package_spec({{{"Source", "c"}, {"Version", "2.5.3"}, {"Build-Depends", ""}}});
 
-            auto map_port = Dependencies::MapPortFile(map);
+            auto map_port = Dependencies::MapPortFile(spec_map.map);
             auto install_plan =
                 Dependencies::create_install_plan(map_port, {spec_a}, StatusParagraphs(std::move(status_paragraphs)));
 
@@ -43,43 +48,30 @@ namespace UnitTest1
         {
             std::vector<std::unique_ptr<StatusParagraph>> status_paragraphs;
 
-            std::unordered_map<PackageSpec, SourceControlFile> map;
-            auto add_scf = [&](std::vector<std::unordered_map<std::string, std::string>>&& fields) -> PackageSpec {
-                auto m_pgh = vcpkg::SourceControlFile::parse_control_file(std::move(fields));
-                Assert::IsTrue(m_pgh.has_value());
-                auto& scf = *m_pgh.get();
-
-                auto spec = PackageSpec::from_name_and_triplet(scf->core_paragraph->name, Triplet::X86_WINDOWS);
-                Assert::IsTrue(spec.has_value());
-                map.emplace(*spec.get(), std::move(*scf.get()));
-                return PackageSpec{*spec.get()};
-            };
+            PackageSpecMap spec_map;
 
-            auto spec_a = add_scf({{{"Source", "a"}, {"Version", "1.2.8"}, {"Build-Depends", "d"}}});
-            auto spec_b = add_scf({{{"Source", "b"}, {"Version", "1.3"}, {"Build-Depends", "d, e"}}});
-            auto spec_c = add_scf({{{"Source", "c"}, {"Version", "2.5.3"}, {"Build-Depends", "e, h"}}});
-            auto spec_d = add_scf({{{"Source", "d"}, {"Version", "4.0"}, {"Build-Depends", "f, g, h"}}});
-            auto spec_e = add_scf({{{"Source", "e"}, {"Version", "1.0"}, {"Build-Depends", "g"}}});
-            auto spec_f = add_scf({{{"Source", "f"}, {"Version", "1.0"}, {"Build-Depends", ""}}});
-            auto spec_g = add_scf({{{"Source", "g"}, {"Version", "1.0"}, {"Build-Depends", ""}}});
-            auto spec_h = add_scf({{{"Source", "h"}, {"Version", "1.0"}, {"Build-Depends", ""}}});
+            auto spec_a = spec_map.get_package_spec({{{"Source", "a"}, {"Version", "1.2.8"}, {"Build-Depends", "d"}}});
+            auto spec_b = spec_map.get_package_spec({{{"Source", "b"}, {"Version", "1.3"}, {"Build-Depends", "d, e"}}});
+            auto spec_c =
+                spec_map.get_package_spec({{{"Source", "c"}, {"Version", "2.5.3"}, {"Build-Depends", "e, h"}}});
+            auto spec_d =
+                spec_map.get_package_spec({{{"Source", "d"}, {"Version", "4.0"}, {"Build-Depends", "f, g, h"}}});
+            auto spec_e = spec_map.get_package_spec({{{"Source", "e"}, {"Version", "1.0"}, {"Build-Depends", "g"}}});
+            auto spec_f = spec_map.get_package_spec({{{"Source", "f"}, {"Version", "1.0"}, {"Build-Depends", ""}}});
+            auto spec_g = spec_map.get_package_spec({{{"Source", "g"}, {"Version", "1.0"}, {"Build-Depends", ""}}});
+            auto spec_h = spec_map.get_package_spec({{{"Source", "h"}, {"Version", "1.0"}, {"Build-Depends", ""}}});
 
-            auto map_port = Dependencies::MapPortFile(map);
+            auto map_port = Dependencies::MapPortFile(spec_map.map);
             auto install_plan = Dependencies::create_install_plan(
                 map_port, {spec_a, spec_b, spec_c}, StatusParagraphs(std::move(status_paragraphs)));
 
             auto iterator_pos = [&](const PackageSpec& spec) -> int {
-                int counter = 0;
-                for (auto&& install_p : install_plan)
-                {
-                    if (install_p.spec == spec)
-                    {
-                        return counter;
-                    }
-                    counter++;
-                }
-                return -1;
+                auto it = std::find_if(
+                    install_plan.begin(), install_plan.end(), [&](auto& action) { return action.spec == spec; });
+                Assert::IsTrue(it != install_plan.end());
+                return (int)(it - install_plan.begin());
             };
+
             int a_pos = iterator_pos(spec_a), b_pos = iterator_pos(spec_b), c_pos = iterator_pos(spec_c),
                 d_pos = iterator_pos(spec_d), e_pos = iterator_pos(spec_e), f_pos = iterator_pos(spec_f),
                 g_pos = iterator_pos(spec_g), h_pos = iterator_pos(spec_h);
@@ -112,32 +104,28 @@ namespace UnitTest1
                                                                               {"Depends", ""},
                                                                               {"Status", "install ok installed"}}));
 
-            std::unordered_map<PackageSpec, SourceControlFile> map;
-            auto add_scf = [&](std::vector<std::unordered_map<std::string, std::string>>&& fields) -> PackageSpec {
-                auto m_pgh = vcpkg::SourceControlFile::parse_control_file(std::move(fields));
-                Assert::IsTrue(m_pgh.has_value());
-                auto& scf = *m_pgh.get();
-
-                auto spec = PackageSpec::from_name_and_triplet(scf->core_paragraph->name, Triplet::X86_WINDOWS);
-                Assert::IsTrue(spec.has_value());
-                map.emplace(*spec.get(), std::move(*scf.get()));
-                return PackageSpec{*spec.get()};
-            };
-
-            auto spec_h = add_scf({{{"Source", "h"}, {"Version", "1.2.8"}, {"Build-Depends", "j, k"}}});
-            auto spec_c = add_scf({{{"Source", "c"}, {"Version", "1.2.8"}, {"Build-Depends", "d, e, f, g, h, j, k"}}});
-            auto spec_k = add_scf({{{"Source", "k"}, {"Version", "1.2.8"}, {"Build-Depends", ""}}});
-            auto spec_b =
-                add_scf({{{"Source", "b"}, {"Version", "1.2.8"}, {"Build-Depends", "c, d, e, f, g, h, j, k"}}});
-            auto spec_d = add_scf({{{"Source", "d"}, {"Version", "1.2.8"}, {"Build-Depends", "e, f, g, h, j, k"}}});
-            auto spec_j = add_scf({{{"Source", "j"}, {"Version", "1.2.8"}, {"Build-Depends", "k"}}});
-            auto spec_f = add_scf({{{"Source", "f"}, {"Version", "1.2.8"}, {"Build-Depends", "g, h, j, k"}}});
-            auto spec_e = add_scf({{{"Source", "e"}, {"Version", "1.2.8"}, {"Build-Depends", "f, g, h, j, k"}}});
-            auto spec_a =
-                add_scf({{{"Source", "a"}, {"Version", "1.2.8"}, {"Build-Depends", "b, c, d, e, f, g, h, j, k"}}});
-            auto spec_g = add_scf({{{"Source", "g"}, {"Version", "1.2.8"}, {"Build-Depends", "h, j, k"}}});
-
-            auto map_port = Dependencies::MapPortFile(map);
+            PackageSpecMap spec_map;
+
+            auto spec_h =
+                spec_map.get_package_spec({{{"Source", "h"}, {"Version", "1.2.8"}, {"Build-Depends", "j, k"}}});
+            auto spec_c = spec_map.get_package_spec(
+                {{{"Source", "c"}, {"Version", "1.2.8"}, {"Build-Depends", "d, e, f, g, h, j, k"}}});
+            auto spec_k = spec_map.get_package_spec({{{"Source", "k"}, {"Version", "1.2.8"}, {"Build-Depends", ""}}});
+            auto spec_b = spec_map.get_package_spec(
+                {{{"Source", "b"}, {"Version", "1.2.8"}, {"Build-Depends", "c, d, e, f, g, h, j, k"}}});
+            auto spec_d = spec_map.get_package_spec(
+                {{{"Source", "d"}, {"Version", "1.2.8"}, {"Build-Depends", "e, f, g, h, j, k"}}});
+            auto spec_j = spec_map.get_package_spec({{{"Source", "j"}, {"Version", "1.2.8"}, {"Build-Depends", "k"}}});
+            auto spec_f =
+                spec_map.get_package_spec({{{"Source", "f"}, {"Version", "1.2.8"}, {"Build-Depends", "g, h, j, k"}}});
+            auto spec_e = spec_map.get_package_spec(
+                {{{"Source", "e"}, {"Version", "1.2.8"}, {"Build-Depends", "f, g, h, j, k"}}});
+            auto spec_a = spec_map.get_package_spec(
+                {{{"Source", "a"}, {"Version", "1.2.8"}, {"Build-Depends", "b, c, d, e, f, g, h, j, k"}}});
+            auto spec_g =
+                spec_map.get_package_spec({{{"Source", "g"}, {"Version", "1.2.8"}, {"Build-Depends", "h, j, k"}}});
+
+            auto map_port = Dependencies::MapPortFile(spec_map.map);
             auto install_plan =
                 Dependencies::create_install_plan(map_port, {spec_a}, StatusParagraphs(std::move(status_paragraphs)));
 
diff --git a/toolsrc/src/vcpkg_Dependencies.cpp b/toolsrc/src/vcpkg_Dependencies.cpp
index 726c147fd..984d0ab4c 100644
--- a/toolsrc/src/vcpkg_Dependencies.cpp
+++ b/toolsrc/src/vcpkg_Dependencies.cpp
@@ -140,23 +140,23 @@ namespace vcpkg::Dependencies
     }
 
     MapPortFile::MapPortFile(const std::unordered_map<PackageSpec, SourceControlFile>& map) : ports(map){};
-    const SourceControlFile* MapPortFile::get_control_file(const PackageSpec& spec) const
+    const SourceControlFile& MapPortFile::get_control_file(const PackageSpec& spec) const
     {
         auto scf = ports.find(spec);
         if (scf == ports.end())
         {
             Checks::exit_fail(VCPKG_LINE_INFO);
         }
-        return &scf->second;
+        return scf->second;
     }
 
     PathsPortFile::PathsPortFile(const VcpkgPaths& paths) : ports(paths){};
-    const SourceControlFile* PathsPortFile::get_control_file(const PackageSpec& spec) const
+    const SourceControlFile& PathsPortFile::get_control_file(const PackageSpec& spec) const
     {
         std::unordered_map<PackageSpec, SourceControlFile>::iterator cache_it = cache.find(spec);
         if (cache_it != cache.end())
         {
-            return &cache_it->second;
+            return cache_it->second;
         }
         Parse::ParseExpected<SourceControlFile> source_control_file =
             Paragraphs::try_load_port(ports.get_filesystem(), ports.port_dir(spec));
@@ -164,9 +164,9 @@ namespace vcpkg::Dependencies
         if (auto scf = source_control_file.get())
         {
             auto it = cache.emplace(spec, std::move(*scf->get()));
-            return &it.first->second;
+            return it.first->second;
         }
-
+        print_error_message(source_control_file.error());
         Checks::exit_fail(VCPKG_LINE_INFO);
     }
 
@@ -201,7 +201,7 @@ namespace vcpkg::Dependencies
                 auto it = status_db.find_installed(spec);
                 if (it != status_db.end()) return InstallPlanAction{spec, {*it->get(), nullopt, nullopt}, request_type};
                 return InstallPlanAction{
-                    spec, {nullopt, nullopt, *port_file_provider.get_control_file(spec)->core_paragraph}, request_type};
+                    spec, {nullopt, nullopt, *port_file_provider.get_control_file(spec).core_paragraph}, request_type};
             }
         };