zhaih commented on code in PR #12208:
URL: https://github.com/apache/lucene/pull/12208#discussion_r1157998240


##########
lucene/sandbox/src/test/org/apache/lucene/sandbox/search/TestTermAutomatonQuery.java:
##########
@@ -842,6 +844,95 @@ public void testRewriteSimplePhrase() throws Exception {
     IOUtils.close(w, r, dir);
   }
 
+  public void testExplainNonMatchingDocument() throws Exception {
+    TermAutomatonQuery q = new TermAutomatonQuery("field");
+    int initState = q.createState();
+    int s1 = q.createState();
+    int s2 = q.createState();
+    q.addTransition(initState, s1, "xml");
+    q.addTransition(s1, s2, "json");
+    q.setAccept(s2, true);
+    q.finish();
+
+    Directory dir = newDirectory();
+    RandomIndexWriter w = new RandomIndexWriter(random(), dir);
+    Document doc = new Document();
+    doc.add(newTextField("field", "protobuf", Field.Store.NO));
+    w.addDocument(doc);
+
+    IndexReader r = w.getReader();
+    IndexSearcher searcher = newSearcher(r);
+    Query rewrite = q.rewrite(searcher);
+    assertTrue(rewrite instanceof PhraseQuery);
+
+    TopDocs topDocs = searcher.search(rewrite, 10);
+    assertEquals(0, topDocs.totalHits.value);
+
+    for (ScoreDoc scoreDoc : topDocs.scoreDocs) {

Review Comment:
   Since `totalHits` is 0, shouldn't this loop be skipped? Then if we want to 
test no match I think we should just feed doc 0 to the searcher?



##########
lucene/sandbox/src/test/org/apache/lucene/sandbox/search/TestTermAutomatonQuery.java:
##########
@@ -842,6 +844,95 @@ public void testRewriteSimplePhrase() throws Exception {
     IOUtils.close(w, r, dir);
   }
 
+  public void testExplainNonMatchingDocument() throws Exception {
+    TermAutomatonQuery q = new TermAutomatonQuery("field");
+    int initState = q.createState();
+    int s1 = q.createState();
+    int s2 = q.createState();
+    q.addTransition(initState, s1, "xml");
+    q.addTransition(s1, s2, "json");
+    q.setAccept(s2, true);
+    q.finish();
+
+    Directory dir = newDirectory();
+    RandomIndexWriter w = new RandomIndexWriter(random(), dir);
+    Document doc = new Document();
+    doc.add(newTextField("field", "protobuf", Field.Store.NO));
+    w.addDocument(doc);
+
+    IndexReader r = w.getReader();
+    IndexSearcher searcher = newSearcher(r);
+    Query rewrite = q.rewrite(searcher);
+    assertTrue(rewrite instanceof PhraseQuery);
+
+    TopDocs topDocs = searcher.search(rewrite, 10);
+    assertEquals(0, topDocs.totalHits.value);
+
+    for (ScoreDoc scoreDoc : topDocs.scoreDocs) {
+      Explanation explanation = searcher.explain(rewrite, scoreDoc.doc);
+      assertNotNull("Explanation should not be null", explanation);
+      assertTrue(
+          "Explanation score should match the actual score",
+          Float.compare(scoreDoc.score, (Float) explanation.getValue()) == 0);
+    }
+
+    IOUtils.close(w, r, dir);
+  }
+
+  // TODO: improve experience of working with explain
+
+  public void testExplainMatchingDocuments() throws Exception {
+    TermAutomatonQuery q = new TermAutomatonQuery("field");
+
+    int initState = q.createState();
+    int s1 = q.createState();
+    int s2 = q.createState();
+    int s3 = q.createState();
+    q.addTransition(initState, s1, "foo");
+    q.addAnyTransition(s1, s2);
+    q.addTransition(s2, s3, "bar");
+    q.setAccept(s3, true);
+    q.finish();
+
+    Directory dir = newDirectory();
+    RandomIndexWriter w = new RandomIndexWriter(random(), dir);
+    Document doc1 = new Document();
+    doc1.add(newTextField("field", "foo x bar", Field.Store.NO));
+    w.addDocument(doc1);
+
+    Document doc2 = new Document();
+    doc2.add(newTextField("field", "foo y bar", Field.Store.NO));
+    w.addDocument(doc2);
+
+    IndexReader r = w.getReader();
+    IndexSearcher searcher = newSearcher(r);
+
+    QueryUtils.check(random(), q, searcher);
+    TopDocs topDocs = searcher.search(q, 10);
+    // Verify that the docs matched fasho
+    assertEquals(2, topDocs.totalHits.value);
+
+    // Iterate through score docs and assert aspects of explanation
+    for (ScoreDoc scoreDoc : topDocs.scoreDocs) {
+      Explanation explanation = searcher.explain(q, scoreDoc.doc);

Review Comment:
   Can we assert somewhere above that searcher rewrite will not change the 
query type? Since the `searcher.explain` internal will rewrite before it runs 
explain.



##########
lucene/sandbox/src/test/org/apache/lucene/sandbox/search/TestTermAutomatonQuery.java:
##########
@@ -842,6 +844,95 @@ public void testRewriteSimplePhrase() throws Exception {
     IOUtils.close(w, r, dir);
   }
 
+  public void testExplainNonMatchingDocument() throws Exception {
+    TermAutomatonQuery q = new TermAutomatonQuery("field");
+    int initState = q.createState();
+    int s1 = q.createState();
+    int s2 = q.createState();
+    q.addTransition(initState, s1, "xml");
+    q.addTransition(s1, s2, "json");
+    q.setAccept(s2, true);
+    q.finish();
+
+    Directory dir = newDirectory();
+    RandomIndexWriter w = new RandomIndexWriter(random(), dir);
+    Document doc = new Document();
+    doc.add(newTextField("field", "protobuf", Field.Store.NO));
+    w.addDocument(doc);
+
+    IndexReader r = w.getReader();
+    IndexSearcher searcher = newSearcher(r);
+    Query rewrite = q.rewrite(searcher);
+    assertTrue(rewrite instanceof PhraseQuery);
+
+    TopDocs topDocs = searcher.search(rewrite, 10);
+    assertEquals(0, topDocs.totalHits.value);
+
+    for (ScoreDoc scoreDoc : topDocs.scoreDocs) {
+      Explanation explanation = searcher.explain(rewrite, scoreDoc.doc);
+      assertNotNull("Explanation should not be null", explanation);
+      assertTrue(
+          "Explanation score should match the actual score",
+          Float.compare(scoreDoc.score, (Float) explanation.getValue()) == 0);
+    }
+
+    IOUtils.close(w, r, dir);
+  }
+
+  // TODO: improve experience of working with explain

Review Comment:
   I do not get what does this TODO means, do you mean making test explain 
easier?



##########
lucene/sandbox/src/test/org/apache/lucene/sandbox/search/TestTermAutomatonQuery.java:
##########
@@ -842,6 +844,95 @@ public void testRewriteSimplePhrase() throws Exception {
     IOUtils.close(w, r, dir);
   }
 
+  public void testExplainNonMatchingDocument() throws Exception {
+    TermAutomatonQuery q = new TermAutomatonQuery("field");
+    int initState = q.createState();
+    int s1 = q.createState();
+    int s2 = q.createState();
+    q.addTransition(initState, s1, "xml");
+    q.addTransition(s1, s2, "json");
+    q.setAccept(s2, true);
+    q.finish();
+
+    Directory dir = newDirectory();
+    RandomIndexWriter w = new RandomIndexWriter(random(), dir);
+    Document doc = new Document();
+    doc.add(newTextField("field", "protobuf", Field.Store.NO));
+    w.addDocument(doc);
+
+    IndexReader r = w.getReader();
+    IndexSearcher searcher = newSearcher(r);
+    Query rewrite = q.rewrite(searcher);
+    assertTrue(rewrite instanceof PhraseQuery);

Review Comment:
   Since we want to test the `TermAutomatonQuery`'s explain, should we try some 
automaton that will not rewrite to phrase query or multi phrase query?



##########
lucene/sandbox/src/test/org/apache/lucene/sandbox/search/TestTermAutomatonQuery.java:
##########
@@ -842,6 +844,95 @@ public void testRewriteSimplePhrase() throws Exception {
     IOUtils.close(w, r, dir);
   }
 
+  public void testExplainNonMatchingDocument() throws Exception {
+    TermAutomatonQuery q = new TermAutomatonQuery("field");
+    int initState = q.createState();
+    int s1 = q.createState();
+    int s2 = q.createState();
+    q.addTransition(initState, s1, "xml");
+    q.addTransition(s1, s2, "json");
+    q.setAccept(s2, true);
+    q.finish();
+
+    Directory dir = newDirectory();
+    RandomIndexWriter w = new RandomIndexWriter(random(), dir);
+    Document doc = new Document();
+    doc.add(newTextField("field", "protobuf", Field.Store.NO));
+    w.addDocument(doc);
+
+    IndexReader r = w.getReader();
+    IndexSearcher searcher = newSearcher(r);
+    Query rewrite = q.rewrite(searcher);
+    assertTrue(rewrite instanceof PhraseQuery);
+
+    TopDocs topDocs = searcher.search(rewrite, 10);
+    assertEquals(0, topDocs.totalHits.value);
+
+    for (ScoreDoc scoreDoc : topDocs.scoreDocs) {
+      Explanation explanation = searcher.explain(rewrite, scoreDoc.doc);
+      assertNotNull("Explanation should not be null", explanation);
+      assertTrue(
+          "Explanation score should match the actual score",
+          Float.compare(scoreDoc.score, (Float) explanation.getValue()) == 0);
+    }
+
+    IOUtils.close(w, r, dir);
+  }
+
+  // TODO: improve experience of working with explain
+

Review Comment:
   Remove empty line?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Reply via email to