* [PATCH v2 0/2] More test fixes @ 2021-10-14 20:58 Glenn Washburn 2021-10-14 20:58 ` [PATCH v2 1/2] tests: Test aborts due to missing requirements should be marked as error instead of skipped Glenn Washburn ` (2 more replies) 0 siblings, 3 replies; 6+ messages in thread From: Glenn Washburn @ 2021-10-14 20:58 UTC (permalink / raw) To: grub-devel, Daniel Kiper; +Cc: Glenn Washburn Updates since v1: * Rebase onto latest master The first patch changes the test exit status from skipped to hard error, for tests which fail due to missing dependencies in the environment. The second is a trivial patch to parameterize all uses of parted in the partmap_test. Glenn Glenn Washburn (2): tests: Test aborts due to missing requirements should be marked as error instead of skipped tests: In partmap_test, use ${parted} variable when checking for binary tests/btrfs_test.in | 4 ++-- tests/cpio_test.in | 2 +- tests/exfat_test.in | 4 ++-- tests/ext234_test.in | 8 ++++---- tests/f2fs_test.in | 4 ++-- tests/fat_test.in | 4 ++-- tests/gzcompress_test.in | 2 +- tests/hfs_test.in | 6 +++--- tests/hfsplus_test.in | 4 ++-- tests/iso9660_test.in | 2 +- tests/jfs_test.in | 4 ++-- tests/lzocompress_test.in | 2 +- tests/minixfs_test.in | 8 ++++---- tests/nilfs2_test.in | 4 ++-- tests/ntfs_test.in | 6 +++--- tests/partmap_test.in | 6 +++--- tests/reiserfs_test.in | 4 ++-- tests/romfs_test.in | 2 +- tests/squashfs_test.in | 2 +- tests/tar_test.in | 2 +- tests/udf_test.in | 4 ++-- tests/xfs_test.in | 4 ++-- tests/xzcompress_test.in | 2 +- tests/zfs_test.in | 4 ++-- 24 files changed, 47 insertions(+), 47 deletions(-) Range-diff against v1: 1: e2f9f26f2 ! 1: 6234f860a tests: Test aborts due to missing requirements should be marked as error instead of skipped @@ tests/gzcompress_test.in: grubshell=@builddir@/grub-shell + exit 99 fi - if [ "$(echo hello | "${grubshell}" --mkrescue-arg=--compress=gz)" != "Hello World" ]; then + v=$(echo hello | "${grubshell}" --mkrescue-arg=--compress=gz) ## tests/hfs_test.in ## @@ tests/hfs_test.in: if [ "x$EUID" = "x" ] ; then @@ tests/lzocompress_test.in: grubshell=@builddir@/grub-shell + exit 99 fi - if [ "$(echo hello | "${grubshell}" --mkrescue-arg=--compress=lzo)" != "Hello World" ]; then + v=$(echo hello | "${grubshell}" --mkrescue-arg=--compress=lzo) ## tests/minixfs_test.in ## @@ tests/minixfs_test.in: if [ "x$EUID" = "x" ] ; then @@ tests/xzcompress_test.in: grubshell=@builddir@/grub-shell + exit 99 fi - if [ "$(echo hello | "${grubshell}" --mkrescue-arg=--compress=xz)" != "Hello World" ]; then + v=$(echo hello | "${grubshell}" --mkrescue-arg=--compress=xz) ## tests/zfs_test.in ## @@ tests/zfs_test.in: if [ "x$EUID" = "x" ] ; then 2: e35d3bb81 = 2: d1aefde55 tests: In partmap_test, use ${parted} variable when checking for binary -- 2.27.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] tests: Test aborts due to missing requirements should be marked as error instead of skipped 2021-10-14 20:58 [PATCH v2 0/2] More test fixes Glenn Washburn @ 2021-10-14 20:58 ` Glenn Washburn 2023-07-19 15:37 ` Julian Andres Klode 2021-10-14 20:58 ` [PATCH v2 2/2] tests: In partmap_test, use ${parted} variable when checking for binary Glenn Washburn 2021-10-18 18:08 ` [PATCH v2 0/2] More test fixes Daniel Kiper 2 siblings, 1 reply; 6+ messages in thread From: Glenn Washburn @ 2021-10-14 20:58 UTC (permalink / raw) To: grub-devel, Daniel Kiper; +Cc: Glenn Washburn Many tests abort due to not being root or missing tools, for instance mkfs commands for file system tests. The tests are exited with code 77, which means they were skipped. A skipped test is a test that should not be run, eg. a test specific to ARM64 should not be run on an x86 build. These aborts are actually a hard error, code 99. That means that the test could not be completed, but not because what was supposed to be tested failed, eg. in these cases where a missing tool prevents the running of a test. Signed-off-by: Glenn Washburn <development@efficientek.com> --- tests/btrfs_test.in | 4 ++-- tests/cpio_test.in | 2 +- tests/exfat_test.in | 4 ++-- tests/ext234_test.in | 8 ++++---- tests/f2fs_test.in | 4 ++-- tests/fat_test.in | 4 ++-- tests/gzcompress_test.in | 2 +- tests/hfs_test.in | 6 +++--- tests/hfsplus_test.in | 4 ++-- tests/iso9660_test.in | 2 +- tests/jfs_test.in | 4 ++-- tests/lzocompress_test.in | 2 +- tests/minixfs_test.in | 8 ++++---- tests/nilfs2_test.in | 4 ++-- tests/ntfs_test.in | 6 +++--- tests/partmap_test.in | 2 +- tests/reiserfs_test.in | 4 ++-- tests/romfs_test.in | 2 +- tests/squashfs_test.in | 2 +- tests/tar_test.in | 2 +- tests/udf_test.in | 4 ++-- tests/xfs_test.in | 4 ++-- tests/xzcompress_test.in | 2 +- tests/zfs_test.in | 4 ++-- 24 files changed, 45 insertions(+), 45 deletions(-) diff --git a/tests/btrfs_test.in b/tests/btrfs_test.in index 0c9bf3a68..0d098c9a2 100644 --- a/tests/btrfs_test.in +++ b/tests/btrfs_test.in @@ -7,12 +7,12 @@ if [ "x$EUID" = "x" ] ; then fi if [ "$EUID" != 0 ] ; then - exit 77 + exit 99 fi if ! which mkfs.btrfs >/dev/null 2>&1; then echo "mkfs.btrfs not installed; cannot test btrfs." - exit 77 + exit 99 fi "@builddir@/grub-fs-tester" btrfs diff --git a/tests/cpio_test.in b/tests/cpio_test.in index 5742cf17b..e2e668cf6 100644 --- a/tests/cpio_test.in +++ b/tests/cpio_test.in @@ -4,7 +4,7 @@ set -e if ! which cpio >/dev/null 2>&1; then echo "cpio not installed; cannot test cpio." - exit 77 + exit 99 fi "@builddir@/grub-fs-tester" cpio_bin diff --git a/tests/exfat_test.in b/tests/exfat_test.in index cd3cd4cb2..7939f25d2 100644 --- a/tests/exfat_test.in +++ b/tests/exfat_test.in @@ -7,12 +7,12 @@ if [ "x$EUID" = "x" ] ; then fi if [ "$EUID" != 0 ] ; then - exit 77 + exit 99 fi if ! which mkfs.exfat >/dev/null 2>&1; then echo "mkfs.exfat not installed; cannot test exFAT." - exit 77 + exit 99 fi "@builddir@/grub-fs-tester" exfat diff --git a/tests/ext234_test.in b/tests/ext234_test.in index 4f1eb527e..4df696710 100644 --- a/tests/ext234_test.in +++ b/tests/ext234_test.in @@ -7,22 +7,22 @@ if [ "x$EUID" = "x" ] ; then fi if [ "$EUID" != 0 ] ; then - exit 77 + exit 99 fi if ! which mkfs.ext2 >/dev/null 2>&1; then echo "mkfs.ext2 not installed; cannot test ext2." - exit 77 + exit 99 fi if ! which mkfs.ext3 >/dev/null 2>&1; then echo "mkfs.ext3 not installed; cannot test ext3." - exit 77 + exit 99 fi if ! which mkfs.ext4 >/dev/null 2>&1; then echo "mkfs.ext4 not installed; cannot test ext4." - exit 77 + exit 99 fi "@builddir@/grub-fs-tester" ext2_old diff --git a/tests/f2fs_test.in b/tests/f2fs_test.in index 8c415db61..85f8cc8bc 100644 --- a/tests/f2fs_test.in +++ b/tests/f2fs_test.in @@ -7,12 +7,12 @@ if [ "x$EUID" = "x" ] ; then fi if [ "$EUID" != 0 ] ; then - exit 77 + exit 99 fi if ! which mkfs.f2fs >/dev/null 2>&1; then echo "mkfs.f2fs not installed; cannot test f2fs." - exit 77 + exit 99 fi diff --git a/tests/fat_test.in b/tests/fat_test.in index b6b4748ca..8a2b37c5c 100644 --- a/tests/fat_test.in +++ b/tests/fat_test.in @@ -7,12 +7,12 @@ if [ "x$EUID" = "x" ] ; then fi if [ "$EUID" != 0 ] ; then - exit 77 + exit 99 fi if ! which mkfs.vfat >/dev/null 2>&1; then echo "mkfs.vfat not installed; cannot test FAT." - exit 77 + exit 99 fi "@builddir@/grub-fs-tester" vfat16a diff --git a/tests/gzcompress_test.in b/tests/gzcompress_test.in index d7a594bb2..8e7e6a633 100644 --- a/tests/gzcompress_test.in +++ b/tests/gzcompress_test.in @@ -21,7 +21,7 @@ grubshell=@builddir@/grub-shell if ! which gzip >/dev/null 2>&1; then echo "gzip not installed; cannot test gzip compression." - exit 77 + exit 99 fi v=$(echo hello | "${grubshell}" --mkrescue-arg=--compress=gz) diff --git a/tests/hfs_test.in b/tests/hfs_test.in index 5b0c5e33e..960f1cbd0 100644 --- a/tests/hfs_test.in +++ b/tests/hfs_test.in @@ -7,17 +7,17 @@ if [ "x$EUID" = "x" ] ; then fi if [ "$EUID" != 0 ] ; then - exit 77 + exit 99 fi if ! which mkfs.hfs >/dev/null 2>&1; then echo "mkfs.hfs not installed; cannot test HFS." - exit 77 + exit 99 fi if ! grep -q mac_roman /proc/modules && ! modprobe mac_roman; then echo "no mac-roman support; cannot test HFS." - exit 77 + exit 99 fi "@builddir@/grub-fs-tester" hfs diff --git a/tests/hfsplus_test.in b/tests/hfsplus_test.in index 85f1c37dc..f727cf0e2 100644 --- a/tests/hfsplus_test.in +++ b/tests/hfsplus_test.in @@ -7,12 +7,12 @@ if [ "x$EUID" = "x" ] ; then fi if [ "$EUID" != 0 ] ; then - exit 77 + exit 99 fi if ! which mkfs.hfsplus >/dev/null 2>&1; then echo "mkfs.hfsplus not installed; cannot test hfsplus." - exit 77 + exit 99 fi "@builddir@/grub-fs-tester" hfsplus diff --git a/tests/iso9660_test.in b/tests/iso9660_test.in index 571b938d7..ed0a5bf8d 100644 --- a/tests/iso9660_test.in +++ b/tests/iso9660_test.in @@ -4,7 +4,7 @@ set -e if ! which xorriso >/dev/null 2>&1; then echo "xorriso not installed; cannot test iso9660." - exit 77 + exit 99 fi "@builddir@/grub-fs-tester" joliet diff --git a/tests/jfs_test.in b/tests/jfs_test.in index 6cf7576b3..d13780e23 100644 --- a/tests/jfs_test.in +++ b/tests/jfs_test.in @@ -7,12 +7,12 @@ if [ "x$EUID" = "x" ] ; then fi if [ "$EUID" != 0 ] ; then - exit 77 + exit 99 fi if ! which mkfs.jfs >/dev/null 2>&1; then echo "mkfs.jfs not installed; cannot test JFS." - exit 77 + exit 99 fi "@builddir@/grub-fs-tester" jfs diff --git a/tests/lzocompress_test.in b/tests/lzocompress_test.in index 42e270df0..915f74bd9 100644 --- a/tests/lzocompress_test.in +++ b/tests/lzocompress_test.in @@ -21,7 +21,7 @@ grubshell=@builddir@/grub-shell if ! which lzop >/dev/null 2>&1; then echo "lzop not installed; cannot test lzo compression." - exit 77 + exit 99 fi v=$(echo hello | "${grubshell}" --mkrescue-arg=--compress=lzo) diff --git a/tests/minixfs_test.in b/tests/minixfs_test.in index 437d92df6..c62f56c8b 100644 --- a/tests/minixfs_test.in +++ b/tests/minixfs_test.in @@ -7,22 +7,22 @@ if [ "x$EUID" = "x" ] ; then fi if [ "$EUID" != 0 ] ; then - exit 77 + exit 99 fi if ! which mkfs.minix >/dev/null 2>&1; then echo "mkfs.minix not installed; cannot test minixfs." - exit 77 + exit 99 fi if ! mkfs.minix -h | grep -- -v > /dev/null; then echo "mkfs.minix doesn't support minix2fs; cannot test minix*fs." - exit 77 + exit 99 fi if ! mkfs.minix -h | grep -- -3 > /dev/null; then echo "mkfs.minix doesn't support minix3fs; cannot test minix*fs." - exit 77 + exit 99 fi "@builddir@/grub-fs-tester" minix diff --git a/tests/nilfs2_test.in b/tests/nilfs2_test.in index ad44d5b33..8cc93754c 100644 --- a/tests/nilfs2_test.in +++ b/tests/nilfs2_test.in @@ -7,12 +7,12 @@ if [ "x$EUID" = "x" ] ; then fi if [ "$EUID" != 0 ] ; then - exit 77 + exit 99 fi if ! which mkfs.nilfs2 >/dev/null 2>&1; then echo "mkfs.nilfs2 not installed; cannot test nilfs2." - exit 77 + exit 99 fi "@builddir@/grub-fs-tester" nilfs2 diff --git a/tests/ntfs_test.in b/tests/ntfs_test.in index 9eb7b01f6..c2b08d27f 100644 --- a/tests/ntfs_test.in +++ b/tests/ntfs_test.in @@ -7,17 +7,17 @@ if [ "x$EUID" = "x" ] ; then fi if [ "$EUID" != 0 ] ; then - exit 77 + exit 99 fi if ! which mkfs.ntfs >/dev/null 2>&1; then echo "mkfs.ntfs not installed; cannot test ntfs." - exit 77 + exit 99 fi if ! which setfattr >/dev/null 2>&1; then echo "setfattr not installed; cannot test ntfs." - exit 77 + exit 99 fi "@builddir@/grub-fs-tester" ntfs diff --git a/tests/partmap_test.in b/tests/partmap_test.in index 0043bcf4c..bc2be78aa 100644 --- a/tests/partmap_test.in +++ b/tests/partmap_test.in @@ -98,7 +98,7 @@ esac if ! which parted >/dev/null 2>&1; then echo "parted not installed; cannot test partmap" - exit 77 + exit 99 fi imgfile="`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX"`" || exit 99 diff --git a/tests/reiserfs_test.in b/tests/reiserfs_test.in index 8ecfe740b..37226c01b 100644 --- a/tests/reiserfs_test.in +++ b/tests/reiserfs_test.in @@ -7,12 +7,12 @@ if [ "x$EUID" = "x" ] ; then fi if [ "$EUID" != 0 ] ; then - exit 77 + exit 99 fi if ! which mkfs.reiserfs >/dev/null 2>&1; then echo "mkfs.reiserfs not installed; cannot test reiserfs." - exit 77 + exit 99 fi "@builddir@/grub-fs-tester" reiserfs diff --git a/tests/romfs_test.in b/tests/romfs_test.in index 98bb50c32..f968e9b7d 100644 --- a/tests/romfs_test.in +++ b/tests/romfs_test.in @@ -4,7 +4,7 @@ set -e if ! which genromfs >/dev/null 2>&1; then echo "genromfs not installed; cannot test romfs." - exit 77 + exit 99 fi "@builddir@/grub-fs-tester" romfs diff --git a/tests/squashfs_test.in b/tests/squashfs_test.in index 2f044f95d..15e70218f 100644 --- a/tests/squashfs_test.in +++ b/tests/squashfs_test.in @@ -4,7 +4,7 @@ set -e if ! which mksquashfs >/dev/null 2>&1; then echo "mksquashfs not installed; cannot test squashfs." - exit 77 + exit 99 fi "@builddir@/grub-fs-tester" squash4_gzip diff --git a/tests/tar_test.in b/tests/tar_test.in index 6e2f2de8b..97944b243 100644 --- a/tests/tar_test.in +++ b/tests/tar_test.in @@ -4,7 +4,7 @@ set -e if ! which tar >/dev/null 2>&1; then echo "tar not installed; cannot test tar." - exit 77 + exit 99 fi "@builddir@/grub-fs-tester" tarfs diff --git a/tests/udf_test.in b/tests/udf_test.in index fb92f0173..302b28ab2 100644 --- a/tests/udf_test.in +++ b/tests/udf_test.in @@ -7,12 +7,12 @@ if [ "x$EUID" = "x" ] ; then fi if [ "$EUID" != 0 ] ; then - exit 77 + exit 99 fi if ! which mkudffs >/dev/null 2>&1; then echo "mkudffs not installed; cannot test UDF." - exit 77 + exit 99 fi "@builddir@/grub-fs-tester" udf diff --git a/tests/xfs_test.in b/tests/xfs_test.in index 03a351359..5e029c182 100644 --- a/tests/xfs_test.in +++ b/tests/xfs_test.in @@ -7,12 +7,12 @@ if [ "x$EUID" = "x" ] ; then fi if [ "$EUID" != 0 ] ; then - exit 77 + exit 99 fi if ! which mkfs.xfs >/dev/null 2>&1; then echo "mkfs.xfs not installed; cannot test xfs." - exit 77 + exit 99 fi diff --git a/tests/xzcompress_test.in b/tests/xzcompress_test.in index cfc6ccba6..6ef73e41e 100644 --- a/tests/xzcompress_test.in +++ b/tests/xzcompress_test.in @@ -21,7 +21,7 @@ grubshell=@builddir@/grub-shell if ! which xz >/dev/null 2>&1; then echo "xz not installed; cannot test xz compression." - exit 77 + exit 99 fi v=$(echo hello | "${grubshell}" --mkrescue-arg=--compress=xz) diff --git a/tests/zfs_test.in b/tests/zfs_test.in index eee62c10d..58cc25b22 100644 --- a/tests/zfs_test.in +++ b/tests/zfs_test.in @@ -7,12 +7,12 @@ if [ "x$EUID" = "x" ] ; then fi if [ "$EUID" != 0 ] ; then - exit 77 + exit 99 fi if ! which zpool >/dev/null 2>&1; then echo "zpool not installed; cannot test zfs." - exit 77 + exit 99 fi "@builddir@/grub-fs-tester" zfs -- 2.27.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] tests: Test aborts due to missing requirements should be marked as error instead of skipped 2021-10-14 20:58 ` [PATCH v2 1/2] tests: Test aborts due to missing requirements should be marked as error instead of skipped Glenn Washburn @ 2023-07-19 15:37 ` Julian Andres Klode 2023-07-19 23:03 ` Glenn Washburn 0 siblings, 1 reply; 6+ messages in thread From: Julian Andres Klode @ 2023-07-19 15:37 UTC (permalink / raw) To: The development of GNU GRUB; +Cc: Daniel Kiper, Glenn Washburn On Thu, Oct 14, 2021 at 03:58:07PM -0500, Glenn Washburn wrote: > Many tests abort due to not being root or missing tools, for instance mkfs > commands for file system tests. The tests are exited with code 77, which > means they were skipped. A skipped test is a test that should not be run, > eg. a test specific to ARM64 should not be run on an x86 build. These aborts > are actually a hard error, code 99. That means that the test could not be > completed, but not because what was supposed to be tested failed, eg. in > these cases where a missing tool prevents the running of a test. This change is inappropriate. We can't just fail the build in distro packaging just because our builders don't run as root. Similarly dependencies - it's our choice which dependencies we install, if we don't care about stuff, it should just skip it. Every distro will have to revert this change presumably so they can build grub. -- debian developer - deb.li/jak | jak-linux.org - free software dev ubuntu core developer i speak de, en ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] tests: Test aborts due to missing requirements should be marked as error instead of skipped 2023-07-19 15:37 ` Julian Andres Klode @ 2023-07-19 23:03 ` Glenn Washburn 0 siblings, 0 replies; 6+ messages in thread From: Glenn Washburn @ 2023-07-19 23:03 UTC (permalink / raw) To: Julian Andres Klode; +Cc: The development of GNU GRUB, Daniel Kiper On Wed, 19 Jul 2023 17:37:55 +0200 Julian Andres Klode <julian.klode@canonical.com> wrote: > On Thu, Oct 14, 2021 at 03:58:07PM -0500, Glenn Washburn wrote: > > Many tests abort due to not being root or missing tools, for instance mkfs > > commands for file system tests. The tests are exited with code 77, which > > means they were skipped. A skipped test is a test that should not be run, > > eg. a test specific to ARM64 should not be run on an x86 build. These aborts > > are actually a hard error, code 99. That means that the test could not be > > completed, but not because what was supposed to be tested failed, eg. in > > these cases where a missing tool prevents the running of a test. > > This change is inappropriate. Yes, I can imagine this change is inappropriate for the _existing_ code that you (or your distro) has built around packaging GRUB. The previous code was actually inappropriate for the GRUB tests. The needs of the GRUB project and the needs of distros don't always align. I don't believe that is the problem here though, where the issue is misplaced blame. Let me be clear here, the purpose of GRUB tests is not so distros can run half of them after the build to see if it passes the smell test. The purpose of the tests are primarily to test regressions and functionality. To that end, the less tests that are run, the less useful the test suite. So the testing system should be implemented to try to run as many tests as possible and clearly note where tests are not being run. Of course, you're free to run tests how ever you want, but that does not mean that the GRUB project should support it, especially when its in conflict with the interests of the project. It occurs to me that perhaps the problematic implication of the previous behavior was not evident, so let me try to clarify for you and anyone else. A someone concerned with testing GRUB, I want to make sure that "make check" does as much testing as is available and I want to know with as much ease as possible when not all available testing is being done. The previous approach did not cut it. When I first started running the tests, before this change was included, I would get a lot of skipped tests. I would then need to dig into the individual test log to see why the test was skipped. Sometimes the test was skipped because it was supposed to be skipped, maybe that test did not apply to that architecture. I don't care about the skipped tests that should be skipped, and its a waste of my time to dig into those. Now tests that were skipped because of a misconfiguration of the testing environment, those I do care about because the test _should_ run but is not. Now suppose I saw skipped tests due to an environment issue and assumed they were supposed to be skipped, then I would be unknowingly not running the full test suite, which defeats the purpose of having those tests. The interpreting the test output should be clear to someone not familiar with the tests. And false positive fails are better than false negatives (not catching true failures). This isn't a theoretical issue. There is someone on this list that has several times graciously taken time to run the test suite and show the test results to the list. And while I very much appreciate the intention and work expended, in none of the cases was the output that useful because half the tests were not run properly due to an improperly testing environment. So in my mind it was mostly a waste of effort. By making it clear via hard error that the test is a test that should be run for the target, but is failing due to an environment issue, there is more information at a glance about what's being tested or not. This separates failed tests that are issues for GRUB, failed tests that are the testers responsibility, and tests that are not run because they should not be. You are advocating for going back to a time where those last two classes are merged into one. In the interests of testing GRUB, that is inappropriate. > We can't just fail the build in distro > packaging just because our builders don't run as root. Then don't do that. The implication that you _need_ to revert to improper test behavior is erroneous. Let assume that packaging GRUB implies running the tests because really that is a good idea and that your builders run unprivileged, also a good idea. The best option I see, with the limited knowledge I have of your situation and to resolve your issue without reverting the patch, is to check the test-suite.log file for ERRORs. This is incredibly simple. Assuming you have a line in the your debian rules file for "make check", you could changes this to something like: make check || grep '^# ERROR: 0$' test-suite.log This should get you the previous behavior. If you actually are serious about running the tests, I've built a system which runs the root required tests in UML. So I've got all the tests running as they should on a build box where I have no access to root. I'd also like to point out that even without this change, "make check" failed, continues to fail, and will likely fail for the foreseeable future. As I'm sure you're aware, the functional tests have been failing for a long time, its a known issue, and no one has stepped up to fix them (though a few have looked into it). So unless you have patches fixing this (patches welcome!) or you're already patching GRUB to not run this test, then "make check" is already failing for you. > Similarly dependencies - it's our choice which dependencies we install, > if we don't care about stuff, it should just skip it. I agree, up to the last 5 words. If you want to control which tests are run, it should not be done by abusing the automake testing system. Non-existence of an executable or package should not be a signal for disabling tests. Instead more appropriate way would be to specify exactly which tests you want to run via the TESTS variable[1]. It comes to mind that it might be nice to have an EXCLUDE_TESTS make variable for specifying tests to exclude, to be more explicit that just having a TESTS variable with some missing tests. Alternatively, propose a patch to automake for a variable IGNORE_HARD_ERRORS, a complement to the exiting DISABLE_HARD_ERRORS, which causes hard errors to affect the return code of make check. > Every distro will have to revert this change presumably so they can > build grub. Distros are free to modify upstream as they see fit, and do to the tune of over a hundred patches at times. However, your presumption that there is only one way to fix this is incorrect. As I noted above, this change need not incur another patch to GRUB source, if the tooling is fixed. Glenn [1] https://www.gnu.org/software/automake/manual/html_node/Scripts_002dbased-Testsuites.html ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] tests: In partmap_test, use ${parted} variable when checking for binary 2021-10-14 20:58 [PATCH v2 0/2] More test fixes Glenn Washburn 2021-10-14 20:58 ` [PATCH v2 1/2] tests: Test aborts due to missing requirements should be marked as error instead of skipped Glenn Washburn @ 2021-10-14 20:58 ` Glenn Washburn 2021-10-18 18:08 ` [PATCH v2 0/2] More test fixes Daniel Kiper 2 siblings, 0 replies; 6+ messages in thread From: Glenn Washburn @ 2021-10-14 20:58 UTC (permalink / raw) To: grub-devel, Daniel Kiper; +Cc: Glenn Washburn Signed-off-by: Glenn Washburn <development@efficientek.com> --- tests/partmap_test.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/partmap_test.in b/tests/partmap_test.in index bc2be78aa..a73c473b0 100644 --- a/tests/partmap_test.in +++ b/tests/partmap_test.in @@ -96,8 +96,8 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in ;; esac -if ! which parted >/dev/null 2>&1; then - echo "parted not installed; cannot test partmap" +if ! which ${parted} >/dev/null 2>&1; then + echo "${parted} not installed; cannot test partmap" exit 99 fi -- 2.27.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 0/2] More test fixes 2021-10-14 20:58 [PATCH v2 0/2] More test fixes Glenn Washburn 2021-10-14 20:58 ` [PATCH v2 1/2] tests: Test aborts due to missing requirements should be marked as error instead of skipped Glenn Washburn 2021-10-14 20:58 ` [PATCH v2 2/2] tests: In partmap_test, use ${parted} variable when checking for binary Glenn Washburn @ 2021-10-18 18:08 ` Daniel Kiper 2 siblings, 0 replies; 6+ messages in thread From: Daniel Kiper @ 2021-10-18 18:08 UTC (permalink / raw) To: Glenn Washburn; +Cc: grub-devel On Thu, Oct 14, 2021 at 03:58:06PM -0500, Glenn Washburn wrote: > Updates since v1: > * Rebase onto latest master > > The first patch changes the test exit status from skipped to hard error, for > tests which fail due to missing dependencies in the environment. > > The second is a trivial patch to parameterize all uses of parted in the > partmap_test. > > Glenn > > Glenn Washburn (2): > tests: Test aborts due to missing requirements should be marked as > error instead of skipped > tests: In partmap_test, use ${parted} variable when checking for > binary Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com> Daniel ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-07-19 23:04 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-14 20:58 [PATCH v2 0/2] More test fixes Glenn Washburn 2021-10-14 20:58 ` [PATCH v2 1/2] tests: Test aborts due to missing requirements should be marked as error instead of skipped Glenn Washburn 2023-07-19 15:37 ` Julian Andres Klode 2023-07-19 23:03 ` Glenn Washburn 2021-10-14 20:58 ` [PATCH v2 2/2] tests: In partmap_test, use ${parted} variable when checking for binary Glenn Washburn 2021-10-18 18:08 ` [PATCH v2 0/2] More test fixes Daniel Kiper
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.