Github user dsmiley commented on a diff in the pull request: https://github.com/apache/lucene-solr/pull/410#discussion_r201076987 --- Diff: solr/core/src/test/org/apache/solr/update/TestNestedUpdateProcessor.java --- @@ -120,25 +122,41 @@ public void before() throws Exception { @Test public void testDeeplyNestedURPGrandChild() throws Exception { + final String[] tests = { + "/response/docs/[0]/id=='" + grandChildId + "'", + "/response/docs/[0]/" + IndexSchema.NEST_PATH_FIELD_NAME + "=='children#0/grandChild#'" + }; indexSampleData(jDoc); - assertJQ(req("q", IndexSchema.NEST_PATH_FIELD_NAME + ":*" + PATH_SEP_CHAR + "grandChild" + NUM_SEP_CHAR + "*" + NUM_SEP_CHAR, + assertJQ(req("q", IndexSchema.NEST_PATH_FIELD_NAME + ":*" + PATH_SEP_CHAR + "grandChild" + NUM_SEP_CHAR + "*", "fl","*", "sort","id desc", "wt","json"), - "/response/docs/[0]/id=='" + grandChildId + "'"); + tests); } @Test public void testDeeplyNestedURPChildren() throws Exception { --- End diff -- This test tests the search behavior more so than literally what the URP is doing. Can you make this more of a unit test around the result of the URP without actually indexing/searching anything? And I would much prefer simpler test assertions that check a complete string value instead of making reference to many variables/constants that need to be concatenated. This makes it plainly clear what the nest path will be; no mental gymnastics are needed to chase down vars/constants to figure it out. I've mentioned before Yonik's advise on avoiding some constants in tests as it helps tests make us aware if in the future we might have a backwards-breaking change; so there are virtues to this way of thinking. It would make this easier to review too.
--- --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org