On Mon, Apr 18, 2022 at 8:05 AM Zorro Lang <zlang@xxxxxxxxxx> wrote:
>
> On Sun, Apr 17, 2022 at 08:40:23PM +0300, Amir Goldstein wrote:
> > Some tests are written before a fix is merged upstream and some tests
> > are written without a known available fix at all. Such is the case with
> > test overlay/061.
> >
> > Introduce a helper _known_issues_on_fs() which can be used to document
> > this situation and print a hint on failure.
> > The helper supports specifying specifc fs with the known issue for
> > generic tests as well as the notation ^FS1 ^FS2 to indicate a known issue
> > on all filesystems expect for FS1 FS2.
> >
> > Setting the variable SKIP_KNOWN_ISSUES=yes, will cause all tests
> > annotated as known issues for the tested fs to be skipped.
> >
> > A future improvement may provide a run option to skip tests based on
> > _known_issues_before_kernel when running the tests on an older kernel
> > version.
> >
> > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> > ---
> > README | 2 ++
> > common/rc | 16 +++++++++++++++-
> > tests/overlay/061 | 2 ++
> > 3 files changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/README b/README
> > index 7da66cb6..8abc3840 100644
> > --- a/README
> > +++ b/README
> > @@ -241,6 +241,8 @@ Misc:
> > this option is supported for all filesystems currently only -overlay is
> > expected to run without issues. For other filesystems additional patches
> > and fixes to the test suite might be needed.
> > + - set SKIP_KNOWN_ISSUES=yes to skip tests for bug without a known fix.
> > + Those tests are annotated with _known_issue_on_fs helper.
> >
> > ______________________
> > USING THE FSQA SUITE
> > diff --git a/common/rc b/common/rc
> > index 2e9dc408..3cf60a7e 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -1597,8 +1597,23 @@ _supported_fs()
> > _notrun "not suitable for this filesystem type: $FSTYP"
> > }
> >
> > +_known_issue_on_fs()
> > +{
> > + # Only "supported" fs have the known issue
> > + _check_supported_fs $* || return
> > +
> > + if [ "$SKIP_KNOWN_ISSUES" = "yes" ]; then
> > + _notrun "known issue for this filesystem type: $FSTYP"
> > + fi
> > +
> > + echo "HINT: You _MAY_ be hit by a known issue for filesystem type $FSTYP." >> $seqres.hints
> > + echo >> $seqres.hints
> > +}
> > +
> > _known_issue_before_kernel()
> > {
> > + # TODO: don't run if $SKIP_KNOWN_ISSUES and kernel version < $1
> > +
>
> As I said in last patch review, it's only for one kernel base line, it might cause
> downstream kernel testing can't run a case, even if it had backported the patches.
>
I agree. This may be useful to some but a bit controversial.
Anyway, I shall remove this TODO.
> > echo "HINT: You _MAY_ be hit by a known issue on kernel version < $1." >> $seqres.hints
> > echo >> $seqres.hints
> > }
> > @@ -4929,7 +4944,6 @@ _require_kernel_config()
> > _has_kernel_config $1 || _notrun "Installed kernel not built with $1"
> > }
> >
> > -
> > init_rc
> >
> > ################################################################################
> > diff --git a/tests/overlay/061 b/tests/overlay/061
> > index b80cf5a0..36be3391 100755
> > --- a/tests/overlay/061
> > +++ b/tests/overlay/061
> > @@ -22,6 +22,8 @@ _begin_fstest posix copyup
> >
> > # real QA test starts here
> > _supported_fs overlay
> > +_known_issue_on_fs overlay
>
> If it's a overlay case, it's absolutely cover an overlay known issue at first.
> Even if it hit a mm or underlying fs bug, the _known_issue_on_fs is helpless
> for that.
I disagree.
There is a subtle difference.
Most of the tests in fstests are regression tests meaning that they
cover an issue that is supposed to be fixed in the tested kernel.
In case of overlayfs, I took "test driven development" a bit further and
wrote fstests to cover non-standard behavior that we want to fix and then
wrote a development roadmap [*] that took more that 10 kernel releases
to complete.
Test overlay/061 is the last of those tests, whose issue was never addressed
and there is no prospect as to when it is going to be addressed.
This is the reason that previous patch removed all those lines from comments:
-# This simple test demonstrates a known issue with overlayfs:
[*] https://github.com/amir73il/overlayfs/wiki/Overlayfs-TODO#Compliance
As a tester, how would you know if something is wrong with your kernel
when a test is failing unless you had an annotation that tells you that the
test is expected to fail?
SKIP_KNOWN_ISSUES takes this one step further and says don't bother
me with tests that are expected to fail.
>
> I can understand above 2 patches, but this patch looks weird to me. We can
> give a hint to downstream testing (from upstream commit side). But deal with
> known issues, that's another story. I'd like to let testers from different
> downstream kernels to deal with their known issues checking by themselves,
> don't depend on upstream fstests for all.
>
Not sure I understand your point.
The intention of this helper was to address the debate over two of Darrick's
new tests. I mentioned them in the cover letter but forgot to mention here:
[1] https://lore.kernel.org/fstests/20220412172853.GG16799@magnolia/
[2] https://lore.kernel.org/fstests/20220414155007.GC17014@magnolia/
There is a difference between:
_supported_fs ext4 xfs btrfs
And:
_supported_fs generic
# https://lore.kernel.org/fstests/20220414155007.GC17014@magnolia/
_known_issue_on_fs ^ext4 ^xfs ^btrfs
The main difference is that the test WILL run and fail on all other fs
and both me and Eryu indicated that should happen.
The difference from only using _supported_fs generic
is that giving more information causes less surprises and less confusion
to other fs developers that get a surprise new failure when pulling fstests
upstream updates.
In the end, that what this series is all about - being more considerate
of other testers and making the information known to test author known
to a much wider audience that does not need to dig in test comments
and long mailing list discussions or overlayfs roadmap wikis to find that
information.
Thanks,
Amir.
