Make getXPathPosition assert on requested child found

Previously, for an XML like this:

    <sharedItems>
        <d v="2017-07-10T09:11:02"/>
        <d v="2017-07-10T09:11:03"/>
        <m/>
    </sharedItems>

the call like

    int pos = getXPathPosition(pDoc, "/x:sharedItems", "absent");

gave 3. That could result in mistakes, when a test would assert
on position "3" for a child element which name is mistyped.

I made such a mistake when creating a unit test trying to assert on
a position of the last element, and writing its name as "x:m", like
in rXPath itself; the return was 3, and I initially wrongly assumed
that the return is 1-based (like in xpath bracketed expressions).

rChildName made const OString&, for consistency with rXPath, or with
rAttribute in getXPath: child name is just a part of a longer xpath.

Change-Id: I7ba9c4466c75b1b10fce1ccf26ef3b56b4e11e87
Reviewed-on: https://gerrit.libreoffice.org/71614
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>

4 files changed, 29 insertions, 20 deletions

diff --git a/include/test/xmltesttools.hxx b/include/test/xmltesttools.hxx
index c24a81eb65d6..f8f10da387bd 100644
--- a/include/test/xmltesttools.hxx
+++ b/include/test/xmltesttools.hxx
@@ -58,7 +58,7 @@ protected:
      * Get the position of the child named rName of the parent node specified by rXPath.
      * Useful for checking relative order of elements.
      */
-    int           getXPathPosition(xmlDocPtr pXmlDoc, const OString& rXPath, const OUString& rChildName);
+    int           getXPathPosition(xmlDocPtr pXmlDoc, const OString& rXPath, const OString& rChildName);
     /**
      * Assert that rXPath exists, and returns exactly one node.
      */
diff --git a/sw/qa/extras/uiwriter/uiwriter.cxx b/sw/qa/extras/uiwriter/uiwriter.cxx
index d6795ead6620..aa32c236eaab 100644
--- a/sw/qa/extras/uiwriter/uiwriter.cxx
+++ b/sw/qa/extras/uiwriter/uiwriter.cxx
@@ -4680,12 +4680,10 @@ void SwUiWriterTest::testBookmarkCollapsed()
         const OString aPath("/office:document-content/office:body/office:text/text:p");
 
         const int pos1 = getXPathPosition(pXmlDoc, aPath, "bookmark");
-        const int pos2 = getXPathPosition(pXmlDoc, aPath, "bookmark-start");
-        const int pos3 = getXPathPosition(pXmlDoc, aPath, "bookmark-end");
-
         CPPUNIT_ASSERT_EQUAL(0, pos1); // found, and it is first
-        CPPUNIT_ASSERT_EQUAL(2, pos2); // not found
-        CPPUNIT_ASSERT_EQUAL(2, pos3); // not found
+
+        CPPUNIT_ASSERT_ASSERTION_FAIL(getXPathPosition(pXmlDoc, aPath, "bookmark-start")); // not found
+        CPPUNIT_ASSERT_ASSERTION_FAIL(getXPathPosition(pXmlDoc, aPath, "bookmark-end")); // not found
     }
 }
 
@@ -4758,11 +4756,10 @@ void SwUiWriterTest::testRemoveBookmarkText()
     {
         const OString aPath("/office:document-content/office:body/office:text/text:p");
 
-        const int pos1 = getXPathPosition(pXmlDoc, aPath, "bookmark");
+        CPPUNIT_ASSERT_ASSERTION_FAIL(getXPathPosition(pXmlDoc, aPath, "bookmark")); // not found
         const int pos2 = getXPathPosition(pXmlDoc, aPath, "bookmark-start");
         const int pos3 = getXPathPosition(pXmlDoc, aPath, "bookmark-end");
 
-        CPPUNIT_ASSERT_EQUAL(3, pos1); // not found
         CPPUNIT_ASSERT_EQUAL(0, pos2); // found, and it is first
         CPPUNIT_ASSERT_EQUAL(1, pos3); // found, and it is second
     }
@@ -4865,12 +4862,11 @@ void SwUiWriterTest::testRemoveBookmarkTextAndAddNew()
     {
         const OString aPath("/office:document-content/office:body/office:text/text:p");
 
-        const int pos1 = getXPathPosition(pXmlDoc, aPath, "bookmark");
+        CPPUNIT_ASSERT_ASSERTION_FAIL(getXPathPosition(pXmlDoc, aPath, "bookmark")); // not found
         const int pos2 = getXPathPosition(pXmlDoc, aPath, "bookmark-start");
         const int pos3 = getXPathPosition(pXmlDoc, aPath, "text");
         const int pos4 = getXPathPosition(pXmlDoc, aPath, "bookmark-end");
 
-        CPPUNIT_ASSERT_EQUAL(4, pos1); // not found
         CPPUNIT_ASSERT_EQUAL(0, pos2);
         CPPUNIT_ASSERT_EQUAL(1, pos3);
         CPPUNIT_ASSERT_EQUAL(2, pos4);
@@ -4935,13 +4931,11 @@ void SwUiWriterTest::testRemoveBookmarkTextAndAddNewAfterReload()
         const int pos1 = getXPathPosition(pXmlDoc, aPath, "bookmark");
         const int pos2 = getXPathPosition(pXmlDoc, aPath, "text");
 
-        const int pos3 = getXPathPosition(pXmlDoc, aPath, "bookmark-start");
-        const int pos4 = getXPathPosition(pXmlDoc, aPath, "bookmark-end");
-
         CPPUNIT_ASSERT_EQUAL(0, pos1);
         CPPUNIT_ASSERT_EQUAL(1, pos2);
-        CPPUNIT_ASSERT_EQUAL(2, pos3); // not found
-        CPPUNIT_ASSERT_EQUAL(2, pos4); // not found
+
+        CPPUNIT_ASSERT_ASSERTION_FAIL(getXPathPosition(pXmlDoc, aPath, "bookmark-start")); // not found
+        CPPUNIT_ASSERT_ASSERTION_FAIL(getXPathPosition(pXmlDoc, aPath, "bookmark-end")); // not found
     }
 }
 
@@ -6852,9 +6846,9 @@ void SwUiWriterTest::testInconsistentBookmark()
         {
             const OString aPath("/office:document-content/office:body/office:text/text:p");
 
-            const OUString aTagBookmarkStart("bookmark-start");
-            const OUString aTagControl("control");
-            const OUString aTagBookmarkEnd("bookmark-end");
+            const OString aTagBookmarkStart("bookmark-start");
+            const OString aTagControl("control");
+            const OString aTagBookmarkEnd("bookmark-end");
 
             const int pos1 = getXPathPosition(pXmlDoc, aPath, aTagBookmarkStart);
             const int pos2 = getXPathPosition(pXmlDoc, aPath, aTagControl);
diff --git a/test/qa/cppunit/test_xpath.cxx b/test/qa/cppunit/test_xpath.cxx
index 83b48c84f488..a5693d1c9f76 100644
--- a/test/qa/cppunit/test_xpath.cxx
+++ b/test/qa/cppunit/test_xpath.cxx
@@ -33,6 +33,8 @@ CPPUNIT_TEST_FIXTURE(TestXPath, test_getXPath)
     CPPUNIT_ASSERT_EQUAL(OUString(), getXPath(pTable, "/xml/item", ""));
     // Must properly return attribute content
     CPPUNIT_ASSERT_EQUAL(OUString("val"), getXPath(pTable, "/xml/item", "attrib"));
+    // Trying to get position of missing child of a node must fail assertion
+    CPPUNIT_ASSERT_ASSERTION_FAIL(getXPathPosition(pTable, "/xml/item", "absent"));
 }
 
 CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/test/source/xmltesttools.cxx b/test/source/xmltesttools.cxx
index 20149cc03c37..3c186dd08d63 100644
--- a/test/source/xmltesttools.cxx
+++ b/test/source/xmltesttools.cxx
@@ -28,6 +28,11 @@ OUString convert(xmlChar const * string) {
     return s;
 }
 
+OString oconvert(xmlChar const * string)
+{
+    return reinterpret_cast<char const *>(string);
+}
+
 }
 
 XmlTestTools::XmlTestTools()
@@ -227,7 +232,7 @@ void XmlTestTools::assertXPathNoAttribute(xmlDocPtr pXmlDoc, const OString& rXPa
     xmlXPathFreeObject(pXmlObj);
 }
 
-int XmlTestTools::getXPathPosition(xmlDocPtr pXmlDoc, const OString& rXPath, const OUString& rChildName)
+int XmlTestTools::getXPathPosition(xmlDocPtr pXmlDoc, const OString& rXPath, const OString& rChildName)
 {
     xmlXPathObjectPtr pXmlObj = getXPathNode(pXmlDoc, rXPath);
     xmlNodeSetPtr pXmlNodes = pXmlObj->nodesetval;
@@ -236,13 +241,21 @@ int XmlTestTools::getXPathPosition(xmlDocPtr pXmlDoc, const OString& rXPath, con
                                  xmlXPathNodeSetGetLength(pXmlNodes));
     xmlNodePtr pXmlNode = pXmlNodes->nodeTab[0];
     int nRet = 0;
+    bool bFound = false;
     for (xmlNodePtr pChild = pXmlNode->children; pChild; pChild = pChild->next)
     {
-        if (convert(pChild->name) == rChildName)
+        if (oconvert(pChild->name) == rChildName)
+        {
+            bFound = true;
             break;
+        }
         ++nRet;
     }
     xmlXPathFreeObject(pXmlObj);
+    CPPUNIT_ASSERT_MESSAGE(OString("In <" + OString(pXmlDoc->name) + ">, XPath '" + rXPath
+                                   + "' child '" + rChildName + "' not found")
+                               .getStr(),
+        bFound);
     return nRet;
 }