On Mon, Jan 09, 2017 at 12:46:23PM +0200, Amir Goldstein wrote: > On Mon, Jan 9, 2017 at 12:17 PM, Eryu Guan <eguan@xxxxxxxxxx> wrote: > > Hi Eric and Amir, > > > > On Fri, Jan 06, 2017 at 08:57:31PM -0800, Eric Biggers wrote: > >> Hi Eryu, > >> > >> On Fri, Jan 06, 2017 at 11:45:07AM +0800, Eryu Guan wrote: > >> > $tmp.* files should be removed in _cleanup() even if the test is not > >> > using any $tmp.* file explicitly, because common helper functions > >> > may take use of them too. > >> > > >> > So cleanup tmp files properly in tests, and add a _cleanup() > >> > function and trap it on exit if the test doesn't do so, to make all > >> > tests consistent on the way they do cleanup. > >> > > >> > Also remove other tmp files used by the tests and the harness so > >> > that we leave no new tmp files in /tmp dir after a full test run. > >> > > >> > >> Why exactly can't the boilerplate go in a preamble file which is sourced by all > >> the tests? For example common/preamble containing something like: > > > > Thanks for your review! As you two are basically suggesting the same > > solution, so I reply to you together. > > > > I agree that we can do a common/preamble for all tests to source, but I > > don't think we need to do that and it's not worth the effort. > > > > IMO, we define and setup the tests in the tests themselves, not in a > > common/preamble file, which makes the tests clear and easy to read. We > > don't want to "hide" any setups in a common file and to be a "surprise" > > to the reader, we don't have to look around for a certain setup. > > > > The motivation for your work is the fact that tmp files are being created > in a way that is "hidden" from the test author and therefore, IMO, > the cleanup of those temp files should also be in a common function > that is "hidden" from the test author. [Sorry, I got back to this late.] That's a good point, so I think what should be done is let common functions clean up their tmp files, so there's no hidden usage of $tmp and tests only need to remove $tmp.* used explicitly in the tests. I checked all the common functions, there're only two that are not cleaning up their tmp files properly, _do() and run_fsx(). I'll send v2 patch for review, which will be a much simpler patch. Thanks, Eryu -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
