[ 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)