Mailing-List: contact common-issues-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
Reply-To: common-issues@hadoop.apache.org
Date: Mon, 14 Oct 2013 18:32:42 +0000 (UTC)
From: "Nathan Roberts (JIRA)" <jira@apache.org>
To: common-issues@hadoop.apache.org
Message-ID: <JIRA.12631277.1360250917131.61363.1381775562851@arcas>
In-Reply-To: <JIRA.12631277.1360250917131@arcas>
References: <JIRA.12631277.1360250917131@arcas>
Subject: [jira] [Commented] (HADOOP-9291) enhance unit-test coverage of
 package o.a.h.metrics2
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


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

Nathan Roberts commented on HADOOP-9291:
----------------------------------------

Ivan, thanks for the update. A couple of comments on the patch. 
# In the following code I don't think there is any guarantee on the order of the tags or the metrics (In fact I saw it fail once because testTag2 was emitted first). The "testMetrics2=1" part of the comment is also not correct.
{code}
   // Check the out file content. Should be something like the following:
    //1360244820087 test1.testRecord1: Context=test1, testTag1=testTagValue1, testTag2=testTagValue2, Hostname=myhost, testMetric1=1, testMetric2=1
    //1360244820089 test1.testRecord2: Context=test1, testTag22=testTagValue22, Hostname=myhost
    Pattern expectedContentPattern = Pattern.compile(
       "^\\d+\\s+test1.testRecord1:\\s+Context=test1,\\s+testTag1=testTagValue1," +
       "\\s+testTag2=testTagValue2,\\s+Hostname=.*,\\s+testMetric1=1,\\s+testMetric2=2" +
    	 "$[\\n\\r]*^\\d+\\s+test1.testRecord2:\\s+Context=test1," +
    	 "\\s+testTag22=testTagValue22,\\s+Hostname=.*$[\\n\\r]*", 
    		Pattern.MULTILINE);
    System.out.println(expectedContentPattern.matcher(outFileContent).matches());
    assertTrue(expectedContentPattern.matcher(outFileContent).matches());
{code}

# This is minor - In TestpatternFilter, it might have been nice to have shouldAccept(wl, tags) actually specifically state which elements of the array are supposed to be accepted vs. rejected (currently it just counts the number of accepts/rejects and assumes the right ones were accepted/rejected). I think there are enough test case combinations that we would catch something wrong, but I didn't study it long enough to be 100% convinced of that.

> enhance unit-test coverage of package o.a.h.metrics2
> ----------------------------------------------------
>
>                 Key: HADOOP-9291
>                 URL: https://issues.apache.org/jira/browse/HADOOP-9291
>             Project: Hadoop Common
>          Issue Type: Test
>    Affects Versions: 3.0.0, 2.3.0
>            Reporter: Ivan A. Veselovsky
>            Assignee: Ivan A. Veselovsky
>         Attachments: HADOOP-9291-branch-0.23--N4.patch, HADOOP-9291--N7.patch, HADOOP-9291-trunk--N4.patch, HADOOP-9291-trunk--N5.patch, HADOOP-9291-trunk--N6.patch, HADOOP-9291-trunk--N6.patch
>
>


--
This message was sent by Atlassian JIRA
(v6.1#6144)