spark-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Shixiong(Ryan) Zhu" <shixi...@databricks.com>
Subject Re: Adding JIRA ID as the prefix for the test case name
Date Thu, 14 Nov 2019 18:13:14 GMT
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 <gurwls223@gmail.com> wrote:

> I opened a PR - https://github.com/apache/spark-website/pull/231
>
> 2019년 11월 13일 (수) 오전 10:43, Hyukjin Kwon <gurwls223@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 <srowen@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 <gurwls223@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
>>> >
>>> >
>>> >
>>>
>>

Mime
View raw message