> On Aug. 14, 2013, 2:13 a.m., Cheolsoo Park wrote:
> > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestGrunt.java,
> >  line 1083
> > <https://reviews.apache.org/r/13535/diff/2/?file=340613#file340613line1083>
> >
> >     I believe we shouldn't remove xargs. It was added by PIG-3099 to avoid 
> > some race condition.

I think it was a mistake. The test fails even before this patch with xargs. The 
touch command will not produce any output. So it does not make sense to have 
xargs after a touch.


> On Aug. 14, 2013, 2:13 a.m., Cheolsoo Park wrote:
> > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestPigServer.java,
> >  line 776
> > <https://reviews.apache.org/r/13535/diff/2/?file=340614#file340614line776>
> >
> >     This is a bit confusing to me. 10 - 4 + 6 = 12, but numTimesInitiated 
> > is set to 10.
> >     
> >     _testSkipParseInRegisterForBatch(false, 10, 4);

It is 10 (4 + 6). Will change the hyphen to full stop so that it is not 
mistaken as minus.


> On Aug. 14, 2013, 2:13 a.m., Cheolsoo Park wrote:
> > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestPigServer.java,
> >  lines 804-805
> > <https://reviews.apache.org/r/13535/diff/2/?file=340614#file340614line804>
> >
> >     If I understand correctly, this is equivalent to calling the followings:
> >     
> >     GruntParser grunt = new GruntParser(in);
> >     grunt.setInteractive(false);
> >     grunt.setParams(pigServer);
> >     grunt.parseStopOnError(false); //batch
> >     
> >     Can you explicitly call them, so it will be easier to identify the 
> > difference when skipParseInRegisterForBatch is on and off?

I added a comment. Left it at grunt.exec() as script execution from Main.java 
is through that call and if anything changes in that method would reflect in 
this test. 


- Rohini


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13535/#review25098
-----------------------------------------------------------


On Aug. 13, 2013, 2:34 p.m., Rohini Palaniswamy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13535/
> -----------------------------------------------------------
> 
> (Updated Aug. 13, 2013, 2:34 p.m.)
> 
> 
> Review request for pig.
> 
> 
> Bugs: PIG-3204
>     https://issues.apache.org/jira/browse/PIG-3204
> 
> 
> Repository: pig
> 
> 
> Description
> -------
> 
> Change parsing from line by line to whole script at once.
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/PigServer.java 
> 1510960 
>   
> http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/tools/grunt/GruntParser.java
>  1510960 
>   
> http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestGrunt.java
>  1510960 
>   
> http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestPigServer.java
>  1510960 
> 
> Diff: https://reviews.apache.org/r/13535/diff/
> 
> 
> Testing
> -------
> 
> New unit tests added to track the number of times a line is parsed. TestGrunt 
> and TestShortcuts test failures fixed.
> 
> 
> Thanks,
> 
> Rohini Palaniswamy
> 
>

Reply via email to