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 <[email protected]>: > 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 <[email protected]> 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 > > >
