[ 
https://issues.apache.org/jira/browse/METRON-1299?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16352433#comment-16352433
 ] 

ASF GitHub Bot commented on METRON-1299:
----------------------------------------

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

    https://github.com/apache/metron/pull/924#discussion_r165982935
  
    --- Diff: 
metron-platform/metron-common/src/test/java/org/apache/metron/common/error/MetronErrorTest.java
 ---
    @@ -53,7 +52,14 @@ public void getJSONObjectShouldReturnBasicInformation() {
         assertEquals(Constants.ErrorType.PARSER_ERROR.getType(), 
errorJSON.get(Constants.ErrorFields.ERROR_TYPE.getName()));
         assertEquals("error", errorJSON.get(Constants.SENSOR_TYPE));
         assertEquals("sensorType", 
errorJSON.get(Constants.ErrorFields.FAILED_SENSOR_TYPE.getName()));
    -    assertTrue(((String) 
errorJSON.get(Constants.ErrorFields.HOSTNAME.getName())).length() > 0);
    +
    +    try {
    +      String hostName = InetAddress.getLocalHost().getHostName();
    --- End diff --
    
    So, in the instance where this happens, is hostName is null or is it that 
there's an Exception thrown?  If it's null, can't we do a:
    ```
    if(hostName == null) {
      LOG.warn("Unable to resolve local hostname, skipping validation."
    }
    else {
      assertTrue(((String) 
errorJSON.get(Constants.ErrorFields.HOSTNAME.getName())).length() > 0);
      assertEquals(hostName, (String) 
errorJSON.get(Constants.ErrorFields.HOSTNAME.getName()));
    }
    ```
    
    I ask because I worry about extraneous *other* exceptions cropping up and 
us ignoring them.  It's a very small nit, let me know what you think. :)


> MetronError tests fail if hostname isn't set
> --------------------------------------------
>
>                 Key: METRON-1299
>                 URL: https://issues.apache.org/jira/browse/METRON-1299
>             Project: Metron
>          Issue Type: Bug
>    Affects Versions: 0.4.1
>         Environment: openSUSE Tumbleweed 20171102, OpenJDK 1.8.0_144
>            Reporter: Stuart Bertram
>            Assignee: Otto Fowler
>            Priority: Major
>
> If I run "mvn package" in the root Metron directory then compilation fails 
> because of a null reference in  
> {{./metron-platform/metron-common/src/test/java/org/apache/metron/common/error/MetronErrorTest.java}}.
>  This happens in {{getJSONObjectShouldReturnBasicInformation}} on line 56 
> because  {{errorJSON.get(Constants.ErrorFields.HOSTNAME.getName())}} is 
> assumed to return a string and the {{length()}} method is called on it. 
> Because the assert doesn't use a message then no obvious reason why it fails 
> is logged.
> The underlying problem is that 
> {{metron-platform/metron-common/src/main/java/org/apache/metron/common/error/MetronError.java}}
>  falls through to a {{catch}} block in {{addHostname()}} when 
> {{netAddress.getLocalHost().getHostName()}} exceptions with {{Name or service 
> not known}} for the host name.
> Setting a hostname that resolves is a build requirement that is out of 
> Metron's control, but if the code specifically handles the fact that it won't 
> always be retrieved then it seems problematic to have tests that assume it 
> works _and_ not make it clear what the failure is when it occurs.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to