From 77ec88f86e5eff5969f397a15b1da1520b14af2b Mon Sep 17 00:00:00 2001
From: Junbo Liang <68558268+junbo75@users.noreply.github.com>
Date: Wed, 20 Oct 2021 11:28:28 -0700
Subject: [PATCH] Improve the stability of metrics gem tests by removing local
 file operations (#4761)

Signed-off-by: Junbo Liang <junbo@amazon.com>
---
 .../Code/Include/Private/MetricsManager.h     | 13 +++++-----
 .../AWSMetrics/Code/Tests/AWSMetricsGemMock.h |  5 ----
 .../Code/Tests/MetricsManagerTest.cpp         | 25 ++++++++++++++-----
 3 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/Gems/AWSMetrics/Code/Include/Private/MetricsManager.h b/Gems/AWSMetrics/Code/Include/Private/MetricsManager.h
index 2cd5ba97a8..3bb06acd75 100644
--- a/Gems/AWSMetrics/Code/Include/Private/MetricsManager.h
+++ b/Gems/AWSMetrics/Code/Include/Private/MetricsManager.h
@@ -31,7 +31,7 @@ namespace AWSMetrics
         static const unsigned int DesiredMaxWorkers = 2;
 
         MetricsManager();
-        ~MetricsManager();
+        virtual ~MetricsManager();
 
         //! Initializing the metrics manager
         //! @return Whether the operation is successful.
@@ -93,6 +93,12 @@ namespace AWSMetrics
         //! @return Total number of requests for sending metrics events.
         int GetNumTotalRequests() const;
 
+    protected:
+        //! Send metrics to a local file.
+        //! @param metricsQueue metricsQueue Metrics queue that stores the metrics.
+        //! @return Outcome of the operation.
+        virtual AZ::Outcome<void, AZStd::string> SendMetricsToFile(AZStd::shared_ptr<MetricsQueue> metricsQueue);
+
     private:
         //! Job management
         void SetupJobContext();
@@ -112,11 +118,6 @@ namespace AWSMetrics
         //! @param metricsQueue Metrics events to send.
         void SendMetricsToServiceApiAsync(const MetricsQueue& metricsQueue);
 
-        //! Send metrics to a local file.
-        //! @param metricsQueue metricsQueue Metrics queue that stores the metrics.
-        //! @return Outcome of the operation.
-        AZ::Outcome<void, AZStd::string> SendMetricsToFile(AZStd::shared_ptr<MetricsQueue> metricsQueue);
-
         //! Push metrics events to the front of the queue for retry.
         //! @param metricsEventsForRetry Metrics events for retry.
         void PushMetricsForRetry(MetricsQueue& metricsEventsForRetry);
diff --git a/Gems/AWSMetrics/Code/Tests/AWSMetricsGemMock.h b/Gems/AWSMetrics/Code/Tests/AWSMetricsGemMock.h
index b77f39c93c..4f40fd6ff1 100644
--- a/Gems/AWSMetrics/Code/Tests/AWSMetricsGemMock.h
+++ b/Gems/AWSMetrics/Code/Tests/AWSMetricsGemMock.h
@@ -139,11 +139,6 @@ namespace AWSMetrics
             return true;
         }
 
-        bool RemoveDirectory(const AZStd::string& directory)
-        {
-            return AZ::IO::SystemFile::DeleteDir(directory.c_str());
-        }
-
         AZ::IO::FileIOBase* m_priorFileIO = nullptr;
         AZ::IO::FileIOBase* m_localFileIO = nullptr;
 
diff --git a/Gems/AWSMetrics/Code/Tests/MetricsManagerTest.cpp b/Gems/AWSMetrics/Code/Tests/MetricsManagerTest.cpp
index 3f87a1079d..9fcebec524 100644
--- a/Gems/AWSMetrics/Code/Tests/MetricsManagerTest.cpp
+++ b/Gems/AWSMetrics/Code/Tests/MetricsManagerTest.cpp
@@ -75,6 +75,23 @@ namespace AZ
 
 namespace AWSMetrics
 {
+    class MetricsManagerMock
+        : public MetricsManager
+    {
+    private:
+        AZ::Outcome<void, AZStd::string> SendMetricsToFile(AZStd::shared_ptr<MetricsQueue> metricsQueue) override
+        {
+            if (AZ::IO::FileIOBase::GetInstance())
+            {
+                return AZ::Success();
+            }
+            else
+            {
+                return AZ::Failure(AZStd::string{ "Invalid File IO" });
+            }
+        }
+    };
+
     class AWSMetricsNotificationBusMock
         : protected AWSMetricsNotificationBus::Handler
     {
@@ -134,13 +151,11 @@ namespace AWSMetrics
             AWSMetricsGemAllocatorFixture::SetUp();
             AWSMetricsRequestBus::Handler::BusConnect();
 
-            m_metricsManager = AZStd::make_unique<MetricsManager>();
+            m_metricsManager = AZStd::make_unique<MetricsManagerMock>();
             AZStd::string configFilePath = CreateClientConfigFile(true, (double) TestMetricsEventSizeInBytes / MbToBytes * 2, DefaultFlushPeriodInSeconds, 0);
             m_settingsRegistry->MergeSettingsFile(configFilePath, AZ::SettingsRegistryInterface::Format::JsonMergePatch, {});
             m_metricsManager->Init();
 
-            RemoveFile(m_metricsManager->GetMetricsFilePath());
-            
             ReplaceLocalFileIOWithMockIO();
         }
 
@@ -149,8 +164,6 @@ namespace AWSMetrics
             RevertMockIOToLocalFileIO();
 
             RemoveFile(GetDefaultTestFilePath());
-            RemoveFile(m_metricsManager->GetMetricsFilePath());
-            RemoveDirectory(m_metricsManager->GetMetricsFileDirectory());
 
             m_metricsManager.reset();
 
@@ -233,7 +246,7 @@ namespace AWSMetrics
             }   
         }
 
-        AZStd::unique_ptr<MetricsManager> m_metricsManager;
+        AZStd::unique_ptr<MetricsManagerMock> m_metricsManager;
         AWSMetricsNotificationBusMock m_notifications;
 
         AZ::IO::FileIOBase* m_fileIOMock;