@David Smiley <[email protected]> It would seem to me that
TestSpatialFilter would be fine with no mention of the port in the name.
It's a confusing identifier.
As for TestGeo3dShapeWGS84ModelRectRelation, there are lots of comments.
Not sure what the value of this comment is in the class
/*
[junit4] 1> S-R Rel: {}, Shape {}, Rectangle {} lap# {}
[CONTAINS, Geo3dShape{planetmodel=PlanetModel:
{xyScaling=1.0011188180710464, zScaling=0.9977622539852008},
shape=GeoPath: {planetmodel=PlanetModel:
{xyScaling=1.0011188180710464, zScaling=0.9977622539852008},
width=1.53588974175501(87.99999999999999),
points={[[X=0.12097657665150223, Y=-0.6754177666095532,
Z=0.7265376136709238], [X=-0.3837892785614207, Y=0.4258049113530899,
Z=0.8180007850434892]]}}},
Rect(minX=4.0,maxX=36.0,minY=16.0,maxY=16.0), 6981](no slf4j subst; sorry)
[junit4] FAILURE 0.59s | Geo3dWGS84ShapeRectRelationTest.testGeoPathRect <<<
[junit4] > Throwable #1: java.lang.AssertionError:
Geo3dShape{planetmodel=PlanetModel: {xyScaling=1.0011188180710464,
zScaling=0.9977622539852008}, shape=GeoPath: {planetmodel=PlanetModel:
{xyScaling=1.0011188180710464, zScaling=0.9977622539852008},
width=1.53588974175501(87.99999999999999),
points={[[X=0.12097657665150223, Y=-0.6754177666095532,
Z=0.7265376136709238], [X=-0.3837892785614207, Y=0.4258049113530899,
Z=0.8180007850434892]]}}} intersect Pt(x=23.81626064835212,y=16.0)
[junit4] > at
__randomizedtesting.SeedInfo.seed([2595268DA3F13FEA:6CC30D8C83453E5D]:0)
[junit4] > at
org.apache.lucene.spatial.spatial4j.RandomizedShapeTestCase._assertIntersect(RandomizedShapeTestCase.java:168)
[junit4] > at
org.apache.lucene.spatial.spatial4j.RandomizedShapeTestCase.assertRelation(RandomizedShapeTestCase.java:153)
[junit4] > at
org.apache.lucene.spatial.spatial4j.RectIntersectionTestHelper.testRelateWithRectangle(RectIntersectionTestHelper.java:128)
[junit4] > at
org.apache.lucene.spatial.spatial4j.Geo3dWGS84ShapeRectRelationTest.testGeoPathRect(Geo3dWGS84ShapeRectRelationTest.java:265)
*/
This comment either. It just seems like these are stream of conscious
notes and maybe they should be captured in the relevant tickets, which
could be referenced. Do you think it should be in the actual source
code?
// Rectangle contains point
//assertTrue(rect.isWithin(pt));
// Path contains point (THIS FAILS)
//assertTrue(path.isWithin(pt));
// What happens: (1) The center point of the horizontal line is within
the path, in fact within a radius of one of the endpoints.
// (2) The point mentioned is NOT inside either SegmentEndpoint.
// (3) The point mentioned is NOT inside the path segment, either. (I
think it should be...)
On Mon, Nov 2, 2020 at 7:00 AM David Smiley <[email protected]> wrote:
> Hi Marcus,
>
> PortSolr3Test is documented "Based off of Solr 3's SpatialFilterTest".
> Why do you propose removing it? A quick gloss over it suggests to me this
> test is a rather straight-forward test to understand and maintain, and
> should be fast. Perhaps the class name should have been something else,
> and consider it's heritage as an implementation detail that is only worthy
> of a comment? But then name it what? IMO it's fine.
>
> TestGeo3dShapeWGS84ModelRectRelation: What about it? This test class is
> 99% implemented by it's subclass -- ShapeRectRelationTestCase. The
> subclass provides the Geo3d context to its superclass, and the rest is
> handled from there. The tests explicitly on this subclass are for some
> regressions.
>
> > Is there anyone keeping a list of test cases that we can get rid of or
> significantly refactor today?
>
> For Solr -- yes (sorta): SOLR-11872
> <https://issues.apache.org/jira/browse/SOLR-11872>. That would be a
> major make-over, but wouldn't really change the number of tests; it'd make
> them easier to maintain and more flexible. There is another issue,
> SOLR-10229,
> pertaining to Solr tests ought to configure Solr themselves and rely less
> on static test config files, which are a mess to maintain.
>
> ~ David Smiley
> Apache Lucene/Solr Search Developer
> http://www.linkedin.com/in/davidwsmiley
>
>
> On Sun, Nov 1, 2020 at 5:21 PM Marcus Eagan <[email protected]> wrote:
>
>> Hi Community,
>>
>> Lately I have been reading a lot of test files in an attempt to
>> understand what they seek to accomplish. Specifically, what stability and
>> reliability assurance does a given test class provide. In short, I have
>> found some test files that I am unsure are required to provide any of the
>> expected guarantees of the project today.
>>
>> It is more possible that I am misreading or don’t know all the history to
>> opine, and I don’t want to waste anyone’s time with a ticket without first
>> raising a discussion here. Below, I’ll include a few examples from Lucene.
>> As of today, I fully intend to step through many of the test files from
>> Solr as well for a related effort, but I started with Lucene because I have
>> ~800 more classes in Solr to review/modify/flag for review and because
>> there is a fast-changing reference impl out there.
>>
>> The first example is the PortSolr3Test class. It seems relevant because
>> it tests some currently relevant cases, but the name suggests that it might
>> not be.
>>
>> Another class I read that has lots of cruft that I don't really could use
>> some guidance one on is the TestGeo3dShapeWGS84ModelRectRelation class.
>> Is it possible there are lots of test classes that are no longer necessary
>> given changes over the versions.
>>
>> Is there anyone keeping a list of test cases that we can get rid of or
>> significantly refactor today?
>>
>> Please advise,
>>
>> Marcus
>>
>>
--
Marcus Eagan
