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 would like to suggest use the same format but: 1. For Python and Java, write a single comment that starts with JIRA ID and short description, e.g. (SPARK-XXXXX: test blah blah) 2. For R, use JIRA ID as a prefix for its test name. [1] git grep -r "SPARK-" -- '*test*.R' [2] git grep -r "SPARK-" -- '*Suite.java' [3] git grep -r "SPARK-" -- '*test*.py' Does that make sense? Adding Felix and Shivaram too. 2019년 11월 15일 (금) 오전 3:13, Shixiong(Ryan) Zhu <shixi...@databricks.com>님이 작성: > 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 <gurwls...@gmail.com> wrote: > >> I opened a PR - https://github.com/apache/spark-website/pull/231 >> >> 2019년 11월 13일 (수) 오전 10:43, Hyukjin Kwon <gurwls...@gmail.com>님이 작성: >> >>> > 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 case a reference to a JIRA ticket might be useful, I do still feel >>> that this should be a backstop and that properly documenting your tests is >>> a much better way of dealing with this. >>> >>> Yeah, the test should be self-descriptive. I don't think adding a JIRA >>> prefix harms this point. Probably I should add this sentence in the >>> guidelines as well. >>> Adding a JIRA prefix just adds one extra hint to track down details. I >>> think it's fine to stick to this practice and make it simpler and clear to >>> follow. >>> >>> > 1. what if multiple JIRA IDs relating to the same test? we just take >>> the very first JIRA ID? >>> Ideally one JIRA should describe one issue and one PR should fix one >>> JIRA with a dedicated test. >>> Yeah, I think I would 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? >>> Yea, let's don't do this. >>> >>> > It's a nice-to-have, not super essential, just because ... >>> It's been asked multiple times and each committer seems having a >>> different understanding on this. >>> It's not a biggie but wanted to make it clear and conclude this. >>> >>> > I'd add this only when a test specifically targets a certain issue. >>> Yes, so this one I am not sure. From what I heard, people adds the JIRA >>> in cases below: >>> >>> - Whenever the JIRA type is a bug >>> - When a PR adds a couple of tests >>> - Only when a test specifically targets a certain issue. >>> - ... >>> >>> Which one do we prefer and simpler to follow? >>> >>> Or I can combine as below (im gonna reword when I actually document >>> this): >>> 1. In general, we should add a JIRA ID as prefix of a test when a PR >>> targets to fix a specific issue. >>> In practice, it usually happens when a JIRA type is a bug or a PR >>> adds a couple of tests. >>> 2. Uses "SPARK-XXXX: test name" format >>> >>> If we have no objection with ^, let me go with this. >>> >>> 2019년 11월 13일 (수) 오전 8:14, Sean Owen <sro...@gmail.com>님이 작성: >>> >>>> 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 what added it in the git history without much trouble. >>>> >>>> On Mon, Nov 11, 2019 at 10:46 AM Hyukjin Kwon <gurwls...@gmail.com> >>>> wrote: >>>> > >>>> > 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: >>>> > >>>> > test("SPARK-XXXXX blah blah") >>>> > test("SPARK-XXXXX: blah blah") >>>> > test("SPARK-XXXXX - blah blah") >>>> > test("[SPARK-XXXXX] blah blah") >>>> > … >>>> > >>>> > It is a good practice to have the JIRA ID in general because, for >>>> instance, >>>> > it makes us put less efforts to track commit histories (or even when >>>> the files >>>> > are totally moved), or to track related information of tests failed. >>>> > Considering Spark's getting big, I think it's good to document. >>>> > >>>> > I would like to suggest this and document it in our guideline: >>>> > >>>> > 1. Add a prefix into a test name when a PR adds a couple of tests. >>>> > 2. Uses "SPARK-XXXX: test name" format which is used in our code base >>>> most >>>> > often[1]. >>>> > >>>> > We should make it simple and clear but closer to the actual practice. >>>> So, I would like to listen to what other people think. I would appreciate >>>> if you guys give some feedback about when to add the JIRA prefix. One >>>> alternative is that, we only add the prefix when the JIRA's type is bug. >>>> > >>>> > [1] >>>> > git grep -E 'test\("\SPARK-([0-9]+):' | wc -l >>>> > 923 >>>> > git grep -E 'test\("\SPARK-([0-9]+) ' | wc -l >>>> > 477 >>>> > git grep -E 'test\("\[SPARK-([0-9]+)\]' | wc -l >>>> > 16 >>>> > git grep -E 'test\("\SPARK-([0-9]+) -' | wc -l >>>> > 13 >>>> > >>>> > >>>> > >>>> >>>
