winget-cli

commit 810a441915a4ee0416120699e0b3f7dc1788e0bd
parent e630cf7c64291c3009e7b2e03ab73e314e501c76
Author: JohnMcPMS <johnmcp@microsoft.com>
Date:   Thu, 23 Sep 2021 09:36:48 -0700

Disable test hooks fixes (#1501)


Diffstat:
M
.github/actions/spelling/allow.txt
 | 
1
+
M
src/AppInstallerCLICore/ExecutionContext.cpp
 | 
6
++++++
M
src/AppInstallerCLICore/ExecutionContext.h
 | 
8
+++++++-
M
src/AppInstallerCLITests/WorkFlow.cpp
 | 
35
++++++++++++++++++-----------------
M
src/AppInstallerCommonCore/GroupPolicy.cpp
 | 
2
++

5 files changed, 34 insertions(+), 18 deletions(-)
diff --git a/.github/actions/spelling/allow.txt b/.github/actions/spelling/allow.txt
@@ -314,6 +314,7 @@ nunit
 nupkg
 nuspec
 OAuth
+ODR
 ofstream
 opencode
 opensource
diff --git a/src/AppInstallerCLICore/ExecutionContext.cpp b/src/AppInstallerCLICore/ExecutionContext.cpp
@@ -211,4 +211,10 @@ namespace AppInstaller::CLI::Execution
         return m_threadGlobals;
     }
 
+#ifndef AICLI_DISABLE_TEST_HOOKS
+    bool Context::ShouldExecuteWorkflowTask(const Workflow::WorkflowTask& task)
+    {
+        return (m_shouldExecuteWorkflowTask ? m_shouldExecuteWorkflowTask(task) : true);
+    }
+#endif
 }
diff --git a/src/AppInstallerCLICore/ExecutionContext.h b/src/AppInstallerCLICore/ExecutionContext.h
@@ -132,9 +132,15 @@ namespace AppInstaller::CLI::Execution
 
 #ifndef AICLI_DISABLE_TEST_HOOKS
         // Enable tests to override behavior
-        virtual bool ShouldExecuteWorkflowTask(const Workflow::WorkflowTask&) { return true; }
+        bool ShouldExecuteWorkflowTask(const Workflow::WorkflowTask& task);
 #endif
 
+    protected:
+        // Neither virtual functions nor member fields can be inside AICLI_DISABLE_TEST_HOOKS
+        // or we could have ODR violations that lead to nasty bugs. So we will simply never
+        // use this if AICLI_DISABLE_TEST_HOOKS is defined.
+        std::function<bool(const Workflow::WorkflowTask&)> m_shouldExecuteWorkflowTask;
+
     private:
         DestructionToken m_disableCtrlHandlerOnExit = false;
         bool m_isTerminated = false;
diff --git a/src/AppInstallerCLITests/WorkFlow.cpp b/src/AppInstallerCLITests/WorkFlow.cpp
@@ -306,7 +306,24 @@ namespace
 
         // For clone
         TestContext(std::ostream& out, std::istream& in, std::shared_ptr<std::vector<WorkflowTaskOverride>> overrides) :
-            m_out(out), m_in(in), m_overrides(overrides), m_isClone(true), Context(out, in) {}
+            m_out(out), m_in(in), m_overrides(overrides), m_isClone(true), Context(out, in)
+        {
+            m_shouldExecuteWorkflowTask = [this](const Workflow::WorkflowTask& task)
+            {
+                auto itr = std::find_if(m_overrides->begin(), m_overrides->end(), [&](const WorkflowTaskOverride& wto) { return wto.Target == task; });
+
+                if (itr == m_overrides->end())
+                {
+                    return true;
+                }
+                else
+                {
+                    itr->Used = true;
+                    itr->Override(*this);
+                    return false;
+                }
+            };
+        }
 
         ~TestContext()
         {
@@ -322,22 +339,6 @@ namespace
             }
         }
 
-        bool ShouldExecuteWorkflowTask(const Workflow::WorkflowTask& task) override
-        {
-            auto itr = std::find_if(m_overrides->begin(), m_overrides->end(), [&](const WorkflowTaskOverride& wto) { return wto.Target == task; });
-
-            if (itr == m_overrides->end())
-            {
-                return true;
-            }
-            else
-            {
-                itr->Used = true;
-                itr->Override(*this);
-                return false;
-            }
-        }
-
         void Override(const WorkflowTaskOverride& wto)
         {
             m_overrides->emplace_back(wto);
diff --git a/src/AppInstallerCommonCore/GroupPolicy.cpp b/src/AppInstallerCommonCore/GroupPolicy.cpp
@@ -322,6 +322,7 @@ namespace AppInstaller::Settings
         return InstanceInternal();
     }
 
+#ifndef AICLI_DISABLE_TEST_HOOKS
     void GroupPolicy::OverrideInstance(GroupPolicy* overridePolicy)
     {
         InstanceInternal(overridePolicy);
@@ -331,4 +332,5 @@ namespace AppInstaller::Settings
     {
         InstanceInternal(nullptr);
     }
+#endif
 } 
\ No newline at end of file