You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pig.apache.org by Cheolsoo Park <pi...@gmail.com> on 2014/02/28 05:07:28 UTC
Review Request 18607: PIG-3784: Port more mini cluster tests to Tez
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18607/
-----------------------------------------------------------
Review request for pig, Daniel Dai and Rohini Palaniswamy.
Repository: pig-git
Description
-------
This patch ports the following unit tests to Tez:
* TestPigContext.java
* TestPigStorage.java
* TestNestedForeach.java
* TestEvalPipeline.java
* TestPigServer.java
The changes include-
1) Made all the mini-cluster tests run in both MR and Tez mode. I didn't convert any local mode tests to mini cluster tests.
2) Deleted tests that are related to ComparisonFunc in TestEvalPipeline because ComparisonFunc has been depreciated since 0.7 (http://wiki.apache.org/pig/Pig070IncompatibleChanges). In fact, I will open a jira to completely remove ComparisonFunc from trunk.
3) Moved testNonExistingSecondDirectoryInSkewJoin that was added by PIG-3469 from TestPigServer to TestSkewedJoin. In addition, the test wasn't complete itself, so I improved it.
4) Fixed warnings, whitespaces and indentations while touching these files. Please hide whitespace changes if that's distracting for review.
Diffs
-----
test/org/apache/pig/test/TestEvalPipeline.java 4d2fc23
test/org/apache/pig/test/TestNestedForeach.java 352b3eb
test/org/apache/pig/test/TestPigContext.java 7a660ee
test/org/apache/pig/test/TestPigServer.java 3fc1d76
test/org/apache/pig/test/TestPigStorage.java b30fc00
test/org/apache/pig/test/TestSkewedJoin.java 0a7ba63
test/tez-tests 287a0b5
Diff: https://reviews.apache.org/r/18607/diff/
Testing
-------
All the tests pass in both tez and mr mode.
Thanks,
Cheolsoo Park
Re: Review Request 18607: PIG-3784: Port more mini cluster tests to Tez
Posted by Cheolsoo Park <pi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18607/
-----------------------------------------------------------
(Updated Feb. 28, 2014, 11:50 p.m.)
Review request for pig, Daniel Dai and Rohini Palaniswamy.
Changes
-------
Incorporate Rohini's comment.
Repository: pig-git
Description
-------
This patch ports the following unit tests to Tez:
* TestPigContext.java
* TestPigStorage.java
* TestNestedForeach.java
* TestEvalPipeline.java
* TestPigServer.java
The changes include-
1) Made all the mini-cluster tests run in both MR and Tez mode. I didn't convert any local mode tests to mini cluster tests.
2) Deleted tests that are related to ComparisonFunc in TestEvalPipeline because ComparisonFunc has been depreciated since 0.7 (http://wiki.apache.org/pig/Pig070IncompatibleChanges). In fact, I will open a jira to completely remove ComparisonFunc from trunk.
3) Moved testNonExistingSecondDirectoryInSkewJoin that was added by PIG-3469 from TestPigServer to TestSkewedJoin. In addition, the test wasn't complete itself, so I improved it.
4) Fixed warnings, whitespaces and indentations while touching these files. Please hide whitespace changes if that's distracting for review.
Diffs (updated)
-----
src/org/apache/pig/PigServer.java 4e52b24
test/org/apache/pig/test/TestCombiner.java 286a802
test/org/apache/pig/test/TestCustomPartitioner.java 6019ac8
test/org/apache/pig/test/TestEvalPipeline.java 4d2fc23
test/org/apache/pig/test/TestNestedForeach.java 352b3eb
test/org/apache/pig/test/TestPigContext.java 7a660ee
test/org/apache/pig/test/TestPigServer.java 3fc1d76
test/org/apache/pig/test/TestPigStorage.java b30fc00
test/org/apache/pig/test/TestSkewedJoin.java 0a7ba63
test/org/apache/pig/test/TestSplitStore.java 1855fe0
test/tez-tests 287a0b5
Diff: https://reviews.apache.org/r/18607/diff/
Testing
-------
All the tests pass in both tez and mr mode.
Thanks,
Cheolsoo Park
Re: Review Request 18607: PIG-3784: Port more mini cluster tests to Tez
Posted by Cheolsoo Park <pi...@gmail.com>.
> On Feb. 28, 2014, 9:15 p.m., Rohini Palaniswamy wrote:
> > test/org/apache/pig/test/TestEvalPipeline.java, line 117
> > <https://reviews.apache.org/r/18607/diff/1/?file=506725#file506725line117>
> >
> > Are the double changes in multiple places needed?
We don't have to, but junit deprecated assertEquals(double, double), so I was replacing it with assertEquals(double, double, double)-
http://junit.sourceforge.net/javadoc/org/junit/Assert.html#assertEquals(double, double)
> On Feb. 28, 2014, 9:15 p.m., Rohini Palaniswamy wrote:
> > test/org/apache/pig/test/TestPigServer.java, lines 112-113
> > <https://reviews.apache.org/r/18607/diff/1/?file=506728#file506728line112>
> >
> > Can we do this in PigServer.java instead of nullifying in each test?
> >
> > - if (PigStats.get() == null) {
> > - PigStats.start(pigContext.getExecutionEngine().instantiatePigStats());
> > - }
> > -
> > - if (ScriptState.get() == null) {
> > - ScriptState.start(pigContext.getExecutionEngine().instantiateScriptState());
> > - }
> > + PigStats.start(pigContext.getExecutionEngine().instantiatePigStats());
> > + ScriptState.start(pigContext.getExecutionEngine().instantiateScriptState());
That's a good idea. Let me try that.
- Cheolsoo
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18607/#review35838
-----------------------------------------------------------
On Feb. 28, 2014, 4:07 a.m., Cheolsoo Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18607/
> -----------------------------------------------------------
>
> (Updated Feb. 28, 2014, 4:07 a.m.)
>
>
> Review request for pig, Daniel Dai and Rohini Palaniswamy.
>
>
> Repository: pig-git
>
>
> Description
> -------
>
> This patch ports the following unit tests to Tez:
> * TestPigContext.java
> * TestPigStorage.java
> * TestNestedForeach.java
> * TestEvalPipeline.java
> * TestPigServer.java
>
> The changes include-
> 1) Made all the mini-cluster tests run in both MR and Tez mode. I didn't convert any local mode tests to mini cluster tests.
> 2) Deleted tests that are related to ComparisonFunc in TestEvalPipeline because ComparisonFunc has been depreciated since 0.7 (http://wiki.apache.org/pig/Pig070IncompatibleChanges). In fact, I will open a jira to completely remove ComparisonFunc from trunk.
> 3) Moved testNonExistingSecondDirectoryInSkewJoin that was added by PIG-3469 from TestPigServer to TestSkewedJoin. In addition, the test wasn't complete itself, so I improved it.
> 4) Fixed warnings, whitespaces and indentations while touching these files. Please hide whitespace changes if that's distracting for review.
>
>
> Diffs
> -----
>
> test/org/apache/pig/test/TestEvalPipeline.java 4d2fc23
> test/org/apache/pig/test/TestNestedForeach.java 352b3eb
> test/org/apache/pig/test/TestPigContext.java 7a660ee
> test/org/apache/pig/test/TestPigServer.java 3fc1d76
> test/org/apache/pig/test/TestPigStorage.java b30fc00
> test/org/apache/pig/test/TestSkewedJoin.java 0a7ba63
> test/tez-tests 287a0b5
>
> Diff: https://reviews.apache.org/r/18607/diff/
>
>
> Testing
> -------
>
> All the tests pass in both tez and mr mode.
>
>
> Thanks,
>
> Cheolsoo Park
>
>
Re: Review Request 18607: PIG-3784: Port more mini cluster tests to Tez
Posted by Rohini Palaniswamy <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18607/#review35838
-----------------------------------------------------------
test/org/apache/pig/test/TestEvalPipeline.java
<https://reviews.apache.org/r/18607/#comment66609>
Are the double changes in multiple places needed?
test/org/apache/pig/test/TestPigServer.java
<https://reviews.apache.org/r/18607/#comment66622>
Can we do this in PigServer.java instead of nullifying in each test?
- if (PigStats.get() == null) {
- PigStats.start(pigContext.getExecutionEngine().instantiatePigStats());
- }
-
- if (ScriptState.get() == null) {
- ScriptState.start(pigContext.getExecutionEngine().instantiateScriptState());
- }
+ PigStats.start(pigContext.getExecutionEngine().instantiatePigStats());
+ ScriptState.start(pigContext.getExecutionEngine().instantiateScriptState());
- Rohini Palaniswamy
On Feb. 28, 2014, 4:07 a.m., Cheolsoo Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18607/
> -----------------------------------------------------------
>
> (Updated Feb. 28, 2014, 4:07 a.m.)
>
>
> Review request for pig, Daniel Dai and Rohini Palaniswamy.
>
>
> Repository: pig-git
>
>
> Description
> -------
>
> This patch ports the following unit tests to Tez:
> * TestPigContext.java
> * TestPigStorage.java
> * TestNestedForeach.java
> * TestEvalPipeline.java
> * TestPigServer.java
>
> The changes include-
> 1) Made all the mini-cluster tests run in both MR and Tez mode. I didn't convert any local mode tests to mini cluster tests.
> 2) Deleted tests that are related to ComparisonFunc in TestEvalPipeline because ComparisonFunc has been depreciated since 0.7 (http://wiki.apache.org/pig/Pig070IncompatibleChanges). In fact, I will open a jira to completely remove ComparisonFunc from trunk.
> 3) Moved testNonExistingSecondDirectoryInSkewJoin that was added by PIG-3469 from TestPigServer to TestSkewedJoin. In addition, the test wasn't complete itself, so I improved it.
> 4) Fixed warnings, whitespaces and indentations while touching these files. Please hide whitespace changes if that's distracting for review.
>
>
> Diffs
> -----
>
> test/org/apache/pig/test/TestEvalPipeline.java 4d2fc23
> test/org/apache/pig/test/TestNestedForeach.java 352b3eb
> test/org/apache/pig/test/TestPigContext.java 7a660ee
> test/org/apache/pig/test/TestPigServer.java 3fc1d76
> test/org/apache/pig/test/TestPigStorage.java b30fc00
> test/org/apache/pig/test/TestSkewedJoin.java 0a7ba63
> test/tez-tests 287a0b5
>
> Diff: https://reviews.apache.org/r/18607/diff/
>
>
> Testing
> -------
>
> All the tests pass in both tez and mr mode.
>
>
> Thanks,
>
> Cheolsoo Park
>
>