* Re: [XFSTESTS v4 0/4] Richacl tests [not found] <1457525199-15355-1-git-send-email-agruenba@redhat.com> @ 2016-03-14 22:24 ` Dave Chinner 2016-03-14 23:23 ` Andreas Gruenbacher 0 siblings, 1 reply; 4+ messages in thread From: Dave Chinner @ 2016-03-14 22:24 UTC (permalink / raw) To: Andreas Gruenbacher; +Cc: xfs, fstests On Wed, Mar 09, 2016 at 01:06:34PM +0100, Andreas Gruenbacher wrote: > Hello, > > here is a new version of the richacl tests. xfstests patches need to be sent to fstests@vger.kernel.org (added to CC list), not xfs@oss.sgi.com. > According to feedback from the > previous posting (http://oss.sgi.com/archives/xfs/2015-12/msg00316.html), each > of the richacl tests is not run separately, on a new scratch filesystem. Oh, my. So, you've taken this one comment: "The rule of thumb is that there should be one xfs test per individual regression test. You've got at least 10 separate regression tests there, so there should be at least 10 xfstests. They should not be aggregated into a single test - if you need to run them all at once, then that is what the richacl test group is for..." And then *implemented your own execution infrastructure* so that the tests are /listed/ as separate tests in a group file but you still /run them/ as one test? I'm almost lost for words. It seems to me that you've ignored all the comments Eric and I have made to you about properly integrating the tests into xfstests so that they are able to be maintained by anyone who works with xfstests. Instead, you've kept most of the wacky stuff and instead made the richacl tests even more of a special snowflake than they were before. This is not rocket science, Andreas. Both Eric and I have spelt out exactly how to convert the richacl test scripts to use xfstests scripts and infrastructure (e.g. http://oss.sgi.com/archives/xfs/2015-11/msg00506.html), but you seem to be willfully ignoring the feedback you are being given. i.e. - the separation of tests between richacl/<test> and tests/<fs>/<test number> is wrong. Implement the tests directly inside tests/<fs>/<test number>, using xfstests infrastructure, please. - still not using .out files and instead are using your own internal frankenstein output matching to determine success or failure. Use the xfstests infrastructure for golden output matching, please. - now has weird-ass richacl test execution from generic/338 and execute the tests correctly from the test harness itself. Again, use the xfstests infrastructure correctly rather than reinventing your own, please. Most of this is as simple as copying the execution parts of your scripts to the xfstests test scripts, and the output parts of the test scripts into the test.out file. There's no new infrastructure needed for running tests, no separate richacl/ script directory, etc. -Dave. -- Dave Chinner david@fromorbit.com ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [XFSTESTS v4 0/4] Richacl tests 2016-03-14 22:24 ` [XFSTESTS v4 0/4] Richacl tests Dave Chinner @ 2016-03-14 23:23 ` Andreas Gruenbacher 2016-03-15 0:36 ` Eric Sandeen 2016-03-15 3:30 ` Dave Chinner 0 siblings, 2 replies; 4+ messages in thread From: Andreas Gruenbacher @ 2016-03-14 23:23 UTC (permalink / raw) To: Dave Chinner; +Cc: XFS Developers, fstests On Mon, Mar 14, 2016 at 11:24 PM, Dave Chinner <david@fromorbit.com> wrote: > On Wed, Mar 09, 2016 at 01:06:34PM +0100, Andreas Gruenbacher wrote: >> Hello, >> >> here is a new version of the richacl tests. > > xfstests patches need to be sent to fstests@vger.kernel.org (added > to CC list), not xfs@oss.sgi.com. Sorry for that. >> According to feedback from the >> previous posting (http://oss.sgi.com/archives/xfs/2015-12/msg00316.html), each >> of the richacl tests is not run separately, on a new scratch filesystem. > > Oh, my. So, you've taken this one comment: > > "The rule of thumb is that there should be one xfs test per > individual regression test. You've got at least 10 separate > regression tests there, so there should be at least 10 > xfstests. They should not be aggregated into a single test > - if you need to run them all at once, then that is what the > richacl test group is for..." > > And then *implemented your own execution infrastructure* so that the > tests are /listed/ as separate tests in a group file but you still > /run them/ as one test? > > I'm almost lost for words. What? Each test runs separately; not sure what makes you think otherwise. > It seems to me that you've ignored all the comments Eric and I have > made to you about properly integrating the tests into xfstests so > that they are able to be maintained by anyone who works with > xfstests. Instead, you've kept most of the wacky stuff and instead > made the richacl tests even more of a special snowflake than they > were before. > > This is not rocket science, Andreas. Both Eric and I have spelt out > exactly how to convert the richacl test scripts to use xfstests > scripts and infrastructure (e.g. > http://oss.sgi.com/archives/xfs/2015-11/msg00506.html), but you seem > to be willfully ignoring the feedback you are being given. i.e. > > - the separation of tests between richacl/<test> and > tests/<fs>/<test number> is wrong. Implement the > tests directly inside tests/<fs>/<test number>, using > xfstests infrastructure, please. > > - still not using .out files and instead are using your own > internal frankenstein output matching to determine success > or failure. Use the xfstests infrastructure for golden > output matching, please. > > - now has weird-ass richacl test execution from generic/338 > and execute the tests correctly from the test harness > itself. Again, use the xfstests infrastructure correctly > rather than reinventing your own, please. > > Most of this is as simple as copying the execution parts of your > scripts to the xfstests test scripts, and the output parts of the > test scripts into the test.out file. There's no new infrastructure > needed for running tests, no separate richacl/ script directory, > etc. I've said again and again that maintaining one set of richacl tests in xfstests and another in the richacl package is going to really painful, and that because of that, I'm trying to find a way of using the same test scripts in both places. That message obviously didn't get through at all though. That is just sad. Andreas ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [XFSTESTS v4 0/4] Richacl tests 2016-03-14 23:23 ` Andreas Gruenbacher @ 2016-03-15 0:36 ` Eric Sandeen 2016-03-15 3:30 ` Dave Chinner 1 sibling, 0 replies; 4+ messages in thread From: Eric Sandeen @ 2016-03-15 0:36 UTC (permalink / raw) To: xfs, fstests On 3/14/16 6:23 PM, Andreas Gruenbacher wrote: > On Mon, Mar 14, 2016 at 11:24 PM, Dave Chinner <david@fromorbit.com> wrote: <snip> >> Most of this is as simple as copying the execution parts of your >> scripts to the xfstests test scripts, and the output parts of the >> test scripts into the test.out file. There's no new infrastructure >> needed for running tests, no separate richacl/ script directory, >> etc. > > I've said again and again that maintaining one set of richacl tests in > xfstests and another in the richacl package is going to really > painful, and that because of that, I'm trying to find a way of using > the same test scripts in both places. That message obviously didn't > get through at all though. That is just sad. Andreas, the issue as I see it is that xfstests is a large community project, with many users who have come to understand its implementation and its quirks^Wconventions. It is run and maintained by many people; over 150 authors have committed patches to the codebase. They understand how it all works together; it is a common language. The way you are proposing your richacl tests integration is unlike any other tests in the codebase; you have, to some degree, spliced your own test harness into xfstests, rather than following the advice of "When in Rome, do as the Romans do." This might make your life a little easier if you plan to maintain a separate repo of tests; in the meantime it makes life harder for the 150+ people who will be running and maintaining xfstests, not your test repo. The advantage of xfstests is that is is a common language, warts and all. It is run by developers and qa groups all over the world. There is a learning curve, but many people have learned it. Implementing your tests in this way adds a new and unique learning curve for all those people, and will make it less likely that others will share in the maintenance burden for these new tests. What I would suggest is that if you have tests which test only richacl userspace functionality, then perhaps you should keep them private to the richacl package. For tests which test kernel functionality, make them native to xfstests. This way you will get good coverage and maintenance help from all the people testing kernelspace with xfstests, and those hacking on richacls can run the tests local to it. This is more or less what e2fsprogs has done, and it seems to work out ok. -Eric ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [XFSTESTS v4 0/4] Richacl tests 2016-03-14 23:23 ` Andreas Gruenbacher 2016-03-15 0:36 ` Eric Sandeen @ 2016-03-15 3:30 ` Dave Chinner 1 sibling, 0 replies; 4+ messages in thread From: Dave Chinner @ 2016-03-15 3:30 UTC (permalink / raw) To: Andreas Gruenbacher; +Cc: XFS Developers, fstests On Tue, Mar 15, 2016 at 12:23:57AM +0100, Andreas Gruenbacher wrote: > On Mon, Mar 14, 2016 at 11:24 PM, Dave Chinner <david@fromorbit.com> wrote: > > On Wed, Mar 09, 2016 at 01:06:34PM +0100, Andreas Gruenbacher wrote: > >> Hello, > >> > >> here is a new version of the richacl tests. > > > > xfstests patches need to be sent to fstests@vger.kernel.org (added > > to CC list), not xfs@oss.sgi.com. > > Sorry for that. > > >> According to feedback from the > >> previous posting (http://oss.sgi.com/archives/xfs/2015-12/msg00316.html), each > >> of the richacl tests is not run separately, on a new scratch filesystem. > > > > Oh, my. So, you've taken this one comment: > > > > "The rule of thumb is that there should be one xfs test per > > individual regression test. You've got at least 10 separate > > regression tests there, so there should be at least 10 > > xfstests. They should not be aggregated into a single test > > - if you need to run them all at once, then that is what the > > richacl test group is for..." > > > > And then *implemented your own execution infrastructure* so that the > > tests are /listed/ as separate tests in a group file but you still > > /run them/ as one test? > > > > I'm almost lost for words. > > What? Each test runs separately; not sure what makes you think otherwise. You added test generic/338, which is *not included in the group file* and so is not run by the test harness. then you added tests generic/339-348, which have a name suffix that matches the script they are supposed to run located in the richacl/ directory. tests generic/339-348 do one thing: they execute generic/338, which then looks at the calling program name (e.g. 339-apply-masks) strips out the test number to get the script it's supposed to run. then it runs $here/richacl/apply-masks, captures all the output and only if the test fails (based on the return value of the script being run) does it dump the output of the test. If I renumber tests on commit (which I do regularly), this magical "run test generic/338" mechanism breaks completely. It's fragile, unmaintainable and *completely unnecessary*. > I've said again and again that maintaining one set of richacl tests in > xfstests and another in the richacl package is going to really > painful, and that because of that, I'm trying to find a way of using > the same test scripts in both places. That message obviously didn't > get through at all though. That is just sad. Yes, I fully understand that's what you are trying to do. I've already explained to you why your approached doesn't work for xfstests, which Eric has further explained in his response. Once we add a test to xfstests, the author *loses control* of the test. Nobody "owns" a set of tests - anyone can modify them and we expect that *everyone* will modify them. e.g. if someone is changing some generic infrastructure, we don't expect them to have to understand that there are these magical richacl tests that you can't move or filter or change in the same way all the other hundreds of tests can be changed. All the tests need to look the same, run the same way, and be structured in the same way. Otherwise we are simply setting ourselve up with a long term maintenance nightmare. If you really need to keep an identical copy of the tests somewhere else, then make the xfstests versions the master copy and write your own harness wrapper around the outside of those scripts. Keeping tests in standard format in xfstests is far more important to me than whether they get kept in sync with some other external package... Cheers, Dave. -- Dave Chinner david@fromorbit.com ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-03-15 3:30 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1457525199-15355-1-git-send-email-agruenba@redhat.com>
2016-03-14 22:24 ` [XFSTESTS v4 0/4] Richacl tests Dave Chinner
2016-03-14 23:23 ` Andreas Gruenbacher
2016-03-15 0:36 ` Eric Sandeen
2016-03-15 3:30 ` Dave Chinner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox