Re: [PATCH 1/3] stash: add test for stash create with no files
Joel Teichroebwrites: > On Sat, Aug 19, 2017 at 1:55 PM, Junio C Hamano wrote: >> >> Are you documenting an existing breakage? Are you extending test >> coverage for some breakage we recently fixed without adding tests to >> ensure that the fix will stay unbroken? Are you planning to touch >> the implementation (perhaps to add yet another feature that uses >> some existing code) and adding new tests in advance to avoid breaking >> the existing code? Some other motivation? >> > > I was just too lazy to write a cover letter, and thought these would > make sense on their own. I'll make sure to include a cover letter next > time. > > I just ripped them out of my patch series on implementing stash as a > builtin[1]. Since I haven't had time, I figured I could at least get > the additional tests I wrote into the codebase. OK, so it is the third one among my random 4 choices ;-) They do look sensible; I did not want to make sure they are not duplicates myself (i.e. a new test that makes sense may not be an undesired addition if the only effect it has is to eat more cycles without improving test coverage). Will queue. Thanks. > The tests mainly expand coverage of git stash, covering a few critical > error cases that didn't have tests. > > [1] https://public-inbox.org/git/20170608005535.13080-1-j...@teichroeb.net/ >
Re: [PATCH 1/3] stash: add test for stash create with no files
Hi Junio, I was just too lazy to write a cover letter, and thought these would make sense on their own. I'll make sure to include a cover letter next time. I just ripped them out of my patch series on implementing stash as a builtin[1]. Since I haven't had time, I figured I could at least get the additional tests I wrote into the codebase. The tests mainly expand coverage of git stash, covering a few critical error cases that didn't have tests. [1] https://public-inbox.org/git/20170608005535.13080-1-j...@teichroeb.net/ On Sat, Aug 19, 2017 at 1:55 PM, Junio C Hamanowrote: > I see three patches that add tests, but it is hard to judge them > without any explanation on what the point of them are. > > Are you documenting an existing breakage? Are you extending test > coverage for some breakage we recently fixed without adding tests to > ensure that the fix will stay unbroken? Are you planning to touch > the implementation (perhaps to add yet another feature that uses > some existing code) and adding new tests in advance to avoid breaking > the existing code? Some other motivation? > > These would have made fine material to write in the cover letter for > this series. > > Thanks.
Re: [PATCH 1/3] stash: add test for stash create with no files
I see three patches that add tests, but it is hard to judge them without any explanation on what the point of them are. Are you documenting an existing breakage? Are you extending test coverage for some breakage we recently fixed without adding tests to ensure that the fix will stay unbroken? Are you planning to touch the implementation (perhaps to add yet another feature that uses some existing code) and adding new tests in advance to avoid breaking the existing code? Some other motivation? These would have made fine material to write in the cover letter for this series. Thanks.