search

planetfederal / geoserver-exts

Other
31 stars 40 forks source link

fix wrong test expectation in QueryResourceTest.testStreamsQuery() #38

Closed groldan closed 10 years ago

groldan commented 10 years ago

@dwins when you have a chance take a look at this PR, which fixes the build. The assertion message says it expects two attributes, but it was instead checking for 1, but was correctly getting 2

groldan commented 10 years ago

@dwins any word on this one? this should fix the build in r2d2

dwins commented 10 years ago

Two comments.

  1. Has the test always been failing? I guess I do not get notifications from r2d2 but I also run the test suite locally during development. Could it be skipped for some reason? What changed?
  2. Even with the change applied I get test errors (not failures):
Results :

Tests in error: 
  testFormatParameter(org.opengeo.gsr.resource.QueryResourceTest): java.lang.IllegalStateException
  testGeometryParameter(org.opengeo.gsr.resource.QueryResourceTest): java.lang.IllegalStateException
  testWhere(org.opengeo.gsr.resource.QueryResourceTest): java.lang.IllegalStateException
  testReturnGeometryAndOutFields(org.opengeo.gsr.resource.QueryResourceTest): java.lang.IllegalStateException
  testInSRandOutSR(org.opengeo.gsr.resource.QueryResourceTest): java.lang.IllegalStateException
  testSpatialRel(org.opengeo.gsr.resource.QueryResourceTest): java.lang.IllegalStateException

Is it cause for concern?

groldan commented 10 years ago

I don't know if the specific test always failed. Can check once i'm back to the office soon. The errors you're getting are due to running them with maven 3 instead of maven 2. They should be somehow fixed or the mvn config so we can use maven 3 El mar 20, 2014 12:44 PM, "David Winslow" notifications@github.com escribió:

Two comments.

1.

Has the test always been failing? I guess I do not get notifications from r2d2 but I also run the test suite locally during development. Could it be skipped for some reason? What changed? 2.

Even with the change applied I get test errors (not failures):

Results :

Tests in error: testFormatParameter(org.opengeo.gsr.resource.QueryResourceTest): java.lang.IllegalStateException testGeometryParameter(org.opengeo.gsr.resource.QueryResourceTest): java.lang.IllegalStateException testWhere(org.opengeo.gsr.resource.QueryResourceTest): java.lang.IllegalStateException testReturnGeometryAndOutFields(org.opengeo.gsr.resource.QueryResourceTest): java.lang.IllegalStateException testInSRandOutSR(org.opengeo.gsr.resource.QueryResourceTest): java.lang.IllegalStateException testSpatialRel(org.opengeo.gsr.resource.QueryResourceTest): java.lang.IllegalStateException

Is it cause for concern?

Reply to this email directly or view it on GitHubhttps://github.com/boundlessgeo/geoserver-exts/pull/38#issuecomment-38182474 .

groldan commented 10 years ago

Well I cannot say if it breaks from the start. The test was added at e9c07098 and I can't build that version because maven hangs out trying to download http://repo.opengeo.org/org/geotools/gt-shapefile-renderer/8-SNAPSHOT/maven-metadata.xml, nor I can build geotools 8.x myself cause it has compile errors. So I give up.

Still the issue is pretty clear to me. The test in errot: assertEquals("Points layer should have two non-geometry fields", 1, fields.size()); says it expects 2, but checks for 1, while the actual result is correctly 2. The fields array is [{"name":"id","type":"esriFieldTypeString","alias":"id"}, {"name":"altitude","type":"esriFieldTypeInteger","alias":"altitude"}]

dwins commented 10 years ago

Yes, it should be fine to merge.