Re: [PATCH 3/3 v2] xfstests: Add support for sections in config file

[Dave Chinner <david@fromorbit.com>]

On Mon, Jul 01, 2013 at 10:36:37AM +0200, Lukáš Czerner wrote:
> On Mon, 1 Jul 2013, Dave Chinner wrote:
> > > Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> > > ---
> > >  check         | 371 +++++++++++++++++++++++++++++++++-------------------------
> > >  common/config | 126 ++++++++++++--------
> > >  common/rc     |  63 +++++-----
> > >  3 files changed, 326 insertions(+), 234 deletions(-)
> > 
> > This patch probably needs to be broken up, too. A substantial part
> > of it is indentation changes, which probably should be split into 2
> > parts - factor code into function, then wrap loop around function.
> > The change to the summary information should be done as a separate
> > patch, too. I suspect many of the common/config changes coul dbe
> > split up, too.
> 
> Ok, I'll see how can I split it up. Btw, indentation in xfstests is
> a mess, because sometimes we're using spaces and sometimes tabs. Is
> there any preference ? (I would definitely prefer tabs)

Tabs.

> > The changing of the $RESULT_BASE should probably also
> > bein a separate patch, because this is something that we'll need to
> > discuss as it changes the structure of the output....
> 
> I am not sure it should be separate from this patch because the new
> structure will only be used if the new config format (with sections)
> is used.

Which is a bit confusing, especially as a separate result directory
might be desired for the output of each section - say I want to keep
all the "config A" results together, but spearate to all the "config
B" results. Placing them all under the same $RESULT_BASE isn't ideal
at that point....

IOWs, I suspect that $RESULT_BASE should be able to be defined in
the section config so that you can redirect results that way.

> > Oh, and why make a distinction between no sections and
> > $OPTIONS_HAVE_SECTIONS in the config file? Surely no sections is
> > just the same as having 1 section....
> 
> Yes, but it's much "nicer" to check boolean option than checking
> what is the name of the first section. Really this is just a
> workaround, because I did not want to change result structure and
> output if one is not using the new config format.

That's another reason why I think that we should be able to have a
per-section RESULTS_BASE - that way the code doesn't care how
RESULTS_BASE is defined or where it points to - the output of
xfstests is *always* the same. That is, after all, why $RESULTS_BASE
was introduced in the first place.

> It could be done
> so that when there are no sections we'll always use "default" section,
> not sure what would people prefer.

Exactly why it needs discussion ;)

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs