[Geoserver-devel] Spotted a couple tests apparently passing by accident

GeoServer GeoServer Developer
1

Hi,
I believe I’ve spotted a couple of tests in WMS that are working by accident, and have
to do with the local catalog functionality.

The two tests are here:
https://github.com/geoserver/geoserver/blob/master/src/wms/src/test/java/org/geoserver/wms/wms_1_1_1/CapabilitiesTest.java#L140

https://github.com/geoserver/geoserver/blob/master/src/wms/src/test/java/org/geoserver/wms/wms_1_1_1/CapabilitiesTest.java#L160

In both cases we are working against a local workspace service, so layers should not be workspace qualified,
and indeed the output is not, but the XPath used is invalid and the tests pass just by accident:

assertTrue(xpath.getMatchingNodes(“//Layer/Name[starts-with(., cite:Forests)]”, dom)
.getLength() == 1);

The intention of the test as written would be to check that there is one layer named cite:Forests, there
are two issues:

  • the xpath is invalid, it should be //Layer/Name[starts-with(., ‘cite:Forests’)] (strings are always between ’ ’ in xpath)
  • once fixed, the result is 0, because we are in a workspace local service so the name is just Forest

The testWorkspaceQualified xpaths are affected the same way.

Here is a patch to fix those:

diff --git a/src/wms/src/test/java/org/geoserver/wms/wms_1_1_1/CapabilitiesTest.java b/src/wms/src/test/java/org/geoserver/wms/wms_1_1_1/CapabilitiesTest.java
index f7f8573…4799b89 100644
— a/src/wms/src/test/java/org/geoserver/wms/wms_1_1_1/CapabilitiesTest.java
+++ b/src/wms/src/test/java/org/geoserver/wms/wms_1_1_1/CapabilitiesTest.java
@@ -142,9 +142,9 @@
Element e = dom.getDocumentElement();
assertEquals(“WMT_MS_Capabilities”, e.getLocalName());
XpathEngine xpath = XMLUnit.newXpathEngine();

  • assertTrue(xpath.getMatchingNodes(“//Layer/Name[starts-with(., cite)]”, dom).getLength() > 0);
  • assertEquals(0, xpath.getMatchingNodes(“//Layer/Name[not(starts-with(., cite))]”, dom)
  • .getLength());
  • assertEquals(0, xpath.getMatchingNodes(“//Layer/Name[starts-with(., ‘cite’)]”, dom).getLength());
  • assertTrue (xpath.getMatchingNodes(“//Layer/Name[not(starts-with(., ‘cite’))]”, dom)
  • .getLength() > 0 );

NodeList nodes = xpath.getMatchingNodes(“//Layer//OnlineResource”, dom);
assertTrue(nodes.getLength() > 0);
@@ -162,8 +162,10 @@
Element e = dom.getDocumentElement();
assertEquals(“WMT_MS_Capabilities”, e.getLocalName());
XpathEngine xpath = XMLUnit.newXpathEngine();

  • assertTrue(xpath.getMatchingNodes(“//Layer/Name[starts-with(., cite:Forests)]”, dom)
  • .getLength() == 1);
  • assertEquals(0, xpath.getMatchingNodes(“//Layer/Name[starts-with(., ‘cite:Forests’)]”, dom)
  • .getLength());
  • assertEquals(1, xpath.getMatchingNodes(“//Layer[Name = ‘Forests’]”, dom)
  • .getLength());
    assertEquals(1, xpath.getMatchingNodes(“//Layer//Layer”, dom).getLength());

NodeList nodes = xpath.getMatchingNodes(“//Layer//OnlineResource”, dom);

Ok to go and fix?

Cheers
Andrea

–

==
Our support, Your Success! Visit http://opensdi.geo-solutions.it for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

2

Yup, seems like an oversight. Thanks for catching it.

On Wed, Nov 21, 2012 at 3:41 AM, Andrea Aime <andrea.aime@anonymised.com> wrote:

Hi,
I believe I’ve spotted a couple of tests in WMS that are working by accident, and have
to do with the local catalog functionality.

The two tests are here:
https://github.com/geoserver/geoserver/blob/master/src/wms/src/test/java/org/geoserver/wms/wms_1_1_1/CapabilitiesTest.java#L140

https://github.com/geoserver/geoserver/blob/master/src/wms/src/test/java/org/geoserver/wms/wms_1_1_1/CapabilitiesTest.java#L160

In both cases we are working against a local workspace service, so layers should not be workspace qualified,
and indeed the output is not, but the XPath used is invalid and the tests pass just by accident:

assertTrue(xpath.getMatchingNodes(“//Layer/Name[starts-with(., cite:Forests)]”, dom)
.getLength() == 1);

The intention of the test as written would be to check that there is one layer named cite:Forests, there
are two issues:

  • the xpath is invalid, it should be //Layer/Name[starts-with(., ‘cite:Forests’)] (strings are always between ’ ’ in xpath)
  • once fixed, the result is 0, because we are in a workspace local service so the name is just Forest

The testWorkspaceQualified xpaths are affected the same way.

Here is a patch to fix those:

diff --git a/src/wms/src/test/java/org/geoserver/wms/wms_1_1_1/CapabilitiesTest.java b/src/wms/src/test/java/org/geoserver/wms/wms_1_1_1/CapabilitiesTest.java
index f7f8573…4799b89 100644
— a/src/wms/src/test/java/org/geoserver/wms/wms_1_1_1/CapabilitiesTest.java
+++ b/src/wms/src/test/java/org/geoserver/wms/wms_1_1_1/CapabilitiesTest.java
@@ -142,9 +142,9 @@
Element e = dom.getDocumentElement();
assertEquals(“WMT_MS_Capabilities”, e.getLocalName());
XpathEngine xpath = XMLUnit.newXpathEngine();

  • assertTrue(xpath.getMatchingNodes(“//Layer/Name[starts-with(., cite)]”, dom).getLength() > 0);
  • assertEquals(0, xpath.getMatchingNodes(“//Layer/Name[not(starts-with(., cite))]”, dom)
  • .getLength());
  • assertEquals(0, xpath.getMatchingNodes(“//Layer/Name[starts-with(., ‘cite’)]”, dom).getLength());
  • assertTrue (xpath.getMatchingNodes(“//Layer/Name[not(starts-with(., ‘cite’))]”, dom)
  • .getLength() > 0 );

NodeList nodes = xpath.getMatchingNodes(“//Layer//OnlineResource”, dom);
assertTrue(nodes.getLength() > 0);
@@ -162,8 +162,10 @@
Element e = dom.getDocumentElement();
assertEquals(“WMT_MS_Capabilities”, e.getLocalName());
XpathEngine xpath = XMLUnit.newXpathEngine();

  • assertTrue(xpath.getMatchingNodes(“//Layer/Name[starts-with(., cite:Forests)]”, dom)
  • .getLength() == 1);
  • assertEquals(0, xpath.getMatchingNodes(“//Layer/Name[starts-with(., ‘cite:Forests’)]”, dom)
  • .getLength());
  • assertEquals(1, xpath.getMatchingNodes(“//Layer[Name = ‘Forests’]”, dom)
  • .getLength());
    assertEquals(1, xpath.getMatchingNodes(“//Layer//Layer”, dom).getLength());

NodeList nodes = xpath.getMatchingNodes(“//Layer//OnlineResource”, dom);

Ok to go and fix?

Cheers
Andrea

–

==
Our support, Your Success! Visit http://opensdi.geo-solutions.it for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

Monitor your physical, virtual and cloud infrastructure from a single
web console. Get in-depth insight into apps, servers, databases, vmware,
SAP, cloud infrastructure, etc. Download 30-day Free Trial.
Pricing starts from $795 for 25 servers or applications!
http://p.sf.net/sfu/zoho_dev2dev_nov

Geoserver-devel mailing list
Geoserver-devel@anonymised.comsts.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

–
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.

3

On Wed, Nov 21, 2012 at 4:11 PM, Justin Deoliveira <jdeolive@anonymised.com> wrote:

Yup, seems like an oversight. Thanks for catching it.

Patch applied :slight_smile:

Cheers
Andrea

–

==
Our support, Your Success! Visit http://opensdi.geo-solutions.it for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it