Kaydet (Commit) d822953c authored tarafından Mike Kaganski's avatar Mike Kaganski

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: 's avatarMike Kaganski <mike.kaganski@collabora.com>
üst 54fa7c42
...@@ -58,7 +58,7 @@ protected: ...@@ -58,7 +58,7 @@ protected:
* Get the position of the child named rName of the parent node specified by rXPath. * Get the position of the child named rName of the parent node specified by rXPath.
* Useful for checking relative order of elements. * 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. * Assert that rXPath exists, and returns exactly one node.
*/ */
......
...@@ -4680,12 +4680,10 @@ void SwUiWriterTest::testBookmarkCollapsed() ...@@ -4680,12 +4680,10 @@ void SwUiWriterTest::testBookmarkCollapsed()
const OString aPath("/office:document-content/office:body/office:text/text:p"); const OString aPath("/office:document-content/office:body/office:text/text:p");
const int pos1 = getXPathPosition(pXmlDoc, aPath, "bookmark"); 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(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() ...@@ -4758,11 +4756,10 @@ void SwUiWriterTest::testRemoveBookmarkText()
{ {
const OString aPath("/office:document-content/office:body/office:text/text:p"); 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 pos2 = getXPathPosition(pXmlDoc, aPath, "bookmark-start");
const int pos3 = getXPathPosition(pXmlDoc, aPath, "bookmark-end"); 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(0, pos2); // found, and it is first
CPPUNIT_ASSERT_EQUAL(1, pos3); // found, and it is second CPPUNIT_ASSERT_EQUAL(1, pos3); // found, and it is second
} }
...@@ -4865,12 +4862,11 @@ void SwUiWriterTest::testRemoveBookmarkTextAndAddNew() ...@@ -4865,12 +4862,11 @@ void SwUiWriterTest::testRemoveBookmarkTextAndAddNew()
{ {
const OString aPath("/office:document-content/office:body/office:text/text:p"); 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 pos2 = getXPathPosition(pXmlDoc, aPath, "bookmark-start");
const int pos3 = getXPathPosition(pXmlDoc, aPath, "text"); const int pos3 = getXPathPosition(pXmlDoc, aPath, "text");
const int pos4 = getXPathPosition(pXmlDoc, aPath, "bookmark-end"); const int pos4 = getXPathPosition(pXmlDoc, aPath, "bookmark-end");
CPPUNIT_ASSERT_EQUAL(4, pos1); // not found
CPPUNIT_ASSERT_EQUAL(0, pos2); CPPUNIT_ASSERT_EQUAL(0, pos2);
CPPUNIT_ASSERT_EQUAL(1, pos3); CPPUNIT_ASSERT_EQUAL(1, pos3);
CPPUNIT_ASSERT_EQUAL(2, pos4); CPPUNIT_ASSERT_EQUAL(2, pos4);
...@@ -4935,13 +4931,11 @@ void SwUiWriterTest::testRemoveBookmarkTextAndAddNewAfterReload() ...@@ -4935,13 +4931,11 @@ void SwUiWriterTest::testRemoveBookmarkTextAndAddNewAfterReload()
const int pos1 = getXPathPosition(pXmlDoc, aPath, "bookmark"); const int pos1 = getXPathPosition(pXmlDoc, aPath, "bookmark");
const int pos2 = getXPathPosition(pXmlDoc, aPath, "text"); 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(0, pos1);
CPPUNIT_ASSERT_EQUAL(1, pos2); 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() ...@@ -6852,9 +6846,9 @@ void SwUiWriterTest::testInconsistentBookmark()
{ {
const OString aPath("/office:document-content/office:body/office:text/text:p"); const OString aPath("/office:document-content/office:body/office:text/text:p");
const OUString aTagBookmarkStart("bookmark-start"); const OString aTagBookmarkStart("bookmark-start");
const OUString aTagControl("control"); const OString aTagControl("control");
const OUString aTagBookmarkEnd("bookmark-end"); const OString aTagBookmarkEnd("bookmark-end");
const int pos1 = getXPathPosition(pXmlDoc, aPath, aTagBookmarkStart); const int pos1 = getXPathPosition(pXmlDoc, aPath, aTagBookmarkStart);
const int pos2 = getXPathPosition(pXmlDoc, aPath, aTagControl); const int pos2 = getXPathPosition(pXmlDoc, aPath, aTagControl);
......
...@@ -33,6 +33,8 @@ CPPUNIT_TEST_FIXTURE(TestXPath, test_getXPath) ...@@ -33,6 +33,8 @@ CPPUNIT_TEST_FIXTURE(TestXPath, test_getXPath)
CPPUNIT_ASSERT_EQUAL(OUString(), getXPath(pTable, "/xml/item", "")); CPPUNIT_ASSERT_EQUAL(OUString(), getXPath(pTable, "/xml/item", ""));
// Must properly return attribute content // Must properly return attribute content
CPPUNIT_ASSERT_EQUAL(OUString("val"), getXPath(pTable, "/xml/item", "attrib")); 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(); CPPUNIT_PLUGIN_IMPLEMENT();
......
...@@ -28,6 +28,11 @@ OUString convert(xmlChar const * string) { ...@@ -28,6 +28,11 @@ OUString convert(xmlChar const * string) {
return s; return s;
} }
OString oconvert(xmlChar const * string)
{
return reinterpret_cast<char const *>(string);
}
} }
XmlTestTools::XmlTestTools() XmlTestTools::XmlTestTools()
...@@ -227,7 +232,7 @@ void XmlTestTools::assertXPathNoAttribute(xmlDocPtr pXmlDoc, const OString& rXPa ...@@ -227,7 +232,7 @@ void XmlTestTools::assertXPathNoAttribute(xmlDocPtr pXmlDoc, const OString& rXPa
xmlXPathFreeObject(pXmlObj); 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); xmlXPathObjectPtr pXmlObj = getXPathNode(pXmlDoc, rXPath);
xmlNodeSetPtr pXmlNodes = pXmlObj->nodesetval; xmlNodeSetPtr pXmlNodes = pXmlObj->nodesetval;
...@@ -236,13 +241,21 @@ int XmlTestTools::getXPathPosition(xmlDocPtr pXmlDoc, const OString& rXPath, con ...@@ -236,13 +241,21 @@ int XmlTestTools::getXPathPosition(xmlDocPtr pXmlDoc, const OString& rXPath, con
xmlXPathNodeSetGetLength(pXmlNodes)); xmlXPathNodeSetGetLength(pXmlNodes));
xmlNodePtr pXmlNode = pXmlNodes->nodeTab[0]; xmlNodePtr pXmlNode = pXmlNodes->nodeTab[0];
int nRet = 0; int nRet = 0;
bool bFound = false;
for (xmlNodePtr pChild = pXmlNode->children; pChild; pChild = pChild->next) for (xmlNodePtr pChild = pXmlNode->children; pChild; pChild = pChild->next)
{ {
if (convert(pChild->name) == rChildName) if (oconvert(pChild->name) == rChildName)
{
bFound = true;
break; break;
}
++nRet; ++nRet;
} }
xmlXPathFreeObject(pXmlObj); xmlXPathFreeObject(pXmlObj);
CPPUNIT_ASSERT_MESSAGE(OString("In <" + OString(pXmlDoc->name) + ">, XPath '" + rXPath
+ "' child '" + rChildName + "' not found")
.getStr(),
bFound);
return nRet; return nRet;
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment