You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@impala.apache.org by Jin Chul Kim <ji...@gmail.com> on 2017/12/14 01:13:29 UTC

A couple of questions

Hi,

Would you please answer the questions? Thanks.

1. Regarding the decorator "pytest.mark.execute_serially", I saw all the
test cases are marked as execute_serially in
tests/shell/test_shell_interactive.py. I guess a few tests should be run
exclusively, but the other tests do not require the serial option. What do
you think about this? I think we should consider to use the decorator when
adding test cases. Minimized serial run can help to reduce overall test
running time by parallelism.

I think the following cases do not require the decorator.
a. Check consistency from test result
b. Test query cancellation
c. Test shell options which are effective on local shell

2. Regarding coding convention, especially function name, in Java, I am
confusing which function name is right: lowerCamelCase or CamelCase. I
guess we may follow lowerCamelCase:
https://google.github.io/styleguide/javaguide.html#s5.2.3-method-names
By the way, some codes did not follow the convention. Please see
fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java#L113,126. I
could see similar case in some files. Do we have to align function name?
What do you think about this?

Best regards,
Jinchul

Re: A couple of questions

Posted by Jin Chul Kim <ji...@gmail.com>.
Hi Tim,

Thanks for your answer. As you confirmed, the issues are valid. The
following tickets have been filed:
- IMPALA-6335 <https://issues.apache.org/jira/browse/IMPALA-6335>: Remove
the unnecessary decorator "pytest.mark.execute_serially" from tests which
can be run in parallel
- IMPALA-6336 <https://issues.apache.org/jira/browse/IMPALA-6336>: Follow
code convention for function method in fe

Best regards,
Jinchul

2017-12-16 8:45 GMT+09:00 Tim Armstrong <ta...@cloudera.com>:

> 1. I think you're right that many of the shell tests don't inherently
> require to be executed serially. Some of them would require work to execute
> in parallel, particularly the ones that inspect files like .impalahistory
> and tests that check the values of global impala daemon metrics.
>
> 2. Yes, Java should use lowerCamelCase. C++ uses UpperCamelCase. This does
> seem inconsistent but is inherited from the Google styles.
>
> I agree that some functions in Java are not following the convention, e.g.
> AnalyzesOk(). I don't know why they are different. It would be nice to fix
> them to be consistent but I'd be concerned about the merge conflicts
> resulting.
>
> On Wed, Dec 13, 2017 at 5:13 PM, Jin Chul Kim <ji...@gmail.com> wrote:
>
> > Hi,
> >
> > Would you please answer the questions? Thanks.
> >
> > 1. Regarding the decorator "pytest.mark.execute_serially", I saw all the
> > test cases are marked as execute_serially in
> > tests/shell/test_shell_interactive.py. I guess a few tests should be run
> > exclusively, but the other tests do not require the serial option. What
> do
> > you think about this? I think we should consider to use the decorator
> when
> > adding test cases. Minimized serial run can help to reduce overall test
> > running time by parallelism.
> >
> > I think the following cases do not require the decorator.
> > a. Check consistency from test result
> > b. Test query cancellation
> > c. Test shell options which are effective on local shell
> >
> > 2. Regarding coding convention, especially function name, in Java, I am
> > confusing which function name is right: lowerCamelCase or CamelCase. I
> > guess we may follow lowerCamelCase:
> > https://google.github.io/styleguide/javaguide.html#s5.2.3-method-names
> > By the way, some codes did not follow the convention. Please see
> > fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java#L113,126.
> I
> > could see similar case in some files. Do we have to align function name?
> > What do you think about this?
> >
> > Best regards,
> > Jinchul
> >
>

Re: A couple of questions

Posted by Tim Armstrong <ta...@cloudera.com>.
1. I think you're right that many of the shell tests don't inherently
require to be executed serially. Some of them would require work to execute
in parallel, particularly the ones that inspect files like .impalahistory
and tests that check the values of global impala daemon metrics.

2. Yes, Java should use lowerCamelCase. C++ uses UpperCamelCase. This does
seem inconsistent but is inherited from the Google styles.

I agree that some functions in Java are not following the convention, e.g.
AnalyzesOk(). I don't know why they are different. It would be nice to fix
them to be consistent but I'd be concerned about the merge conflicts
resulting.

On Wed, Dec 13, 2017 at 5:13 PM, Jin Chul Kim <ji...@gmail.com> wrote:

> Hi,
>
> Would you please answer the questions? Thanks.
>
> 1. Regarding the decorator "pytest.mark.execute_serially", I saw all the
> test cases are marked as execute_serially in
> tests/shell/test_shell_interactive.py. I guess a few tests should be run
> exclusively, but the other tests do not require the serial option. What do
> you think about this? I think we should consider to use the decorator when
> adding test cases. Minimized serial run can help to reduce overall test
> running time by parallelism.
>
> I think the following cases do not require the decorator.
> a. Check consistency from test result
> b. Test query cancellation
> c. Test shell options which are effective on local shell
>
> 2. Regarding coding convention, especially function name, in Java, I am
> confusing which function name is right: lowerCamelCase or CamelCase. I
> guess we may follow lowerCamelCase:
> https://google.github.io/styleguide/javaguide.html#s5.2.3-method-names
> By the way, some codes did not follow the convention. Please see
> fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java#L113,126. I
> could see similar case in some files. Do we have to align function name?
> What do you think about this?
>
> Best regards,
> Jinchul
>