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