Re: [PATCH v4 1/3] tests: Split ZFS ZSTD Test Into New File

On Thu, 6 Nov 2025 14:16:11 +0100
Daniel Kiper <[email protected]> wrote:

> On Wed, Nov 05, 2025 at 10:04:25PM -0600, Andrew Hamilton wrote:
> > Split ZFS ZSTD test into its own test script. Add a check
> > to the new test script to see if the zfs utility installed
> > on the host supports "zstd" compression before running the
> > test and skip / error-out the test if not. It seems at least some
> > zfs-fuse binaries do not support zstd compression and the current
> > test will fail in that case. Splitting into a new file will avoid
> > masking other test failures due to missing zstd support.
> >
> > Signed-off-by: Andrew Hamilton <[email protected]>
> > ---
> >  .gitignore             |  1 +
> >  Makefile.util.def      |  6 ++++++
> >  tests/zfs_test.in      |  1 -
> >  tests/zfs_zstd_test.in | 30 ++++++++++++++++++++++++++++++
> >  4 files changed, 37 insertions(+), 1 deletion(-)
> >  create mode 100644 tests/zfs_zstd_test.in
> >
> > diff --git a/.gitignore b/.gitignore
> > index 524f2e6d0..67ad2d26d 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -283,3 +283,4 @@ widthspec.bin
> >  /xfs_test
> >  /xzcompress_test
> >  /zfs_test
> > +/zfs_zstd_test
> > diff --git a/Makefile.util.def b/Makefile.util.def
> > index 91720e627..7b91c0b61 100644
> > --- a/Makefile.util.def
> > +++ b/Makefile.util.def
> > @@ -914,6 +914,12 @@ script = {
> >    common = tests/zfs_test.in;
> >  };
> >
> > +script = {
> > +  testcase = native;
> > +  name = zfs_zstd_test;
> > +  common = tests/zfs_zstd_test.in;
> > +};
> > +
> >  script = {
> >    testcase = native;
> >    name = cpio_test;
> > diff --git a/tests/zfs_test.in b/tests/zfs_test.in
> > index 0d0a57f7d..58cc25b22 100644
> > --- a/tests/zfs_test.in
> > +++ b/tests/zfs_test.in
> > @@ -19,7 +19,6 @@ fi
> >  "@builddir@/grub-fs-tester" zfs_lzjb
> >  "@builddir@/grub-fs-tester" zfs_gzip
> >  "@builddir@/grub-fs-tester" zfs_zle
> > -"@builddir@/grub-fs-tester" zfs_zstd
> >  "@builddir@/grub-fs-tester" zfs_raidz3
> >  "@builddir@/grub-fs-tester" zfs_raidz2
> >  "@builddir@/grub-fs-tester" zfs_raidz
> > diff --git a/tests/zfs_zstd_test.in b/tests/zfs_zstd_test.in
> > new file mode 100644
> > index 000000000..fa187b061
> > --- /dev/null
> > +++ b/tests/zfs_zstd_test.in
> > @@ -0,0 +1,30 @@
> > +#!@BUILD_SHEBANG@
> > +
> > +set -e
> > +
> > +if [ "x$EUID" = "x" ] ; then
> > +  EUID=`id -u`
> > +fi
> > +
> > +if [ "$EUID" != 0 ] ; then
> > +   exit 99
> > +fi
> > +
> > +if ! which zpool >/dev/null 2>&1; then
> > +   echo "zpool not installed; cannot test zfs."
> > +   exit 99
> > +fi
> > +
> > +if ! which zfs >/dev/null 2>&1; then
> > +   echo "zfs not installed; cannot test zfs."
> > +   exit 99
> > +fi
> > +
> > +# If OpenZFS is not installed (only zfs-fuse for example) then
> > +# report fail for ZSTD compression testing.
> > +if ! zfs get 2>&1 | grep -F "compression" | grep -F "zstd"; then
> > +   echo "zfs zstd compression not supported; cannot test zfs zstd."
> > +   exit 99
> 
> I think you should return 77 instead of 99 in all cases...

The return values should remain as 99, which represents a failure of a
test that is not because the actual thing we want to test fails. This
includes failure to properly setup the environment. zfs-fuse is not
adequate to run the tests and the one running the tests should be
notified of that. For proper testing the kernel module should be used.

A return of 77 indicates that the test does not apply to the GRUB
configuration, which it does in this case. If GRUB were built for a
target that does not support ZFS, then a 77 would be appropriate. A 77
might also be appropriate for tests that test features that are
conditionally compiled. GRUB can be compiled without ZFS support, but
that's only for the user-space utilities. The target zfs module is
unconditionally built.

Glenn

> Otherwise patches LGTM. So, you can add my RB to all of them after
> fixing the issue mentioned above.
> 
> Daniel
> 
> _______________________________________________
> Grub-devel mailing list
> [email protected]
> https://lists.gnu.org/mailman/listinfo/grub-devel

_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel