Github user gerlowskija commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/425#discussion_r204601997
  
    --- Diff: 
solr/core/src/test/org/apache/solr/cloud/TestSolrCloudWithSecureImpersonation.java
 ---
    @@ -223,37 +223,26 @@ private String getExpectedHostExMsg(String user) {
     
       @Test
       public void testProxyNoConfigGroups() throws Exception {
    -    try {
    -      solrClient.request(getProxyRequest("noGroups","bar"));
    -      fail("Expected RemoteSolrException");
    -    }
    -    catch (HttpSolrClient.RemoteSolrException ex) {
    -      
assertTrue(ex.getMessage().contains(getExpectedGroupExMsg("noGroups", "bar")));
    -    }
    +    HttpSolrClient.RemoteSolrException e = 
expectThrows(HttpSolrClient.RemoteSolrException.class,
    +        () -> solrClient.request(getProxyRequest("noGroups","bar"))
    +    );
    +    assertTrue(e.getMessage().contains(getExpectedGroupExMsg("noGroups", 
"bar")));
       }
     
       @Test
       public void testProxyWrongHost() throws Exception {
    -    try {
    -      solrClient.request(getProxyRequest("wrongHost","bar"));
    -      fail("Expected RemoteSolrException");
    -    }
    -    catch (HttpSolrClient.RemoteSolrException ex) {
    -      
assertTrue(ex.getMessage().contains(getExpectedHostExMsg("wrongHost")));
    -    }
    +    HttpSolrClient.RemoteSolrException e = 
expectThrows(HttpSolrClient.RemoteSolrException.class,
    +        () -> solrClient.request(getProxyRequest("wrongHost","bar"))
    +    );
    +    assertTrue(e.getMessage().contains(getExpectedHostExMsg("wrongHost")));
       }
     
       @Test
       public void testProxyNoConfigHosts() throws Exception {
    -    try {
    -      solrClient.request(getProxyRequest("noHosts","bar"));
    -      fail("Expected RemoteSolrException");
    -    }
    -    catch (HttpSolrClient.RemoteSolrException ex) {
    -      // FixMe: this should return an exception about the host being 
invalid,
    -      // but a bug (HADOOP-11077) causes an NPE instead.
    -      
//assertTrue(ex.getMessage().contains(getExpectedHostExMsg("noHosts")));
    -    }
    +    HttpSolrClient.RemoteSolrException e = 
expectThrows(HttpSolrClient.RemoteSolrException.class,
    +        () -> solrClient.request(getProxyRequest("noHosts","bar"))
    +    );
    +    assertTrue(e.getMessage().contains(getExpectedHostExMsg("noHosts")));
    --- End diff --
    
    [-1] I'd prefer this assertion remain commented out (and the attached 
comment stick around as well).
    
    I'm really paranoid about introducing actual test changes in with the 
refactor-only JIRA.  If you'd like to see it fixed, I'm happy to review it as a 
part of a different JIRA/PR.  But I'd rather not have the two mix.


---

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

Reply via email to