dsmiley commented on code in PR #4676:
URL: https://github.com/apache/solr/pull/4676#discussion_r3661950963


##########
solr/core/src/test/org/apache/solr/jersey/PostRequestDecorationFilterTest.java:
##########


Review Comment:
   TBH... I have little faith in unit testing low level plumbing like this.  
IMO they even have negative value as it's yet another thing to change if we 
change the plumbing.  It's better to accomplish the high level goals (like what 
led you to uncover this bug).  My comment applies to all your tests here.  Just 
because you write a line of code doesn't mean it needs a direct test.



##########
solr/core/src/java/org/apache/solr/jersey/PostRequestDecorationFilter.java:
##########
@@ -55,6 +55,10 @@ public void filter(
     }
     final SolrQueryRequest solrQueryRequest =
         (SolrQueryRequest) requestContext.getProperty(SOLR_QUERY_REQUEST);
+    if (solrQueryRequest == null) {
+      log.debug("Skipping QTime assignment because no SolrQueryRequest was 
attached");

Review Comment:
   trace



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to