From ee537ae8fa0b3dc4e01b133bd3351c477043fd32 Mon Sep 17 00:00:00 2001 From: John Date: Mon, 28 Jun 2021 13:01:57 +0100 Subject: [PATCH] Address PR comments Signed-off-by: John --- .../Code/Source/PythonCoverageEditorModule.h | 2 +- .../PythonCoverageEditorSystemComponent.cpp | 32 +++++++++---------- .../PythonCoverageEditorSystemComponent.h | 25 ++++++++------- 3 files changed, 29 insertions(+), 30 deletions(-) diff --git a/AutomatedTesting/Gem/PythonCoverage/Code/Source/PythonCoverageEditorModule.h b/AutomatedTesting/Gem/PythonCoverage/Code/Source/PythonCoverageEditorModule.h index 180ed79656..2295d20f3e 100644 --- a/AutomatedTesting/Gem/PythonCoverage/Code/Source/PythonCoverageEditorModule.h +++ b/AutomatedTesting/Gem/PythonCoverage/Code/Source/PythonCoverageEditorModule.h @@ -22,7 +22,7 @@ namespace PythonCoverage { public: AZ_CLASS_ALLOCATOR_DECL - AZ_RTTI(PythonCoverageEditorModule, "{32C0FFEA-09A7-460F-9257-5BDEF74FCD5B}"); + AZ_RTTI(PythonCoverageEditorModule, "{32C0FFEA-09A7-460F-9257-5BDEF74FCD5B}", AZ::Module); PythonCoverageEditorModule(); ~PythonCoverageEditorModule(); diff --git a/AutomatedTesting/Gem/PythonCoverage/Code/Source/PythonCoverageEditorSystemComponent.cpp b/AutomatedTesting/Gem/PythonCoverage/Code/Source/PythonCoverageEditorSystemComponent.cpp index c33986192a..dec9581296 100644 --- a/AutomatedTesting/Gem/PythonCoverage/Code/Source/PythonCoverageEditorSystemComponent.cpp +++ b/AutomatedTesting/Gem/PythonCoverage/Code/Source/PythonCoverageEditorSystemComponent.cpp @@ -21,7 +21,7 @@ namespace PythonCoverage { - constexpr char* const Caller = "PythonCoverageEditorSystemComponent"; + static constexpr char* const LogCallSite = "PythonCoverageEditorSystemComponent"; void PythonCoverageEditorSystemComponent::Reflect(AZ::ReflectContext* context) { @@ -36,11 +36,8 @@ namespace PythonCoverage AzToolsFramework::EditorPythonScriptNotificationsBus::Handler::BusConnect(); AZ::EntitySystemBus::Handler::BusConnect(); - // Attempt to discover the output directory for the test coverage files - ParseCoverageOutputDirectory(); - // If no output directory discovered, coverage gathering will be disabled - if (m_coverageState == CoverageState::Disabled) + if (ParseCoverageOutputDirectory() == CoverageState::Disabled) { return; } @@ -73,38 +70,38 @@ namespace PythonCoverage } } - void PythonCoverageEditorSystemComponent::ParseCoverageOutputDirectory() + PythonCoverageEditorSystemComponent::CoverageState PythonCoverageEditorSystemComponent::ParseCoverageOutputDirectory() { m_coverageState = CoverageState::Disabled; const AZStd::string configFilePath = LY_TEST_IMPACT_DEFAULT_CONFIG_FILE; if (configFilePath.empty()) { - AZ_Warning(Caller, false, "No test impact analysis framework config file specified."); - return; + AZ_Warning(LogCallSite, false, "No test impact analysis framework config file specified."); + return m_coverageState; } const auto fileSize = AZ::IO::SystemFile::Length(configFilePath.c_str()); if(!fileSize) { - AZ_Error(Caller, false, "Test impact analysis framework config file '%s' does not exist", configFilePath.c_str()); - return; + AZ_Error(LogCallSite, false, "Test impact analysis framework config file '%s' does not exist", configFilePath.c_str()); + return m_coverageState; } AZStd::vector buffer(fileSize + 1); buffer[fileSize] = '\0'; if (!AZ::IO::SystemFile::Read(configFilePath.c_str(), buffer.data())) { - AZ_Error(Caller, false, "Could not read contents of test impact analysis framework config file '%s'", configFilePath.c_str()); - return; + AZ_Error(LogCallSite, false, "Could not read contents of test impact analysis framework config file '%s'", configFilePath.c_str()); + return m_coverageState; } const AZStd::string configurationData = AZStd::string(buffer.begin(), buffer.end()); rapidjson::Document configurationFile; if (configurationFile.Parse(configurationData.c_str()).HasParseError()) { - AZ_Error(Caller, false, "Could not parse test impact analysis framework config file data, JSON has errors"); - return; + AZ_Error(LogCallSite, false, "Could not parse test impact analysis framework config file data, JSON has errors"); + return m_coverageState; } const auto& tempConfig = configurationFile["workspace"]["temp"]; @@ -118,6 +115,7 @@ namespace PythonCoverage // Everything is good to go, await the first python test case m_coverageState = CoverageState::Idle; + return m_coverageState; } void PythonCoverageEditorSystemComponent::WriteCoverageFile() @@ -146,13 +144,13 @@ namespace PythonCoverage m_coverageFile.c_str(), AZ::IO::SystemFile::SF_OPEN_CREATE | AZ::IO::SystemFile::SF_OPEN_CREATE_PATH | AZ::IO::SystemFile::SF_OPEN_WRITE_ONLY)) { - AZ_Error(Caller, false, "Couldn't open file '%s' for writing", m_coverageFile.c_str()); + AZ_Error(LogCallSite, false, "Couldn't open file '%s' for writing", m_coverageFile.c_str()); return; } if (!file.Write(bytes.data(), bytes.size())) { - AZ_Error(Caller, false, "Couldn't write contents for file '%s'", m_coverageFile.c_str()); + AZ_Error(LogCallSite, false, "Couldn't write contents for file '%s'", m_coverageFile.c_str()); return; } } @@ -228,7 +226,7 @@ namespace PythonCoverage { // We need to be able to pinpoint the coverage data to the specific test case names otherwise we will not be able // to specify which specific tests should be run in the future (filename does not necessarily equate to test case name) - AZ_Error(Caller, false, "No test case specified, coverage data gathering will be disabled for this test"); + AZ_Error(LogCallSite, false, "No test case specified, coverage data gathering will be disabled for this test"); return; } diff --git a/AutomatedTesting/Gem/PythonCoverage/Code/Source/PythonCoverageEditorSystemComponent.h b/AutomatedTesting/Gem/PythonCoverage/Code/Source/PythonCoverageEditorSystemComponent.h index 68f9299bcc..c2d6950ade 100644 --- a/AutomatedTesting/Gem/PythonCoverage/Code/Source/PythonCoverageEditorSystemComponent.h +++ b/AutomatedTesting/Gem/PythonCoverage/Code/Source/PythonCoverageEditorSystemComponent.h @@ -39,6 +39,14 @@ namespace PythonCoverage PythonCoverageEditorSystemComponent() = default; private: + //! The coverage state for Python tests. + enum class CoverageState : AZ::u8 + { + Disabled, //!< Python coverage is disabled. + Idle, //!< Python coverage is enabled but not actively gathering coverage data. + Gathering //!< Python coverage is enabled and actively gathering coverage data. + }; + // AZ::Component overrides... void Activate() override; void Deactivate() override; @@ -49,17 +57,17 @@ namespace PythonCoverage // AZ::EditorPythonScriptNotificationsBus ... void OnStartExecuteByFilenameAsTest(AZStd::string_view filename, AZStd::string_view testCase, const AZStd::vector& args) override; - //! Attempts to parse the test impact analysis framework configuration file. - //! If either the test impact analysis framework is disabled or the configuration file cannot be parsed, python coverage - //! is disabled. - void ParseCoverageOutputDirectory(); - //! Enumerates all of the loaded shared library modules and the component descriptors that belong to them. void EnumerateAllModuleComponents(); //! Enumerates all of the component descriptors for the specified entity. void EnumerateComponentsForEntity(const AZ::EntityId& entityId); + //! Attempts to parse the test impact analysis framework configuration file. + //! If either the test impact analysis framework is disabled or the configuration file cannot be parsed, python coverage is disabled. + //! @returns The coverage state after the parsing attempt. + CoverageState ParseCoverageOutputDirectory(); + //! Returns all of the shared library modules that parent the component descriptors of the specified set of activated entities. //! @note Entity component descriptors are still retrieved even if the entity in question has since been deactivated. //! @param entityComponents The set of activated entities and their component descriptors to get the parent modules for. @@ -69,13 +77,6 @@ namespace PythonCoverage //! Writes the current coverage data snapshot to disk. void WriteCoverageFile(); - enum class CoverageState : AZ::u8 - { - Disabled, //!< Python coverage is disabled. - Idle, //!< Python coverage is enabled but not actively gathering coverage data. - Gathering //!< Python coverage is enabled and actively gathering coverage data. - }; - CoverageState m_coverageState = CoverageState::Disabled; //!< Current coverage state. AZStd::unordered_map> m_entityComponentMap; //!< Map of //!< component IDs to component descriptors for all activated entities, organized by test cases.