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
>>>>
>>>
