Re: Adding JIRA ID as the prefix for the test case name

2019-11-21 Thread Hyukjin Kwon
I opened a PR - https://github.com/apache/spark-website/pull/232 2019년 11월 19일 (화) 오전 9:22, Hyukjin Kwon 님이 작성: > Let me document as below in few days: > > 1. For Python and Java, write a single comment that starts with JIRA ID > and short description, e.g. (SPARK-X: test blah blah) > 2. For

Re: Adding JIRA ID as the prefix for the test case name

2019-11-18 Thread Hyukjin Kwon
Let me document as below in few days: 1. For Python and Java, write a single comment that starts with JIRA ID and short description, e.g. (SPARK-X: test blah blah) 2. For R, use JIRA ID as a prefix for its test name. assuming everybody is happy. 2019년 11월 18일 (월) 오전 11:36, Hyukjin Kwon 님이

Re: Adding JIRA ID as the prefix for the test case name

2019-11-17 Thread Hyukjin Kwon
Actually there are not so many Java test cases in Spark (because Scala runs on JVM as everybody knows)[1]. Given that, I think we can avoid to put some efforts on this for now .. I don't mind if somebody wants to give a shot since it looks good anyway but to me I wouldn't spend so much time on

Re: Adding JIRA ID as the prefix for the test case name

2019-11-17 Thread Steve Loughran
Test reporters do often contain some assumptions about the characters in the test methods. Historically JUnit XML reporters have never sanitised the method names so XML injection attacks have been fairly trivial. Haven't tried this for a while. That whole JUnit XML report "standard" was actually

Re: Adding JIRA ID as the prefix for the test case name

2019-11-16 Thread Hyukjin Kwon
DisplayName looks good in general but actually here I would like first to find a existing pattern to document in guidelines given the actual existing practice we all are used to. I'm trying to be very conservative since this guidelines affect everybody. I think it might be better to discuss

Re: Adding JIRA ID as the prefix for the test case name

2019-11-15 Thread Steve Loughran
Junit5: Display names. Goes all the way to the XML. https://junit.org/junit5/docs/current/user-guide/#writing-tests-display-names On Thu, Nov 14, 2019 at 6:13 PM Shixiong(Ryan) Zhu wrote: > Should we also add a guideline for non Scala tests? Other languages (Java, > Python, R) don't support

Re: Adding JIRA ID as the prefix for the test case name

2019-11-14 Thread Felix Cheung
this is about test description and not test file name right? if yes I don’t see a problem. From: Hyukjin Kwon Sent: Thursday, November 14, 2019 6:03:02 PM To: Shixiong(Ryan) Zhu Cc: dev ; Felix Cheung ; Shivaram Venkataraman Subject: Re: Adding JIRA ID as the

Re: Adding JIRA ID as the prefix for the test case name

2019-11-14 Thread Hyukjin Kwon
Yeah, sounds good to have it. In case of R, it seems not quite common to write down JIRA ID [1] but looks some have the prefix in its test name in general. In case of Python and Java, seems we time to time write a JIRA ID in the comment right under the test method [2][3]. Given this pattern, I

Re: Adding JIRA ID as the prefix for the test case name

2019-11-14 Thread Shixiong(Ryan) Zhu
Should we also add a guideline for non Scala tests? Other languages (Java, Python, R) don't support using string as a test name. Best Regards, Ryan On Thu, Nov 14, 2019 at 4:04 AM Hyukjin Kwon wrote: > I opened a PR - https://github.com/apache/spark-website/pull/231 > > 2019년 11월 13일 (수) 오전

Re: Adding JIRA ID as the prefix for the test case name

2019-11-14 Thread Hyukjin Kwon
I opened a PR - https://github.com/apache/spark-website/pull/231 2019년 11월 13일 (수) 오전 10:43, Hyukjin Kwon 님이 작성: > > In general a test should be self descriptive and I don't think we should > be adding JIRA ticket references wholesale. Any action that the reader has > to take to understand why a

Re: Adding JIRA ID as the prefix for the test case name

2019-11-12 Thread Hyukjin Kwon
> In general a test should be self descriptive and I don't think we should be adding JIRA ticket references wholesale. Any action that the reader has to take to understand why a test was introduced is one too many. However in some cases the thing we are trying to test is very subtle and in that

Re: Adding JIRA ID as the prefix for the test case name

2019-11-12 Thread Sean Owen
Let's suggest "SPARK-12345:" but not go back and change a bunch of test cases. I'd add this only when a test specifically targets a certain issue. It's a nice-to-have, not super essential, just because in the rare case you need to understand why a test asserts something, you can go back and find

Re: Adding JIRA ID as the prefix for the test case name

2019-11-12 Thread Xin Ren
+1 Two confusions to clarify: 1. what if multiple JIRA IDs relating to the same test? we just take the very first JIRA ID? 2. are we going to have a full scan of all existing tests and attach a JIRA ID to it? Thank you Hyukjin :) On Tue, Nov 12, 2019 at 1:47 PM Dongjoon Hyun wrote: > Thank

Re: Adding JIRA ID as the prefix for the test case name

2019-11-12 Thread Dongjoon Hyun
Thank you for the suggestion, Hyukjin. Previously, we added Jira IDs for the bug fix PR test cases as Gabor said. For the new features (and improvements), we didn't add them because all test cases in the newly added test suite share the same prefix JIRA ID in that case. It might looks

Re: Adding JIRA ID as the prefix for the test case name

2019-11-12 Thread Gabor Somogyi
+1 for having that consistent rule in test names. +1 for making it a guideline. +1 defining exact guides in general. Until now I've followed the alternative (only add the prefix when the JIRA's type is bug) and that way I knew that such tests contain edge cases. In case of new features I'm pretty

Re: Adding JIRA ID as the prefix for the test case name

2019-11-11 Thread Hyukjin Kwon
In few days, I will wrote this in our guidelines probably after rewording it a bit better: 1. Add a prefix into a test name when a PR adds a couple of tests. 2. Uses "SPARK-: test name" format. Please let me know if you have any different opinion about what/when to write the JIRA ID as the

Re: Adding JIRA ID as the prefix for the test case name

2019-11-11 Thread Gengliang
+1 for making it a guideline. This is helpful when the test cases are moved to a different file. On Mon, Nov 11, 2019 at 3:23 PM Takeshi Yamamuro wrote: > +1 for having that consistent rule in test names. > This is a trivial problem though, I think documenting this rule in the > contribution

Re: Adding JIRA ID as the prefix for the test case name

2019-11-11 Thread Takeshi Yamamuro
+1 for having that consistent rule in test names. This is a trivial problem though, I think documenting this rule in the contribution guide might be able to make reviewer overhead a little smaller. Bests, Takeshi On Tue, Nov 12, 2019 at 1:46 AM Hyukjin Kwon wrote: > Hi all, > > Maybe it's not

Adding JIRA ID as the prefix for the test case name

2019-11-11 Thread Hyukjin Kwon
Hi all, Maybe it's not a big deal but it brought some confusions time to time into Spark dev and community. I think it's time to discuss about when/which format to add a JIRA ID as a prefix for the test case name in Scala test cases. Currently we have many test case names with prefixes as below: