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

Jonathan Coveney commented on PIG-2975:
---------------------------------------

Koji,

It's been a pleasure :)

Ok, I'm king nitpick, but my last nitpick is to statically import 
assertEquals() (and any other junit methods) instead of calling 
Assert.whatever. Why? I'm slowly but surely trying to promote a consistent 
style in all of the unit tests.

As far as the annoying l1 etc variable, I'll let it slide ;) You're right that 
it's a common pattern, but I think it's a bad one. That said: this isn't the 
place to fix that.

Also, your tests are great, though a few more comments could be helpful. 
"compareTwoObjects" is a bit misleading, for example. I know not all of the 
tests have great comments (I'm probably guilty of this myself), but a line or 
two per would go a long way. Be the commit you want to see in the world and all 
that jazz!

In the meantime, I'm going to make sure the tests run, and make sure that the 
unit tests fail on trunk (ie that it is isolating the issue).

Thanks for being patient with ME, Koji
Jon
                
> TestTypedMap.testOrderBy failing with incorrect result 
> -------------------------------------------------------
>
>                 Key: PIG-2975
>                 URL: https://issues.apache.org/jira/browse/PIG-2975
>             Project: Pig
>          Issue Type: Sub-task
>    Affects Versions: 0.11
>            Reporter: Koji Noguchi
>            Assignee: Koji Noguchi
>            Priority: Blocker
>             Fix For: 0.11
>
>         Attachments: PIG-2975-0_jco.patch, PIG-2975-0_jco-v2.patch, 
> pig-2975-trunk_v01.txt, pig-2975-trunk_v02-broken.txt, 
> pig-2975-trunk_v03-unionapproach.txt, pig-2975-trunk_v04-purerawcompare.txt, 
> pig-2975-trunk_v05-BinInterSedesRawComparatorAndlightweight-withouttest.txt, 
> pig-2975-trunk_v05-BinInterSedesRawComparatorAndlightweight-withtest.txt
>
>
> Looked at 
> {noformat}
> junit.framework.AssertionFailedError
>     at org.apache.pig.test.TestTypedMap.testOrderBy(TestTypedMap.java:352)
> {noformat}
> This looks like a valid test case failing with incorrect result.
> {noformat}
> % cat test/orderby.txt
> [key#1,key9#23]
> [key#3,key3#2]
> [key#22]
> % cat test/orderby.pig
> a = load 'test/orderby.txt' as (m:[]);
> b = foreach a generate m#'key' as b0;
> dump b;
> c = order b by b0;
> dump c;
> % java ... org.apache.pig.Main    -x local test/orderby.pig 
> [dump b]
> (1)
> (3)
> (22)
> ...
> [dump c]
> (1)
> (1)
> (22)
> %
> where did the '(3)' go?
> {noformat}

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to