All of lore.kernel.org
 help / color / mirror / Atom feed
* generic/411 clash with TEST_DIR=/mnt
@ 2017-03-07 16:50 Darrick J. Wong
  2017-03-08  3:50 ` Zorro Lang
  0 siblings, 1 reply; 9+ messages in thread
From: Darrick J. Wong @ 2017-03-07 16:50 UTC (permalink / raw)
  To: Zorro Lang; +Cc: fstests

Hi Zorro,

I noticed that generic/411 creates some sort of directory structure
involving $TEST_DIR/$seq/$$_mpA/mnt1/mnt2, then pipes the findmnt output
through _filter_test_dir.

Unfortunately, on my test system I have TEST_DIR=/mnt, so the filtering
produces this output:

QA output created by 411
------
TEST_DIR/411 SCRATCH_DEV
mpA SCRATCH_DEV
mpATEST_DIR1 SCRATCH_DEV
mpB SCRATCH_DEV
mpBTEST_DIR1 SCRATCH_DEV
mpBTEST_DIR1/TEST_DIR2 SCRATCH_DEV
mpC SCRATCH_DEV
mpCTESTDIR1 SCRATCH_DEV
======
crash test passed

Which means that the golden output comparison fails. :(

I'm not sure what's a proper fix here: changing _filter_test_dir to be
more picky about what gets sed'ed?

e.g. sed -e "s,\([[:space:]]\)$TEST_DIR,\1TEST_DIR,g/"

Or just to change the test to use directory names that are less likely to be
mistaken for TEST_* and SCRATCH_*?

--D

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: generic/411 clash with TEST_DIR=/mnt
  2017-03-07 16:50 generic/411 clash with TEST_DIR=/mnt Darrick J. Wong
@ 2017-03-08  3:50 ` Zorro Lang
  2017-03-08  6:09   ` Eryu Guan
  2017-03-08  6:50   ` Amir Goldstein
  0 siblings, 2 replies; 9+ messages in thread
From: Zorro Lang @ 2017-03-08  3:50 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: fstests

On Tue, Mar 07, 2017 at 08:50:27AM -0800, Darrick J. Wong wrote:
> Hi Zorro,
> 
> I noticed that generic/411 creates some sort of directory structure
> involving $TEST_DIR/$seq/$$_mpA/mnt1/mnt2, then pipes the findmnt output
> through _filter_test_dir.
> 
> Unfortunately, on my test system I have TEST_DIR=/mnt, so the filtering
> produces this output:
> 
> QA output created by 411
> ------
> TEST_DIR/411 SCRATCH_DEV
> mpA SCRATCH_DEV
> mpATEST_DIR1 SCRATCH_DEV
> mpB SCRATCH_DEV
> mpBTEST_DIR1 SCRATCH_DEV
> mpBTEST_DIR1/TEST_DIR2 SCRATCH_DEV
> mpC SCRATCH_DEV
> mpCTESTDIR1 SCRATCH_DEV
> ======

Hmm, that's really a problem, thanks for finding it:) If someone
use "/mnt" as TEST_DIR, _filter_test_dir will change all "/mnt"
things to "TEST_DIR".

Except we recommend all cases shouldn't contain "/mnt"
in its test path name. Or we'd better change _filter_test_dir,
make it to be more picky.

I prefer the second way, except someone has a third way :)

> crash test passed
> 
> Which means that the golden output comparison fails. :(
> 
> I'm not sure what's a proper fix here: changing _filter_test_dir to be
> more picky about what gets sed'ed?
> 
> e.g. sed -e "s,\([[:space:]]\)$TEST_DIR,\1TEST_DIR,g/"

Except [[:space:]]$TEST_DIR, maybe we should think about "^$TEST_DIR" too,
e.g:

echo "/mnt/mnt1/mnt2     /mnt/mnt3/mnt4" | \
     sed -e "s,\([[:space:]]\)$TEST_DIR,\1TEST_DIR,g" | \
     sed -e "s,^$TEST_DIR,TEST_DIR,g"

(Is there a simpler way to do above things ^^ ? :)

But this change will affect lots of cases, we need to run full around
xfstests to make sure it won't change some cases' golden output. What
do you think?

Thanks,
Zorro

> 
> Or just to change the test to use directory names that are less likely to be
> mistaken for TEST_* and SCRATCH_*?
> 
> --D
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: generic/411 clash with TEST_DIR=/mnt
  2017-03-08  3:50 ` Zorro Lang
@ 2017-03-08  6:09   ` Eryu Guan
  2017-03-08  6:29     ` Zorro Lang
  2017-03-08 18:11     ` Darrick J. Wong
  2017-03-08  6:50   ` Amir Goldstein
  1 sibling, 2 replies; 9+ messages in thread
From: Eryu Guan @ 2017-03-08  6:09 UTC (permalink / raw)
  To: Zorro Lang; +Cc: Darrick J. Wong, fstests

On Wed, Mar 08, 2017 at 11:50:45AM +0800, Zorro Lang wrote:
> On Tue, Mar 07, 2017 at 08:50:27AM -0800, Darrick J. Wong wrote:
> > Hi Zorro,
> > 
> > I noticed that generic/411 creates some sort of directory structure
> > involving $TEST_DIR/$seq/$$_mpA/mnt1/mnt2, then pipes the findmnt output
> > through _filter_test_dir.
> > 
> > Unfortunately, on my test system I have TEST_DIR=/mnt, so the filtering

Ah, that's a rare setup, I'm curious what you use for SCRATCH_MNT :)

> > produces this output:
> > 
> > QA output created by 411
> > ------
> > TEST_DIR/411 SCRATCH_DEV
> > mpA SCRATCH_DEV
> > mpATEST_DIR1 SCRATCH_DEV
> > mpB SCRATCH_DEV
> > mpBTEST_DIR1 SCRATCH_DEV
> > mpBTEST_DIR1/TEST_DIR2 SCRATCH_DEV
> > mpC SCRATCH_DEV
> > mpCTESTDIR1 SCRATCH_DEV
> > ======
> 
> Hmm, that's really a problem, thanks for finding it:) If someone
> use "/mnt" as TEST_DIR, _filter_test_dir will change all "/mnt"
> things to "TEST_DIR".
> 
> Except we recommend all cases shouldn't contain "/mnt"
> in its test path name. Or we'd better change _filter_test_dir,
> make it to be more picky.
> 
> I prefer the second way, except someone has a third way :)
> 
> > crash test passed
> > 
> > Which means that the golden output comparison fails. :(
> > 
> > I'm not sure what's a proper fix here: changing _filter_test_dir to be
> > more picky about what gets sed'ed?
> > 
> > e.g. sed -e "s,\([[:space:]]\)$TEST_DIR,\1TEST_DIR,g/"
> 
> Except [[:space:]]$TEST_DIR, maybe we should think about "^$TEST_DIR" too,
> e.g:
> 
> echo "/mnt/mnt1/mnt2     /mnt/mnt3/mnt4" | \
>      sed -e "s,\([[:space:]]\)$TEST_DIR,\1TEST_DIR,g" | \
>      sed -e "s,^$TEST_DIR,TEST_DIR,g"
> 
> (Is there a simpler way to do above things ^^ ? :)
> 
> But this change will affect lots of cases, we need to run full around
> xfstests to make sure it won't change some cases' golden output. What
> do you think?
> 
> Thanks,
> Zorro
> 
> > 
> > Or just to change the test to use directory names that are less likely to be
> > mistaken for TEST_* and SCRATCH_*?

I'd prefer this easier way :) IMO, getting the filters right is tricky
(see Amir's recent patch for example, "4e965d8 fstests: fix test and
scratch filters for overlapping DEV/MNT paths"), and, as you said, will
affect more tests. Changing the directory names used in generic/411 is
much easier, prefixing with $seq should do the work, e.g. ${seq}_mnt1

Thanks,
Eryu

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: generic/411 clash with TEST_DIR=/mnt
  2017-03-08  6:09   ` Eryu Guan
@ 2017-03-08  6:29     ` Zorro Lang
  2017-03-08 18:11     ` Darrick J. Wong
  1 sibling, 0 replies; 9+ messages in thread
From: Zorro Lang @ 2017-03-08  6:29 UTC (permalink / raw)
  To: Eryu Guan; +Cc: Darrick J. Wong, fstests

On Wed, Mar 08, 2017 at 02:09:59PM +0800, Eryu Guan wrote:
> On Wed, Mar 08, 2017 at 11:50:45AM +0800, Zorro Lang wrote:
> > On Tue, Mar 07, 2017 at 08:50:27AM -0800, Darrick J. Wong wrote:
> > > Hi Zorro,
> > > 
> > > I noticed that generic/411 creates some sort of directory structure
> > > involving $TEST_DIR/$seq/$$_mpA/mnt1/mnt2, then pipes the findmnt output
> > > through _filter_test_dir.
> > > 
> > > Unfortunately, on my test system I have TEST_DIR=/mnt, so the filtering
> 
> Ah, that's a rare setup, I'm curious what you use for SCRATCH_MNT :)

me too :)

> 
> > > produces this output:
> > > 
> > > QA output created by 411
> > > ------
> > > TEST_DIR/411 SCRATCH_DEV
> > > mpA SCRATCH_DEV
> > > mpATEST_DIR1 SCRATCH_DEV
> > > mpB SCRATCH_DEV
> > > mpBTEST_DIR1 SCRATCH_DEV
> > > mpBTEST_DIR1/TEST_DIR2 SCRATCH_DEV
> > > mpC SCRATCH_DEV
> > > mpCTESTDIR1 SCRATCH_DEV
> > > ======
> > 
> > Hmm, that's really a problem, thanks for finding it:) If someone
> > use "/mnt" as TEST_DIR, _filter_test_dir will change all "/mnt"
> > things to "TEST_DIR".
> > 
> > Except we recommend all cases shouldn't contain "/mnt"
> > in its test path name. Or we'd better change _filter_test_dir,
> > make it to be more picky.
> > 
> > I prefer the second way, except someone has a third way :)
> > 
> > > crash test passed
> > > 
> > > Which means that the golden output comparison fails. :(
> > > 
> > > I'm not sure what's a proper fix here: changing _filter_test_dir to be
> > > more picky about what gets sed'ed?
> > > 
> > > e.g. sed -e "s,\([[:space:]]\)$TEST_DIR,\1TEST_DIR,g/"
> > 
> > Except [[:space:]]$TEST_DIR, maybe we should think about "^$TEST_DIR" too,
> > e.g:
> > 
> > echo "/mnt/mnt1/mnt2     /mnt/mnt3/mnt4" | \
> >      sed -e "s,\([[:space:]]\)$TEST_DIR,\1TEST_DIR,g" | \
> >      sed -e "s,^$TEST_DIR,TEST_DIR,g"
> > 
> > (Is there a simpler way to do above things ^^ ? :)
> > 
> > But this change will affect lots of cases, we need to run full around
> > xfstests to make sure it won't change some cases' golden output. What
> > do you think?
> > 
> > Thanks,
> > Zorro
> > 
> > > 
> > > Or just to change the test to use directory names that are less likely to be
> > > mistaken for TEST_* and SCRATCH_*?
> 
> I'd prefer this easier way :) IMO, getting the filters right is tricky
> (see Amir's recent patch for example, "4e965d8 fstests: fix test and
> scratch filters for overlapping DEV/MNT paths"), and, as you said, will
> affect more tests. Changing the directory names used in generic/411 is
> much easier, prefixing with $seq should do the work, e.g. ${seq}_mnt1

If we don't change the filter, we have a rule acquiescently, all sub-path
shouldn't contain $TEST_DIR or $SCRATCH_DIR. e.g:

TEST_DIR=/mnt/test; mkdir $TEST_DIR/mnt/test1
TEST_DIR=/mnt/1; mkdir $TEST_DIR/mnt/10

Thanks,
Zorro

> 
> Thanks,
> Eryu

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: generic/411 clash with TEST_DIR=/mnt
  2017-03-08  3:50 ` Zorro Lang
  2017-03-08  6:09   ` Eryu Guan
@ 2017-03-08  6:50   ` Amir Goldstein
  2017-03-08  7:14     ` Zorro Lang
  1 sibling, 1 reply; 9+ messages in thread
From: Amir Goldstein @ 2017-03-08  6:50 UTC (permalink / raw)
  To: Zorro Lang; +Cc: Darrick J. Wong, fstests

On Wed, Mar 8, 2017 at 5:50 AM, Zorro Lang <zlang@redhat.com> wrote:
> On Tue, Mar 07, 2017 at 08:50:27AM -0800, Darrick J. Wong wrote:
>> Hi Zorro,
>>
>> I noticed that generic/411 creates some sort of directory structure
>> involving $TEST_DIR/$seq/$$_mpA/mnt1/mnt2, then pipes the findmnt output
>> through _filter_test_dir.
>>
>> Unfortunately, on my test system I have TEST_DIR=/mnt, so the filtering
>> produces this output:
>>
>> QA output created by 411
>> ------
>> TEST_DIR/411 SCRATCH_DEV
>> mpA SCRATCH_DEV
>> mpATEST_DIR1 SCRATCH_DEV
>> mpB SCRATCH_DEV
>> mpBTEST_DIR1 SCRATCH_DEV
>> mpBTEST_DIR1/TEST_DIR2 SCRATCH_DEV
>> mpC SCRATCH_DEV
>> mpCTESTDIR1 SCRATCH_DEV
>> ======
>
> Hmm, that's really a problem, thanks for finding it:) If someone
> use "/mnt" as TEST_DIR, _filter_test_dir will change all "/mnt"
> things to "TEST_DIR".
>
> Except we recommend all cases shouldn't contain "/mnt"
> in its test path name. Or we'd better change _filter_test_dir,
> make it to be more picky.
>
> I prefer the second way, except someone has a third way :)
>
>> crash test passed
>>
>> Which means that the golden output comparison fails. :(
>>
>> I'm not sure what's a proper fix here: changing _filter_test_dir to be
>> more picky about what gets sed'ed?
>>
>> e.g. sed -e "s,\([[:space:]]\)$TEST_DIR,\1TEST_DIR,g/"
>
> Except [[:space:]]$TEST_DIR, maybe we should think about "^$TEST_DIR" too,
> e.g:
>
> echo "/mnt/mnt1/mnt2     /mnt/mnt3/mnt4" | \
>      sed -e "s,\([[:space:]]\)$TEST_DIR,\1TEST_DIR,g" | \
>      sed -e "s,^$TEST_DIR,TEST_DIR,g"
>

That's not enough.
It's true that currently golden outputs only have ' ' and ^ preceding TEST_DIR,
but why should we limit the solution to TEST_DIR
SCRATCH_MNT could just as well be /mnt and we have SCRATCH_MNT
in golden outputs following '\'' '\"' and : as well.

> (Is there a simpler way to do above things ^^ ? :)
>

I'm not a regexp master, but can't we setup an expression that matches
$TEST_DIR.\S and only replaces the $TEST_DIR part,
that would replace only the first occurrence of  $TEST_DIR is every word.

> But this change will affect lots of cases, we need to run full around
> xfstests to make sure it won't change some cases' golden output. What
> do you think?
>
> Thanks,
> Zorro
>
>>
>> Or just to change the test to use directory names that are less likely to be
>> mistaken for TEST_* and SCRATCH_*?
>>
>> --D
>> --
>> To unsubscribe from this list: send the line "unsubscribe fstests" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: generic/411 clash with TEST_DIR=/mnt
  2017-03-08  6:50   ` Amir Goldstein
@ 2017-03-08  7:14     ` Zorro Lang
  2017-03-08  8:20       ` Amir Goldstein
  0 siblings, 1 reply; 9+ messages in thread
From: Zorro Lang @ 2017-03-08  7:14 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: Darrick J. Wong, fstests

On Wed, Mar 08, 2017 at 08:50:50AM +0200, Amir Goldstein wrote:
> On Wed, Mar 8, 2017 at 5:50 AM, Zorro Lang <zlang@redhat.com> wrote:
> > On Tue, Mar 07, 2017 at 08:50:27AM -0800, Darrick J. Wong wrote:
> >> Hi Zorro,
> >>
> >> I noticed that generic/411 creates some sort of directory structure
> >> involving $TEST_DIR/$seq/$$_mpA/mnt1/mnt2, then pipes the findmnt output
> >> through _filter_test_dir.
> >>
> >> Unfortunately, on my test system I have TEST_DIR=/mnt, so the filtering
> >> produces this output:
> >>
> >> QA output created by 411
> >> ------
> >> TEST_DIR/411 SCRATCH_DEV
> >> mpA SCRATCH_DEV
> >> mpATEST_DIR1 SCRATCH_DEV
> >> mpB SCRATCH_DEV
> >> mpBTEST_DIR1 SCRATCH_DEV
> >> mpBTEST_DIR1/TEST_DIR2 SCRATCH_DEV
> >> mpC SCRATCH_DEV
> >> mpCTESTDIR1 SCRATCH_DEV
> >> ======
> >
> > Hmm, that's really a problem, thanks for finding it:) If someone
> > use "/mnt" as TEST_DIR, _filter_test_dir will change all "/mnt"
> > things to "TEST_DIR".
> >
> > Except we recommend all cases shouldn't contain "/mnt"
> > in its test path name. Or we'd better change _filter_test_dir,
> > make it to be more picky.
> >
> > I prefer the second way, except someone has a third way :)
> >
> >> crash test passed
> >>
> >> Which means that the golden output comparison fails. :(
> >>
> >> I'm not sure what's a proper fix here: changing _filter_test_dir to be
> >> more picky about what gets sed'ed?
> >>
> >> e.g. sed -e "s,\([[:space:]]\)$TEST_DIR,\1TEST_DIR,g/"
> >
> > Except [[:space:]]$TEST_DIR, maybe we should think about "^$TEST_DIR" too,
> > e.g:
> >
> > echo "/mnt/mnt1/mnt2     /mnt/mnt3/mnt4" | \
> >      sed -e "s,\([[:space:]]\)$TEST_DIR,\1TEST_DIR,g" | \
> >      sed -e "s,^$TEST_DIR,TEST_DIR,g"
> >
> 
> That's not enough.
> It's true that currently golden outputs only have ' ' and ^ preceding TEST_DIR,
> but why should we limit the solution to TEST_DIR
> SCRATCH_MNT could just as well be /mnt and we have SCRATCH_MNT
> in golden outputs following '\'' '\"' and : as well.

Hmm, yes, you're right, maybe follow other things... I didn't think
about this. So change the filter is not easy.

> 
> > (Is there a simpler way to do above things ^^ ? :)
> >
> 
> I'm not a regexp master, but can't we setup an expression that matches
> $TEST_DIR.\S and only replaces the $TEST_DIR part,
> that would replace only the first occurrence of  $TEST_DIR is every word.

Maybe output $TEST_* or $SCRATCH_* twice in one line.

Maybe there's not a better way to change the filter. Stop all cases using
sub-path name that's same with $TEST_* or $SCRATCH_* maybe the best way.

Thanks,
Zorro

> 
> > But this change will affect lots of cases, we need to run full around
> > xfstests to make sure it won't change some cases' golden output. What
> > do you think?
> >
> > Thanks,
> > Zorro
> >
> >>
> >> Or just to change the test to use directory names that are less likely to be
> >> mistaken for TEST_* and SCRATCH_*?
> >>
> >> --D
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe fstests" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > --
> > To unsubscribe from this list: send the line "unsubscribe fstests" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: generic/411 clash with TEST_DIR=/mnt
  2017-03-08  7:14     ` Zorro Lang
@ 2017-03-08  8:20       ` Amir Goldstein
  0 siblings, 0 replies; 9+ messages in thread
From: Amir Goldstein @ 2017-03-08  8:20 UTC (permalink / raw)
  To: Zorro Lang; +Cc: Darrick J. Wong, fstests

On Wed, Mar 8, 2017 at 9:14 AM, Zorro Lang <zlang@redhat.com> wrote:
> On Wed, Mar 08, 2017 at 08:50:50AM +0200, Amir Goldstein wrote:
>> On Wed, Mar 8, 2017 at 5:50 AM, Zorro Lang <zlang@redhat.com> wrote:
>> > On Tue, Mar 07, 2017 at 08:50:27AM -0800, Darrick J. Wong wrote:
>> >> Hi Zorro,
>> >>
>> >> I noticed that generic/411 creates some sort of directory structure
>> >> involving $TEST_DIR/$seq/$$_mpA/mnt1/mnt2, then pipes the findmnt output
>> >> through _filter_test_dir.
>> >>
>> >> Unfortunately, on my test system I have TEST_DIR=/mnt, so the filtering
>> >> produces this output:
>> >>
>> >> QA output created by 411
>> >> ------
>> >> TEST_DIR/411 SCRATCH_DEV
>> >> mpA SCRATCH_DEV
>> >> mpATEST_DIR1 SCRATCH_DEV
>> >> mpB SCRATCH_DEV
>> >> mpBTEST_DIR1 SCRATCH_DEV
>> >> mpBTEST_DIR1/TEST_DIR2 SCRATCH_DEV
>> >> mpC SCRATCH_DEV
>> >> mpCTESTDIR1 SCRATCH_DEV
>> >> ======
>> >
>> > Hmm, that's really a problem, thanks for finding it:) If someone
>> > use "/mnt" as TEST_DIR, _filter_test_dir will change all "/mnt"
>> > things to "TEST_DIR".
>> >
>> > Except we recommend all cases shouldn't contain "/mnt"
>> > in its test path name. Or we'd better change _filter_test_dir,
>> > make it to be more picky.
>> >
>> > I prefer the second way, except someone has a third way :)
>> >
>> >> crash test passed
>> >>
>> >> Which means that the golden output comparison fails. :(
>> >>
>> >> I'm not sure what's a proper fix here: changing _filter_test_dir to be
>> >> more picky about what gets sed'ed?
>> >>
>> >> e.g. sed -e "s,\([[:space:]]\)$TEST_DIR,\1TEST_DIR,g/"
>> >
>> > Except [[:space:]]$TEST_DIR, maybe we should think about "^$TEST_DIR" too,
>> > e.g:
>> >
>> > echo "/mnt/mnt1/mnt2     /mnt/mnt3/mnt4" | \
>> >      sed -e "s,\([[:space:]]\)$TEST_DIR,\1TEST_DIR,g" | \
>> >      sed -e "s,^$TEST_DIR,TEST_DIR,g"
>> >
>>
>> That's not enough.
>> It's true that currently golden outputs only have ' ' and ^ preceding TEST_DIR,
>> but why should we limit the solution to TEST_DIR
>> SCRATCH_MNT could just as well be /mnt and we have SCRATCH_MNT
>> in golden outputs following '\'' '\"' and : as well.
>
> Hmm, yes, you're right, maybe follow other things... I didn't think
> about this. So change the filter is not easy.
>
>>
>> > (Is there a simpler way to do above things ^^ ? :)
>> >
>>
>> I'm not a regexp master, but can't we setup an expression that matches
>> $TEST_DIR.\S and only replaces the $TEST_DIR part,
>> that would replace only the first occurrence of  $TEST_DIR is every word.
>
> Maybe output $TEST_* or $SCRATCH_* twice in one line.
>
> Maybe there's not a better way to change the filter. Stop all cases using
> sub-path name that's same with $TEST_* or $SCRATCH_* maybe the best way.
>

Or maybe there is

$ echo '/mnt/mnt/mnt /mnt/mnt1/mnt2 "/mnt/mnt/mnt:/mnt/mnt/mnt"' | sed
-e 's,\B/mnt,TEST_DIR,g'
TEST_DIR/mnt/mnt TEST_DIR/mnt1/mnt2 "TEST_DIR/mnt/mnt:TEST_DIR/mnt/mnt"

\b should stand for word boundary, so I thought regexp should use \b, but that
yields the opposite result. \B yields the correct result, but I do not
understand why...?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: generic/411 clash with TEST_DIR=/mnt
  2017-03-08  6:09   ` Eryu Guan
  2017-03-08  6:29     ` Zorro Lang
@ 2017-03-08 18:11     ` Darrick J. Wong
  2017-03-08 18:38       ` Zorro Lang
  1 sibling, 1 reply; 9+ messages in thread
From: Darrick J. Wong @ 2017-03-08 18:11 UTC (permalink / raw)
  To: Eryu Guan; +Cc: Zorro Lang, fstests

On Wed, Mar 08, 2017 at 02:09:59PM +0800, Eryu Guan wrote:
> On Wed, Mar 08, 2017 at 11:50:45AM +0800, Zorro Lang wrote:
> > On Tue, Mar 07, 2017 at 08:50:27AM -0800, Darrick J. Wong wrote:
> > > Hi Zorro,
> > > 
> > > I noticed that generic/411 creates some sort of directory structure
> > > involving $TEST_DIR/$seq/$$_mpA/mnt1/mnt2, then pipes the findmnt output
> > > through _filter_test_dir.
> > > 
> > > Unfortunately, on my test system I have TEST_DIR=/mnt, so the filtering
> 
> Ah, that's a rare setup, I'm curious what you use for SCRATCH_MNT :)

/opt, naturally. :P

(I see the discussion has already jumped to amending g/411, so I'll go there.)

--D

> > > produces this output:
> > > 
> > > QA output created by 411
> > > ------
> > > TEST_DIR/411 SCRATCH_DEV
> > > mpA SCRATCH_DEV
> > > mpATEST_DIR1 SCRATCH_DEV
> > > mpB SCRATCH_DEV
> > > mpBTEST_DIR1 SCRATCH_DEV
> > > mpBTEST_DIR1/TEST_DIR2 SCRATCH_DEV
> > > mpC SCRATCH_DEV
> > > mpCTESTDIR1 SCRATCH_DEV
> > > ======
> > 
> > Hmm, that's really a problem, thanks for finding it:) If someone
> > use "/mnt" as TEST_DIR, _filter_test_dir will change all "/mnt"
> > things to "TEST_DIR".
> > 
> > Except we recommend all cases shouldn't contain "/mnt"
> > in its test path name. Or we'd better change _filter_test_dir,
> > make it to be more picky.
> > 
> > I prefer the second way, except someone has a third way :)
> > 
> > > crash test passed
> > > 
> > > Which means that the golden output comparison fails. :(
> > > 
> > > I'm not sure what's a proper fix here: changing _filter_test_dir to be
> > > more picky about what gets sed'ed?
> > > 
> > > e.g. sed -e "s,\([[:space:]]\)$TEST_DIR,\1TEST_DIR,g/"
> > 
> > Except [[:space:]]$TEST_DIR, maybe we should think about "^$TEST_DIR" too,
> > e.g:
> > 
> > echo "/mnt/mnt1/mnt2     /mnt/mnt3/mnt4" | \
> >      sed -e "s,\([[:space:]]\)$TEST_DIR,\1TEST_DIR,g" | \
> >      sed -e "s,^$TEST_DIR,TEST_DIR,g"
> > 
> > (Is there a simpler way to do above things ^^ ? :)
> > 
> > But this change will affect lots of cases, we need to run full around
> > xfstests to make sure it won't change some cases' golden output. What
> > do you think?
> > 
> > Thanks,
> > Zorro
> > 
> > > 
> > > Or just to change the test to use directory names that are less likely to be
> > > mistaken for TEST_* and SCRATCH_*?
> 
> I'd prefer this easier way :) IMO, getting the filters right is tricky
> (see Amir's recent patch for example, "4e965d8 fstests: fix test and
> scratch filters for overlapping DEV/MNT paths"), and, as you said, will
> affect more tests. Changing the directory names used in generic/411 is
> much easier, prefixing with $seq should do the work, e.g. ${seq}_mnt1
> 
> Thanks,
> Eryu
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: generic/411 clash with TEST_DIR=/mnt
  2017-03-08 18:11     ` Darrick J. Wong
@ 2017-03-08 18:38       ` Zorro Lang
  0 siblings, 0 replies; 9+ messages in thread
From: Zorro Lang @ 2017-03-08 18:38 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: fstests

On Wed, Mar 08, 2017 at 10:11:26AM -0800, Darrick J. Wong wrote:
> On Wed, Mar 08, 2017 at 02:09:59PM +0800, Eryu Guan wrote:
> > On Wed, Mar 08, 2017 at 11:50:45AM +0800, Zorro Lang wrote:
> > > On Tue, Mar 07, 2017 at 08:50:27AM -0800, Darrick J. Wong wrote:
> > > > Hi Zorro,
> > > > 
> > > > I noticed that generic/411 creates some sort of directory structure
> > > > involving $TEST_DIR/$seq/$$_mpA/mnt1/mnt2, then pipes the findmnt output
> > > > through _filter_test_dir.
> > > > 
> > > > Unfortunately, on my test system I have TEST_DIR=/mnt, so the filtering
> > 
> > Ah, that's a rare setup, I'm curious what you use for SCRATCH_MNT :)
> 
> /opt, naturally. :P
> 
> (I see the discussion has already jumped to amending g/411, so I'll go there.)

Hi Darrick,

The discussion has already been far from amending g/411, Amir show an
advanced regex method to change _filter functions :)

Thanks,
Zorro

> 
> --D
> 
> > > > produces this output:
> > > > 
> > > > QA output created by 411
> > > > ------
> > > > TEST_DIR/411 SCRATCH_DEV
> > > > mpA SCRATCH_DEV
> > > > mpATEST_DIR1 SCRATCH_DEV
> > > > mpB SCRATCH_DEV
> > > > mpBTEST_DIR1 SCRATCH_DEV
> > > > mpBTEST_DIR1/TEST_DIR2 SCRATCH_DEV
> > > > mpC SCRATCH_DEV
> > > > mpCTESTDIR1 SCRATCH_DEV
> > > > ======
> > > 
> > > Hmm, that's really a problem, thanks for finding it:) If someone
> > > use "/mnt" as TEST_DIR, _filter_test_dir will change all "/mnt"
> > > things to "TEST_DIR".
> > > 
> > > Except we recommend all cases shouldn't contain "/mnt"
> > > in its test path name. Or we'd better change _filter_test_dir,
> > > make it to be more picky.
> > > 
> > > I prefer the second way, except someone has a third way :)
> > > 
> > > > crash test passed
> > > > 
> > > > Which means that the golden output comparison fails. :(
> > > > 
> > > > I'm not sure what's a proper fix here: changing _filter_test_dir to be
> > > > more picky about what gets sed'ed?
> > > > 
> > > > e.g. sed -e "s,\([[:space:]]\)$TEST_DIR,\1TEST_DIR,g/"
> > > 
> > > Except [[:space:]]$TEST_DIR, maybe we should think about "^$TEST_DIR" too,
> > > e.g:
> > > 
> > > echo "/mnt/mnt1/mnt2     /mnt/mnt3/mnt4" | \
> > >      sed -e "s,\([[:space:]]\)$TEST_DIR,\1TEST_DIR,g" | \
> > >      sed -e "s,^$TEST_DIR,TEST_DIR,g"
> > > 
> > > (Is there a simpler way to do above things ^^ ? :)
> > > 
> > > But this change will affect lots of cases, we need to run full around
> > > xfstests to make sure it won't change some cases' golden output. What
> > > do you think?
> > > 
> > > Thanks,
> > > Zorro
> > > 
> > > > 
> > > > Or just to change the test to use directory names that are less likely to be
> > > > mistaken for TEST_* and SCRATCH_*?
> > 
> > I'd prefer this easier way :) IMO, getting the filters right is tricky
> > (see Amir's recent patch for example, "4e965d8 fstests: fix test and
> > scratch filters for overlapping DEV/MNT paths"), and, as you said, will
> > affect more tests. Changing the directory names used in generic/411 is
> > much easier, prefixing with $seq should do the work, e.g. ${seq}_mnt1
> > 
> > Thanks,
> > Eryu
> > --
> > To unsubscribe from this list: send the line "unsubscribe fstests" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-03-08 18:40 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-07 16:50 generic/411 clash with TEST_DIR=/mnt Darrick J. Wong
2017-03-08  3:50 ` Zorro Lang
2017-03-08  6:09   ` Eryu Guan
2017-03-08  6:29     ` Zorro Lang
2017-03-08 18:11     ` Darrick J. Wong
2017-03-08 18:38       ` Zorro Lang
2017-03-08  6:50   ` Amir Goldstein
2017-03-08  7:14     ` Zorro Lang
2017-03-08  8:20       ` Amir Goldstein

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.