Adding new test cases to TestBuiltin.java

[Cheolsoo Park <ch...@cloudera.com>]

Hi all,

While reviewing PIG-2881 (Add SUBTRACT eval func), I had 2 questions:

1) How do we decide whether an eval func be a built-in func? For example,
should SUBTRACT be added to the o.a.pig.builtin or piggybank?
2) Do we want to continue to add test cases to TestBuiltin.java for new
built-in eval funcs? That file is now ~3k line long, so I was wondering
whether we should start breaking it into smaller files.

Currently, Joel's patch for PIG-2881 is adding the new eval func as a
built-in and creating its own new test suite rather than adding test cases
to TestBuiltin.java. I wanted to know what other people think before
committing his patch.
https://issues.apache.org/jira/browse/PIG-2881?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13487241

Thanks!
Cheolsoo

Re: Adding new test cases to TestBuiltin.java

[Julien Le Dem <ju...@twitter.com>]

When adding a udf (say package.MyUDF) or any other class the test
should be named after the class in the same package
(package.TestMyUDF) in the test folder.
All new tests should follow this pattern.
It's not always 1-1 Class - TestClass but that should be the general idea
Julien

On Tue, Oct 30, 2012 at 2:43 PM, Cheolsoo Park <ch...@cloudera.com> wrote:
> Hi all,
>
> While reviewing PIG-2881 (Add SUBTRACT eval func), I had 2 questions:
>
> 1) How do we decide whether an eval func be a built-in func? For example,
> should SUBTRACT be added to the o.a.pig.builtin or piggybank?
> 2) Do we want to continue to add test cases to TestBuiltin.java for new
> built-in eval funcs? That file is now ~3k line long, so I was wondering
> whether we should start breaking it into smaller files.
>
> Currently, Joel's patch for PIG-2881 is adding the new eval func as a
> built-in and creating its own new test suite rather than adding test cases
> to TestBuiltin.java. I wanted to know what other people think before
> committing his patch.
> https://issues.apache.org/jira/browse/PIG-2881?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13487241
>
> Thanks!
> Cheolsoo

Re: Adding new test cases to TestBuiltin.java

[Alan Gates <ga...@hortonworks.com>]

On Oct 30, 2012, at 2:43 PM, Cheolsoo Park wrote:

> Hi all,
> 
> While reviewing PIG-2881 (Add SUBTRACT eval func), I had 2 questions:
> 
> 1) How do we decide whether an eval func be a built-in func? For example,
> should SUBTRACT be added to the o.a.pig.builtin or piggybank?

The question here is what do we as Pig want to support.  Things in piggybank are best effort.  Things in Pig itself we are committing to keep working and upgrading moving forward.  If we believe a function makes sense for a large enough group of users and that we can maintain it, then we should move it into Pig.

Alan.