+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 <dongjoon.h...@gmail.com>
wrote:
> 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 redundant.
>
> However, I'm +1 for Hyukjin's original suggestion because we had better
> have the official rule for this in some ways.
>
> Thank you again, Hyukjin.
>
> Bests,
> Dongjoon.
>
>
>
> On Tue, Nov 12, 2019 at 1:13 AM Gabor Somogyi <gabor.g.somo...@gmail.com>
> wrote:
>
>> +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 sure there is a reason to introduce it
>> but at the moment can't imagine a use-case where it can help us (want to
>> convert it to daily routine).
>>
>> > This is helpful when the test cases are moved to a different file.
>> The test can be found by name without jira ID
>>
>>
>> On Tue, Nov 12, 2019 at 5:31 AM Hyukjin Kwon <gurwls...@gmail.com> wrote:
>>
>>> 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-XXXX: test name" format.
>>>
>>> Please let me know if you have any different opinion about what/when to
>>> write the JIRA ID as the prefix.
>>> I would like to make sure this simple rule is closer to the actual
>>> practice from you guys.
>>>
>>>
>>> 2019년 11월 12일 (화) 오전 8:41, Gengliang <ltn...@gmail.com>님이 작성:
>>>
>>>> +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 <linguin....@gmail.com>
>>>> wrote:
>>>>
>>>>> +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 <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
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>> --
>>>>> ---
>>>>> Takeshi Yamamuro
>>>>>
>>>>
