From 69a76b1dfafc1720736004291cab73ab79f61261 Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Mon, 21 Aug 2017 18:32:49 -0700 Subject: [PATCH] Audio VTS: improve audio policy validation error messages If multiple config are found, each will be validated before failing the test. Also add a missing new line before dumping libxml2 errors. Test: run the following script to check for regressions and test that invalid config make the test fail. runTest() {( set -xu local readonly MODULE=$1 local readonly TEST=$2 local readonly XML=$3 runVTS() { vts-tradefed run commandAndExit vts \ --skip-all-system-status-check --primary-abi-only \ --skip-preconditions --module $MODULE -t $TEST | awk '/FAILED: 0/{e=1} {print} END {exit !e}'; } local FAIL=0 # used instead of return if clean up is needed echo "# Test valid config" runVTS || return 1 echo "# Test multiple invalid match" adb shell touch /system/etc/$XML runVTS && FAIL=2 adb shell rm /system/etc/$XML echo "# Test multiple valid match (not supported if xinclude)" if !adb shell grep -q xi:include /vendor/etc/$XML; then adb shell cp /{vendor,system}/etc/$XML runVTS || FAIL=3 adb shell rm /system/etc/$XML fi echo "# Test invalid config" adb shell sed -i "'2i'" /vendor/etc/$XML runVTS && FAIL=4 adb shell sed -i 2d /vendor/etc/$XML echo "#Test no config" adb shell mv /vendor/etc/$XML{,.hide} runVTS && FAIL=5 adb shell mv /vendor/etc/$XML{.hide,} echo "# Test that the test did not break the config" runVTS || FAIL=6 return $FAIL )} runTest VtsHalAudioV2_0Target CheckConfig.audioPolicyConfigurationValidation audio_policy_configuration.xml && runTest VtsHalAudioEffectV2_0Target CheckConfig.audioEffectsConfigurationValidation audio_effects.xml && echo TEST PASSED Bug: 64881365 Change-Id: If0443f85e0d687eed04819337519e4d9f09f5ca9 Signed-off-by: Kevin Rocard --- .../functional/ValidateAudioConfiguration.cpp | 16 +++---- .../utility/include/utility/ValidateXml.h | 35 +++++++++++++++- audio/common/test/utility/src/ValidateXml.cpp | 42 +++++++++++++++++-- audio/effect/2.0/vts/functional/Android.bp | 1 + .../ValidateAudioEffectsConfiguration.cpp | 14 ++++--- 5 files changed, 87 insertions(+), 21 deletions(-) diff --git a/audio/2.0/vts/functional/ValidateAudioConfiguration.cpp b/audio/2.0/vts/functional/ValidateAudioConfiguration.cpp index ec3259a1e3..4e280f5aa2 100644 --- a/audio/2.0/vts/functional/ValidateAudioConfiguration.cpp +++ b/audio/2.0/vts/functional/ValidateAudioConfiguration.cpp @@ -20,15 +20,11 @@ #include "utility/ValidateXml.h" TEST(CheckConfig, audioPolicyConfigurationValidation) { - const char* configName = "audio_policy_configuration.xml"; - const char* possibleConfigLocations[] = {"/odm/etc", "/vendor/etc", "/system/etc"}; - const char* configSchemaPath = "/data/local/tmp/audio_policy_configuration.xsd"; + RecordProperty("description", + "Verify that the audio policy configuration file " + "is valid according to the schema"); - for (std::string folder : possibleConfigLocations) { - const auto configPath = folder + '/' + configName; - if (access(configPath.c_str(), R_OK) == 0) { - ASSERT_VALID_XML(configPath.c_str(), configSchemaPath); - return; // The framework does not read past the first config file found - } - } + std::vector locations = {"/odm/etc", "/vendor/etc", "/system/etc"}; + EXPECT_ONE_VALID_XML_MULTIPLE_LOCATIONS("audio_policy_configuration.xml", locations, + "/data/local/tmp/audio_policy_configuration.xsd"); } diff --git a/audio/common/test/utility/include/utility/ValidateXml.h b/audio/common/test/utility/include/utility/ValidateXml.h index fdfa50666e..d7188396ca 100644 --- a/audio/common/test/utility/include/utility/ValidateXml.h +++ b/audio/common/test/utility/include/utility/ValidateXml.h @@ -32,13 +32,44 @@ namespace utility { * See ASSERT_VALID_XML for a helper macro. */ ::testing::AssertionResult validateXml(const char* xmlFilePathExpr, const char* xsdFilePathExpr, - const char* xmlFilePath, const char* xsdPathName); + const char* xmlFilePath, const char* xsdFilePath); -/** Helper gtest ASSERT to test xml validity against an xsd. */ +/** Helper gtest ASSERT to test XML validity against an XSD. */ #define ASSERT_VALID_XML(xmlFilePath, xsdFilePath) \ ASSERT_PRED_FORMAT2(::android::hardware::audio::common::test::utility::validateXml, \ xmlFilePath, xsdFilePath) +/** Helper gtest EXPECT to test XML validity against an XSD. */ +#define EXPECT_VALID_XML(xmlFilePath, xsdFilePath) \ + EXPECT_PRED_FORMAT2(::android::hardware::audio::common::test::utility::validateXml, \ + xmlFilePath, xsdFilePath) + +/** Validate an XML according to an xsd. + * The XML file must be in at least one of the provided locations. + * If multiple are found, all are validated. + */ +::testing::AssertionResult validateXmlMultipleLocations( + const char* xmlFileNameExpr, const char* xmlFileLocationsExpr, const char* xsdFilePathExpr, + const char* xmlFileName, std::vector xmlFileLocations, const char* xsdFilePath); + +/** ASSERT that an XML is valid according to an xsd. + * The XML file must be in at least one of the provided locations. + * If multiple are found, all are validated. + */ +#define ASSERT_ONE_VALID_XML_MULTIPLE_LOCATIONS(xmlFileName, xmlFileLocations, xsdFilePath) \ + ASSERT_PRED_FORMAT3( \ + ::android::hardware::audio::common::test::utility::validateXmlMultipleLocations, \ + xmlFileName, xmlFileLocations, xsdFilePath) + +/** EXPECT an XML to be valid according to an xsd. + * The XML file must be in at least one of the provided locations. + * If multiple are found, all are validated. + */ +#define EXPECT_ONE_VALID_XML_MULTIPLE_LOCATIONS(xmlFileName, xmlFileLocations, xsdFilePath) \ + EXPECT_PRED_FORMAT3( \ + ::android::hardware::audio::common::test::utility::validateXmlMultipleLocations, \ + xmlFileName, xmlFileLocations, xsdFilePath) + } // utility } // test } // common diff --git a/audio/common/test/utility/src/ValidateXml.cpp b/audio/common/test/utility/src/ValidateXml.cpp index 784f9401ec..30dec30309 100644 --- a/audio/common/test/utility/src/ValidateXml.cpp +++ b/audio/common/test/utility/src/ValidateXml.cpp @@ -17,6 +17,8 @@ #define LOG_TAG "ValidateAudioConfig" #include +#include + #define LIBXML_SCHEMAS_ENABLED #include #define LIBXML_XINCLUDE_ENABLED @@ -94,9 +96,9 @@ struct Libxml2Global { Libxml2Global libxml2; auto context = [&]() { - return std::string() + " While validating: " + xmlFilePathExpr + + return std::string() + " While validating: " + xmlFilePathExpr + "\n Which is: " + xmlFilePath + "\nAgainst the schema: " + xsdFilePathExpr + - "\n Which is: " + xsdFilePath + "Libxml2 errors\n" + libxml2.getErrors(); + "\n Which is: " + xsdFilePath + "\nLibxml2 errors:\n" + libxml2.getErrors(); }; auto schemaParserCtxt = make_xmlUnique(xmlSchemaNewParserCtxt(xsdFilePath)); @@ -117,7 +119,7 @@ struct Libxml2Global { auto schemaCtxt = make_xmlUnique(xmlSchemaNewValidCtxt(schema.get())); int ret = xmlSchemaValidateDoc(schemaCtxt.get(), doc.get()); if (ret > 0) { - return ::testing::AssertionFailure() << "xml is not valid according to the xsd.\n" + return ::testing::AssertionFailure() << "XML is not valid according to the xsd\n" << context(); } if (ret < 0) { @@ -127,6 +129,40 @@ struct Libxml2Global { return ::testing::AssertionSuccess(); } +::testing::AssertionResult validateXmlMultipleLocations( + const char* xmlFileNameExpr, const char* xmlFileLocationsExpr, const char* xsdFilePathExpr, + const char* xmlFileName, std::vector xmlFileLocations, const char* xsdFilePath) { + using namespace std::string_literals; + + std::vector errors; + std::vector foundFiles; + + for (const char* location : xmlFileLocations) { + std::string xmlFilePath = location + "/"s + xmlFileName; + if (access(xmlFilePath.c_str(), F_OK) != 0) { + // If the file does not exist ignore this location and fallback on the next one + continue; + } + foundFiles.push_back(" " + xmlFilePath + '\n'); + auto result = validateXml("xmlFilePath", xsdFilePathExpr, xmlFilePath.c_str(), xsdFilePath); + if (!result) { + errors.push_back(result.message()); + } + } + + if (foundFiles.empty()) { + errors.push_back("No xml file found in provided locations.\n"); + } + + return ::testing::AssertionResult(errors.empty()) + << errors.size() << " error" << (errors.size() == 1 ? " " : "s ") + << std::accumulate(begin(errors), end(errors), "occurred during xml validation:\n"s) + << " While validating all: " << xmlFileNameExpr + << "\n Which is: " << xmlFileName + << "\n In the following folders: " << xmlFileLocationsExpr + << "\n Which is: " << ::testing::PrintToString(xmlFileLocations); +} + } // utility } // test } // common diff --git a/audio/effect/2.0/vts/functional/Android.bp b/audio/effect/2.0/vts/functional/Android.bp index 7b421cb7e7..f5a49b33e8 100644 --- a/audio/effect/2.0/vts/functional/Android.bp +++ b/audio/effect/2.0/vts/functional/Android.bp @@ -30,6 +30,7 @@ cc_test { "libxml2", ], shared_libs: [ + "libeffectsconfig", "libicuuc", ], } diff --git a/audio/effect/2.0/vts/functional/ValidateAudioEffectsConfiguration.cpp b/audio/effect/2.0/vts/functional/ValidateAudioEffectsConfiguration.cpp index fdc1347497..d0bc6908d6 100644 --- a/audio/effect/2.0/vts/functional/ValidateAudioEffectsConfiguration.cpp +++ b/audio/effect/2.0/vts/functional/ValidateAudioEffectsConfiguration.cpp @@ -15,16 +15,18 @@ */ #include +#include + +#include #include "utility/ValidateXml.h" TEST(CheckConfig, audioEffectsConfigurationValidation) { RecordProperty("description", "Verify that the effects configuration file is valid according to the schema"); - const char* xmlConfigFile = "/vendor/etc/audio_effects.xml"; - // Not every device uses XML configuration, so only validate - // if the XML configuration actually exists. - if (access(xmlConfigFile, F_OK) == 0) { - ASSERT_VALID_XML(xmlConfigFile, "/data/local/tmp/audio_effects_conf_V2_0.xsd"); - } + using namespace android::effectsConfig; + + std::vector locations(std::begin(DEFAULT_LOCATIONS), std::end(DEFAULT_LOCATIONS)); + EXPECT_ONE_VALID_XML_MULTIPLE_LOCATIONS(DEFAULT_NAME, locations, + "/data/local/tmp/audio_effects_conf_V2_0.xsd"); }