Merge pull request #330 from krocard/add-functional-test-for-element-handle

krocard · krocard · commit 49c3d81a64f6 · 2016-01-08T14:43:25.000+01:00
Add functional tests for element handle

Test the following ElementHandle apis:
 - `getMappingData`
 - `{s,g}etAsXML` with Raw and Hex format
diff --git a/parameter/ConfigurableElement.cpp b/parameter/ConfigurableElement.cpp
@@ -275,18 +275,19 @@ bool CConfigurableElement::setSettingsAsBytes(const std::vector<uint8_t> &bytes,
     return true;
 }
 
-void CConfigurableElement::getListOfElementsWithMapping(
-    std::list<const CConfigurableElement *> &configurableElementPath) const
+std::list<const CConfigurableElement *> CConfigurableElement::getConfigurableElementContext() const
 {
-    // Check parent
-    const CElement *pParent = getParent();
-    if (isOfConfigurableElementType(pParent)) {
+    std::list<const CConfigurableElement *> configurableElementPath;
 
-        const CConfigurableElement *pConfigurableElement =
-            static_cast<const CConfigurableElement *>(pParent);
+    const CElement *element = this;
+    while (element != nullptr and isOfConfigurableElementType(element)) {
+        auto configurableElement = static_cast<const CConfigurableElement *>(element);
 
-        pConfigurableElement->getListOfElementsWithMapping(configurableElementPath);
+        configurableElementPath.push_back(configurableElement);
+        element = element->getParent();
     }
+
+    return configurableElementPath;
 }
 
 // Used for simulation and virtual subsystems
diff --git a/parameter/ConfigurableElement.h b/parameter/ConfigurableElement.h
@@ -133,22 +133,11 @@ class PARAMETER_EXPORT CConfigurableElement : public CElement
     bool setSettingsAsBytes(const std::vector<uint8_t> &bytes,
                             CParameterAccessContext &parameterAccessContext) const;
 
-    /**
-     * Get the list of all the ancestors that have a mapping.
-     *
-     * The mapping is represented as a std::string of all the mapping data (key:value) defined in
-     * the
-     * context of the element.
-     * In this class, the method is generic and calls its parent getListOfElementsWithMappings(...)
-     * method.
-     *
-     * @param[in:out] configurableElementPath List of all the ConfigurableElements found
-     * that have a mapping. Elements are added at the end of the list, so the root Element will be
-     * the last one.
+    /** @return List of all ConfigurableElements that have a mapping relevant in this context.
+     *          Ie: return self and CConfigurableElement ancestor of this node.
      *
      */
-    virtual void getListOfElementsWithMapping(
-        std::list<const CConfigurableElement *> &configurableElementPath) const;
+    std::list<const CConfigurableElement *> getConfigurableElementContext() const;
 
     // Used for simulation and virtual subsystems
     virtual void setDefaultValues(CParameterAccessContext &parameterAccessContext) const;
diff --git a/parameter/ElementHandle.cpp b/parameter/ElementHandle.cpp
@@ -116,12 +116,16 @@ bool ElementHandle::getMappingData(const string &strKey, string &strValue) const
 {
     const std::string *pStrValue;
 
-    if (!mElement.getMappingData(strKey, pStrValue)) {
-        return false;
-    }
+    // Seach for the key in self and ancestors
+    auto elements = mElement.getConfigurableElementContext();
 
-    strValue = *pStrValue;
-    return true;
+    for (auto *element : elements)
+        if (element->getMappingData(strKey, pStrValue)) {
+            strValue = *pStrValue;
+            return true;
+        }
+
+    return false;
 }
 
 bool ElementHandle::getStructureAsXML(std::string &xmlSettings, std::string &error) const
diff --git a/parameter/InstanceConfigurableElement.cpp b/parameter/InstanceConfigurableElement.cpp
@@ -114,19 +114,6 @@ bool CInstanceConfigurableElement::map(IMapper &mapper, std::string &strError)
     return true;
 }
 
-void CInstanceConfigurableElement::getListOfElementsWithMapping(
-    std::list<const CConfigurableElement *> &configurableElementPath) const
-{
-    const CTypeElement *pTypeElement = getTypeElement();
-
-    if (pTypeElement && pTypeElement->hasMappingData()) {
-
-        configurableElementPath.push_back(this);
-    }
-
-    base::getListOfElementsWithMapping(configurableElementPath);
-}
-
 // Element properties
 void CInstanceConfigurableElement::showProperties(std::string &strResult) const
 {
diff --git a/parameter/InstanceConfigurableElement.h b/parameter/InstanceConfigurableElement.h
@@ -92,22 +92,6 @@ class PARAMETER_EXPORT CInstanceConfigurableElement : public CConfigurableElemen
     // Array Length
     size_t getArrayLength() const;
 
-    /**
-     * Get the list of all the ancestors that have a mapping.
-     *
-     * The mapping is represented as a std::string of all the mapping data (key:value) defined in
-     * the
-     * context of the element.
-     * In this class, the method is generic and calls its parent getListOfElementsWithMappings(...)
-     * method.
-     *
-     * @param[in:out] configurableElementPath List of all the ConfigurableElements found
-     * that have a mapping. Elements are added at the end of the list, so the root Element will be
-     * the last one.
-     */
-    virtual void getListOfElementsWithMapping(
-        std::list<const CConfigurableElement *> &configurableElementPath) const;
-
     virtual void structureToXml(CXmlElement &xmlElement,
                                 CXmlSerializingContext &serializingContext) const;
 
diff --git a/parameter/ParameterMgr.cpp b/parameter/ParameterMgr.cpp
@@ -1993,14 +1993,6 @@ bool CParameterMgr::accessParameterValue(const string &strPath, string &strValue
 // User get parameter mapping
 bool CParameterMgr::getParameterMapping(const string &strPath, string &strResult) const
 {
-    CPathNavigator pathNavigator(strPath);
-
-    // Nagivate through system class
-    if (!pathNavigator.navigateThrough(getConstSystemClass()->getName(), strResult)) {
-
-        return false;
-    }
-
     // Get the ConfigurableElement corresponding to strPath
     const CConfigurableElement *pConfigurableElement = getConfigurableElement(strPath, strResult);
     if (!pConfigurableElement) {
@@ -2009,8 +2001,7 @@ bool CParameterMgr::getParameterMapping(const string &strPath, string &strResult
     }
 
     // Find the list of the ancestors of the current ConfigurableElement that have a mapping
-    list<const CConfigurableElement *> configurableElementPath;
-    pConfigurableElement->getListOfElementsWithMapping(configurableElementPath);
+    auto configurableElementPath = pConfigurableElement->getConfigurableElementContext();
 
     // Get the Subsystem containing the ConfigurableElement
     const CSubsystem *pSubsystem = pConfigurableElement->getBelongingSubsystem();
diff --git a/parameter/include/ElementHandle.h b/parameter/include/ElementHandle.h
@@ -91,9 +91,10 @@ class PARAMETER_EXPORT ElementHandle
 
     std::vector<ElementHandle> getChildren();
 
-    /** Get Mapping Data
+    /** Get mapping data of the element context
      *
-     * Retrieve Mapping data associated to a given key if any
+     * Retrieve mapping data associated to a given key if any.
+     * If the key is not present in this element, query ancestors.
      *
      * @param[in] strKey the input mapping key
      * @param[out] strValue the resulting mapping value in case of success
diff --git a/test/functional-tests/Handle.cpp b/test/functional-tests/Handle.cpp
@@ -42,8 +42,12 @@
 #include <libxml/tree.h>
 
 #include <string>
+#include <list>
+
+#include <stdlib.h>
 
 using std::string;
+using std::list;
 using Bytes = std::vector<uint8_t>;
 
 namespace parameterFramework
@@ -156,11 +160,21 @@ struct AllParamsPF : public ParameterFramework
         if (result != expected) {
             utility::TmpFile resultFile(result);
             utility::TmpFile expectedFile(expected);
-            auto gitCommand = "git --no-pager diff --word-diff-regex='[^ <>]+' --color --no-index ";
-            auto diffSuccess =
-                system((gitCommand + resultFile.getPath() + ' ' + expectedFile.getPath()).c_str());
-            if (diffSuccess != 0) {
-                WARN("Failed to pretty-print the difference between actual and expected results.");
+            string command = "git --no-pager diff --word-diff-regex='[^ <>]+'"
+                             "                    --color --no-index --exit-code " +
+                             resultFile.getPath() + ' ' + expectedFile.getPath();
+
+            // `system` return -1 or 127 on failure, the command error code otherwise
+            // `git diff` return 1 if the files are the different (thanks to --exit-code)
+            auto status = system(command.c_str());
+#ifdef WIFEXITED // Posix platform
+            bool success = WIFEXITED(status) and WEXITSTATUS(status) == 1;
+#else
+            bool success = status == 1;
+#endif
+            if (not success) {
+                WARN("Warning: Failed to pretty-print the difference between "
+                     "actual and expected results with `git diff'");
             }
         }
     }
@@ -363,6 +377,24 @@ static const char *testBasicSettingsXML = R"(
         <BitParameter Name="thirty_two">4294967295</BitParameter>
       </BitParameterBlock>
 )";
+static const char *testRawHexBasicSettingsXML = R"(
+      <BooleanParameter Name="bool">0x1</BooleanParameter>
+      <BooleanParameter Name="bool_array">0x0 0x1</BooleanParameter>
+      <IntegerParameter Name="integer">0x0064</IntegerParameter>
+      <IntegerParameter Name="integer_array">0xFFFFFFF6 0x00000000 0x00000008 0x0000000A</IntegerParameter>
+      <FixedPointParameter ValueSpace="Raw" Name="fix_point">0x24000000</FixedPointParameter>
+      <FixedPointParameter ValueSpace="Raw" Name="fix_point_array">0x72000000 0x0B000000 0xF0000000</FixedPointParameter>
+      <EnumParameter Name="enum">five</EnumParameter>
+      <EnumParameter Name="enum_array">eight min eight min</EnumParameter>
+      <StringParameter Name="string">A string of 32 character.@@@@@@@</StringParameter>
+      <BitParameterBlock Name="bit_block">
+        <BitParameter Name="one">0x1</BitParameter>
+        <BitParameter Name="two">0x2</BitParameter>
+        <BitParameter Name="six">0xA</BitParameter>
+        <BitParameter Name="sixteen">0x48</BitParameter>
+        <BitParameter Name="thirty_two">0xFFFFFFFF</BitParameter>
+      </BitParameterBlock>
+)";
 
 SCENARIO_METHOD(SettingsTestPF, "Export and import XML settings", "[handler][settings][xml]")
 {
@@ -377,12 +409,26 @@ SCENARIO_METHOD(SettingsTestPF, "Export and import XML settings", "[handler][set
         checkXMLEq(basicParams.getAsXML(),
                    mkBasicSettings(defaultBasicSettingsXML, "parameter_block"));
     }
-    WHEN ("Importing basic parameter XML") {
-        string testSettings = mkBasicSettings(testBasicSettingsXML, "parameter_block");
-        CHECK_NOTHROW(basicParams.setAsXML(testSettings));
+    string testSettings = mkBasicSettings(testBasicSettingsXML, "parameter_block");
+    string rawTestSettings = mkBasicSettings(testRawHexBasicSettingsXML, "parameter_block");
+
+    auto checkExport = [&] {
         THEN ("Exported settings should be the ones imported") {
             checkXMLEq(basicParams.getAsXML(), testSettings);
         }
+        THEN ("Exported raw settings should be the ones imported") {
+            setRawValueSpace(true);
+            setHexOutputFormat(true);
+            checkXMLEq(basicParams.getAsXML(), rawTestSettings);
+        }
+    };
+    WHEN ("Importing basic parameter XML") {
+        CHECK_NOTHROW(basicParams.setAsXML(testSettings));
+        checkExport();
+    }
+    WHEN ("Importing raw basic parameter XML") {
+        CHECK_NOTHROW(basicParams.setAsXML(rawTestSettings));
+        checkExport();
     }
 }
 
@@ -497,4 +543,63 @@ SCENARIO_METHOD(SettingsTestPF, "Import basic params in one format, export in an
     }
 }
 
+struct MappingPF : public ParameterFramework
+{
+    MappingPF() : ParameterFramework{getConfig()} { REQUIRE_NOTHROW(start()); }
+
+    Config getConfig()
+    {
+        Config config;
+        config.instances = "<BooleanParameter Name='bool' Mapping='bool_map'>";
+        config.subsystemMapping = "subsystem_mapping";
+        return config;
+    }
+};
+
+SCENARIO("Mapping handle access", "[handler][mapping]")
+{
+    GIVEN ("A PF with mappings") {
+        Config config;
+        config.subsystemMapping = "rootK:rootV";
+        config.components = "<ComponentType   Name='componentType' Mapping='typeK:typeV'        />";
+        config.instances = "<BooleanParameter Name='param'         Mapping='paramK:paramV'      />"
+                           "<Component        Name='component'     Mapping='instanceK:instanceV'  "
+                           "           Type='componentType'                                     />";
+        ParameterFramework pf{config};
+        REQUIRE_NOTHROW(pf.start());
+
+        struct TestVector
+        {
+            string path;
+            list<string> valid;
+            list<string> invalid;
+        };
+        list<TestVector> testVectors = {
+            // clang-format off
+            {"/test/test",           {"root"},                     {"param", "type", "instance"}},
+            {"/test/test/param",     {"root", "param"},            {"type", "instance"}},
+            {"/test/test/component", {"root", "type", "instance"}, {"param"}}
+            // clang-format on
+        };
+
+        for (auto &test : testVectors) {
+            GIVEN ("An element handle of " + test.path) {
+                ElementHandle handle(pf, test.path);
+
+                for (auto &valid : test.valid) {
+                    THEN ("The following mapping should exist: " + valid) {
+                        CHECK(handle.getMappingData(valid + "K") == valid + "V");
+                    }
+                }
+
+                for (auto &invalid : test.invalid) {
+                    THEN ("The following mapping should not exist: " + invalid) {
+                        CHECK_THROWS_AS(handle.getMappingData(invalid + "K"), Exception);
+                    }
+                }
+            }
+        }
+    }
+}
+
 } // namespace parameterFramework
diff --git a/test/functional-tests/include/Config.hpp b/test/functional-tests/include/Config.hpp
@@ -50,6 +50,9 @@ struct Config
         (this->*member) = std::forward<T>(value);
     }
 
+    /** Mapping attribute of the test subsystem. */
+    std::string subsystemMapping;
+
     /** Instances of the test subsystem.
      *
      * Content of the configuration
diff --git a/test/functional-tests/include/ConfigFiles.hpp b/test/functional-tests/include/ConfigFiles.hpp
@@ -43,9 +43,11 @@ class ConfigFiles
 {
 public:
     ConfigFiles(const Config &config)
-        : mStructureFile(format(mStructureTemplate, {{"type", config.subsystemType},
-                                                     {"instances", config.instances},
-                                                     {"components", config.components}})),
+        : mStructureFile(
+              format(mStructureTemplate, {{"type", config.subsystemType},
+                                          {"instances", config.instances},
+                                          {"components", config.components},
+                                          {"subsystemMapping", config.subsystemMapping}})),
           mDomainsFile(format(mDomainsTemplate, {{"domains", config.domains}})),
           mConfigFile(format(mConfigTemplate, {{"structurePath", mStructureFile.getPath()},
                                                {"domainsPath", mDomainsFile.getPath()},
@@ -101,7 +103,7 @@ class ConfigFiles
      )";
     const char *mStructureTemplate = R"(<?xml version='1.0' encoding='UTF-8'?>
         <SystemClass Name='test'>
-            <Subsystem Name='test' Type='{type}'>
+            <Subsystem Name='test' Type='{type}' Mapping='{subsystemMapping}'>
                 <ComponentLibrary>
                     {components}
                 </ComponentLibrary>
diff --git a/test/functional-tests/include/ElementHandle.hpp b/test/functional-tests/include/ElementHandle.hpp
@@ -62,6 +62,15 @@ class ElementHandle : private FailureWrapper<::ElementHandle>
      */
     size_t getSize() const { return EH::getSize(); }
 
+    std::string getMappingData(const std::string &key)
+    {
+        std::string value;
+        if (not EH::getMappingData(key, value)) {
+            throw Exception("Could not find mapping key \"" + key + "\" in " + EH::getPath());
+        }
+        return value;
+    }
+
     /** Wrap EH::setAsDouble to throw an exception on failure. */
     void setAsDouble(double value) { mayFailCall(&EH::setAsDouble, value); }
     /** Wrap EH::getAsDouble to throw an exception on failure. */
diff --git a/test/functional-tests/include/ParameterFramework.hpp b/test/functional-tests/include/ParameterFramework.hpp
@@ -84,7 +84,6 @@ class ParameterFramework : private parameterFramework::ConfigFiles,
     using PF::getValidateSchemasOnStart;
     using PF::isValueSpaceRaw;
     using PF::isOutputRawFormatHex;
-    using PF::setOutputRawFormat;
     using PF::isTuningModeOn;
     using PF::isAutoSyncOn;
     using PF::setLogger;
@@ -112,6 +111,10 @@ class ParameterFramework : private parameterFramework::ConfigFiles,
      *  of PF::setValueSpace. */
     void setRawValueSpace(bool enable) { setValueSpace(enable); }
 
+    /** Renaming for better readability (and coherency with PF::isValueSpaceRaw)
+     *  of PF::setValueSpace. */
+    void setHexOutputFormat(bool enable) { setOutputRawFormat(enable); }
+
     /** Wrap PF::setTuningMode to throw an exception on failure. */
     void setTuningMode(bool enable) { mayFailCall(&PF::setTuningMode, enable); }
 

Original file line number	Diff line number	Diff line change
`@@ -275,18 +275,19 @@ bool CConfigurableElement::setSettingsAsBytes(const std::vector<uint8_t> &bytes,`
`275`	`275`	`return true;`
`276`	`276`	`}`
`277`	`277`
`278`		`-void CConfigurableElement::getListOfElementsWithMapping(`
`279`		`- std::list<const CConfigurableElement *> &configurableElementPath) const`
	`278`	`+std::list<const CConfigurableElement *> CConfigurableElement::getConfigurableElementContext() const`
`280`	`279`	`{`
`281`		`- // Check parent`
`282`		`- const CElement *pParent = getParent();`
`283`		`- if (isOfConfigurableElementType(pParent)) {`
	`280`	`+ std::list<const CConfigurableElement *> configurableElementPath;`
`284`	`281`
`285`		`- const CConfigurableElement *pConfigurableElement =`
`286`		`- static_cast<const CConfigurableElement *>(pParent);`
	`282`	`+ const CElement *element = this;`
	`283`	`+ while (element != nullptr and isOfConfigurableElementType(element)) {`
	`284`	`+ auto configurableElement = static_cast<const CConfigurableElement *>(element);`
`287`	`285`
`288`		`- pConfigurableElement->getListOfElementsWithMapping(configurableElementPath);`
	`286`	`+ configurableElementPath.push_back(configurableElement);`
	`287`	`+ element = element->getParent();`
`289`	`288`	`}`
	`289`	`+`
	`290`	`+ return configurableElementPath;`
`290`	`291`	`}`
`291`	`292`
`292`	`293`	`// Used for simulation and virtual subsystems`
Original file line number	Diff line number	Diff line change
`@@ -91,9 +91,10 @@ class PARAMETER_EXPORT ElementHandle`
`91`	`91`
`92`	`92`	`std::vector<ElementHandle> getChildren();`
`93`	`93`
`94`		`- /** Get Mapping Data`
	`94`	`+ /** Get mapping data of the element context`
`95`	`95`	`*`
`96`		`- * Retrieve Mapping data associated to a given key if any`
	`96`	`+ * Retrieve mapping data associated to a given key if any.`
	`97`	`+ * If the key is not present in this element, query ancestors.`
`97`	`98`	`*`
`98`	`99`	`* @param[in] strKey the input mapping key`
`99`	`100`	`* @param[out] strValue the resulting mapping value in case of success`
Original file line number	Diff line number	Diff line change
`@@ -50,6 +50,9 @@ struct Config`
`50`	`50`	`(this->*member) = std::forward<T>(value);`
`51`	`51`	`}`
`52`	`52`
	`53`	`+ /** Mapping attribute of the test subsystem. */`
	`54`	`+ std::string subsystemMapping;`
	`55`	`+`
`53`	`56`	`/** Instances of the test subsystem.`
`54`	`57`	`*`
`55`	`58`	`* Content of the configuration`