spark-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Hyukjin Kwon <gurwls...@gmail.com>
Subject Re: Adding JIRA ID as the prefix for the test case name
Date Thu, 14 Nov 2019 12:03:38 GMT
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