| Message ID | 1464858659-610-1-git-send-email-jtulak@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jun 02, 2016 at 11:10:59AM +0200, Jan Tulak wrote: > Because we recently changed how mkfs behaves when it gets incorrect/invalid > values, update the expected output to reflect the change. This will break test with old-behavior xfsprogs. But I'm not sure what the best solution is.. And it seems that generic/054 and generic/055 are failing because of the same reason, if so, fix them together? 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
Sorry for the spam, I'm sending it once more and this time it should pass vger.kernel.org spam filter. ---------- Forwarded message ---------- From: Jan Tulak <jtulak@redhat.com> Date: Tue, Jun 7, 2016 at 10:18 AM Subject: Re: [PATCH] xfstests: update xfs/096 for new behaviour To: Eryu Guan <eguan@redhat.com> Cc: fstests@vger.kernel.org On Tue, Jun 7, 2016 at 5:45 AM, Eryu Guan <eguan@redhat.com> wrote: > > On Thu, Jun 02, 2016 at 11:10:59AM +0200, Jan Tulak wrote: > > Because we recently changed how mkfs behaves when it gets incorrect/invalid > > values, update the expected output to reflect the change. > > This will break test with old-behavior xfsprogs. But I'm not sure what > the best solution is.. Hmm, well, an "if version > something" could work, together with testing the changed text directly in the test and making the correct output quiet. I can add something like that to xfstests and make it a function (has_mkfs_old_input_format) for easier use in the tests, but... It seems that only this single test is broken by the change, and I don't know if we want this backward compatibility in future tests. The only case when I see a usage would be finding a bug and then using the test to bisect the commit, while going over the change boundary. And will the persons doing this remember that there is a check for this? Or will they vaguely remember that there was some change and just look for the version and make their own "if version"? I would like a centralised solution, but I'm really afraid that it would be of no use. And moving the output text into the test is the only way I can think of for this specific test. > > > And it seems that generic/054 and generic/055 are failing because of the > same reason, if so, fix them together? These two tests should be fixed by the -l su minval patch, so it is just this one. Cheers, Jan
On Tue, Jun 7, 2016 at 10:18 AM, Jan Tulak <jtulak@redhat.com> wrote: > > > > On Tue, Jun 7, 2016 at 5:45 AM, Eryu Guan <eguan@redhat.com> wrote: >> >> On Thu, Jun 02, 2016 at 11:10:59AM +0200, Jan Tulak wrote: >> > Because we recently changed how mkfs behaves when it gets >> > incorrect/invalid >> > values, update the expected output to reflect the change. >> >> This will break test with old-behavior xfsprogs. But I'm not sure what >> the best solution is.. > > > Hmm, well, an "if version > something" could work, together with testing the > changed text directly in the test and making the correct output quiet. > > I can add something like that to xfstests and make it a function > (has_mkfs_old_input_format) for easier use in the tests, but... It seems > that only this single test is broken by the change, and I don't know if we > want this backward compatibility in future tests. > > The only case when I see a usage would be finding a bug and then using the > test to bisect the commit, while going over the change boundary. And will > the persons doing this remember that there is a check for this? Or will they > vaguely remember that there was some change and just look for the version > and make their own "if version"? > > I would like a centralised solution, but I'm really afraid that it would be > of no use. And moving the output text into the test is the only way I can > think of for this specific test. > > >> >> >> And it seems that generic/054 and generic/055 are failing because of the >> same reason, if so, fix them together? > > > These two tests should be fixed by the -l su minval patch, so it is just > this one. > Mmm, I spent some time on this but did not figure out any nice solution. Or... I found one, but I'm not sure how you will like it. Making the test to comply both versions is difficult because it is not just the error message that differs, but also that this run is now invalid: # test log stripe greater than LR size --- mkfs=-l version=2,su=266240 --- It differs also in what should fail. So rather than making some complicated logic, I got the idea to make a duplicity of this test. One will run with old version and skipped on the new, the other vice versa. Naming can utilize the text suffixes, so we would have xfs/096 and xfs/096-old-mkfs-inputs. It is not ideal, but looks better than some in-test filtering... What do you think? Cheers, Jan
On Thu, Jun 23, 2016 at 1:22 PM, Jan Tulak <jtulak@redhat.com> wrote: > On Tue, Jun 7, 2016 at 10:18 AM, Jan Tulak <jtulak@redhat.com> wrote: >> >> >> >> On Tue, Jun 7, 2016 at 5:45 AM, Eryu Guan <eguan@redhat.com> wrote: >>> >>> On Thu, Jun 02, 2016 at 11:10:59AM +0200, Jan Tulak wrote: >>> > Because we recently changed how mkfs behaves when it gets >>> > incorrect/invalid >>> > values, update the expected output to reflect the change. >>> >>> This will break test with old-behavior xfsprogs. But I'm not sure what >>> the best solution is.. >> >> >> Hmm, well, an "if version > something" could work, together with testing the >> changed text directly in the test and making the correct output quiet. >> >> I can add something like that to xfstests and make it a function >> (has_mkfs_old_input_format) for easier use in the tests, but... It seems >> that only this single test is broken by the change, and I don't know if we >> want this backward compatibility in future tests. >> >> The only case when I see a usage would be finding a bug and then using the >> test to bisect the commit, while going over the change boundary. And will >> the persons doing this remember that there is a check for this? Or will they >> vaguely remember that there was some change and just look for the version >> and make their own "if version"? >> >> I would like a centralised solution, but I'm really afraid that it would be >> of no use. And moving the output text into the test is the only way I can >> think of for this specific test. >> >> >>> >>> >>> And it seems that generic/054 and generic/055 are failing because of the >>> same reason, if so, fix them together? >> >> >> These two tests should be fixed by the -l su minval patch, so it is just >> this one. >> > > Mmm, I spent some time on this but did not figure out any nice > solution. Or... I found one, but I'm not sure how you will like it. > > Making the test to comply both versions is difficult because it is not > just the error message that differs, but also that this run is now > invalid: > > # test log stripe greater than LR size > --- mkfs=-l version=2,su=266240 --- > > It differs also in what should fail. So rather than making some > complicated logic, I got the idea to make a duplicity of this test. > One will run with old version and skipped on the new, the other vice > versa. Naming can utilize the text suffixes, so we would have xfs/096 > and xfs/096-old-mkfs-inputs. > > It is not ideal, but looks better than some in-test filtering... What > do you think? > Just a correction, the new test name would be something like XXX-old-mkfs-inputs-096, the sequential number has to be unique. Thanks, Jan
diff --git a/tests/xfs/096.out.internal b/tests/xfs/096.out.internal index 80201d2..82c2043 100644 --- a/tests/xfs/096.out.internal +++ b/tests/xfs/096.out.internal @@ -2,18 +2,62 @@ QA output created by 096 # su too big but must be a multiple of fs block size too --- mkfs=-l version=2,su=262656 --- -log stripe unit (262656) must be a multiple of the block size (4096) +Illegal value 262656 for -l su option. value is too large +Usage: mkfs.xfs +/* blocksize */ [-b log=n|size=num] +/* data subvol */ [-d agcount=n,agsize=n,file,name=xxx,size=num, + (sunit=value,swidth=value|su=num,sw=num|noalign), + sectlog=n|sectsize=num +/* force overwrite */ [-f] +/* inode size */ [-i log=n|perblock=n|size=num,maxpct=n,attr=0|1|2, + projid32bit=0|1,sparse=0|1] +/* no discard */ [-K] +/* log subvol */ [-l agnum=n,internal,size=num,logdev=xxx,version=n + sunit=value|su=num,sectlog=n|sectsize=num, + lazy-count=0|1] +/* label */ [-L label (maximum 12 characters)] +/* naming */ [-n log=n|size=num,version=N|ci,ftype=0|1] +/* no-op info only */ [-N] +/* prototype file */ [-p fname] +/* quiet */ [-q] +/* realtime subvol */ [-r extsize=num,size=num,rtdev=xxx] +/* sectorsize */ [-s log=n|size=num] +/* version */ [-V] + devicename +<devicename> is required unless -d name=xxx is given. +<num> is xxx (bytes), xxxs (sectors), xxxb (fs blocks), xxxk (xxx KiB), + xxxm (xxx MiB), xxxg (xxx GiB), xxxt (xxx TiB) or xxxp (xxx PiB). +<value> is xxx (512 byte blocks). # test log stripe greater than LR size --- mkfs=-l version=2,su=266240 --- -meta-data=DEV isize=N agcount=N, agsize=N blks -data = bsize=4096 blocks=N, imaxpct=N - = sunit=0 swidth=0 blks, unwritten=1 -naming =version 2 bsize=4096 -log =LOG bsize=4096 blocks=N, version=N - = sunit=N blks -realtime =REALTIME extsz=N, blocks=N, rtextents=N +Illegal value 266240 for -l su option. value is too large +Usage: mkfs.xfs +/* blocksize */ [-b log=n|size=num] +/* data subvol */ [-d agcount=n,agsize=n,file,name=xxx,size=num, + (sunit=value,swidth=value|su=num,sw=num|noalign), + sectlog=n|sectsize=num +/* force overwrite */ [-f] +/* inode size */ [-i log=n|perblock=n|size=num,maxpct=n,attr=0|1|2, + projid32bit=0|1,sparse=0|1] +/* no discard */ [-K] +/* log subvol */ [-l agnum=n,internal,size=num,logdev=xxx,version=n + sunit=value|su=num,sectlog=n|sectsize=num, + lazy-count=0|1] +/* label */ [-L label (maximum 12 characters)] +/* naming */ [-n log=n|size=num,version=N|ci,ftype=0|1] +/* no-op info only */ [-N] +/* prototype file */ [-p fname] +/* quiet */ [-q] +/* realtime subvol */ [-r extsize=num,size=num,rtdev=xxx] +/* sectorsize */ [-s log=n|size=num] +/* version */ [-V] + devicename +<devicename> is required unless -d name=xxx is given. +<num> is xxx (bytes), xxxs (sectors), xxxb (fs blocks), xxxk (xxx KiB), + xxxm (xxx MiB), xxxg (xxx GiB), xxxt (xxx TiB) or xxxp (xxx PiB). +<value> is xxx (512 byte blocks). # same test but get log stripe from data stripe
Because we recently changed how mkfs behaves when it gets incorrect/invalid values, update the expected output to reflect the change. Signed-off-by: Jan Tulak <jtulak@redhat.com> --- tests/xfs/096.out.internal | 60 +++++++++++++++++++++++++++++++++++++++------- 1 file changed, 52 insertions(+), 8 deletions(-)