Unverified Commit b5cadfdf authored by Silvan Mosberger's avatar Silvan Mosberger Committed by GitHub
Browse files

bash: Add tests.withChecks passthru to run tests (#435033)

parents 0b9306b5 6d5da104
Loading
Loading
Loading
Loading
+100 −7
Original line number Diff line number Diff line
@@ -6,6 +6,10 @@
  updateAutotoolsGnuConfigScriptsHook,
  bison,
  util-linux,
  coreutils,
  libredirect,
  glibcLocales,
  gnused,

  interactive ? true,
  readline,
@@ -29,13 +33,13 @@ lib.warnIf (withDocs != null)
    bash: `.override { withDocs = true; }` is deprecated, the docs are always included.
  ''
  stdenv.mkDerivation
  rec {
  (fa: {
    pname = "bash${lib.optionalString interactive "-interactive"}";
    version = "5.3${patch_suffix}";
    version = "5.3${fa.patch_suffix}";
    patch_suffix = "p${toString (builtins.length upstreamPatches)}";

    src = fetchurl {
      url = "mirror://gnu/bash/bash-${lib.removeSuffix patch_suffix version}.tar.gz";
      url = "mirror://gnu/bash/bash-${lib.removeSuffix fa.patch_suffix fa.version}.tar.gz";
      hash = "sha256-DVzYaWX4aaJs9k9Lcb57lvkKO6iz104n6OnZ1VUPMbo=";
    };

@@ -137,8 +141,7 @@ lib.warnIf (withDocs != null)
      "SHOBJ_LIBS=-lbash"
    ];

    nativeCheckInputs = [ util-linux ];
    doCheck = false; # dependency cycle, needs to be interactive
    doCheck = false; # Can't be enabled by default due to dependency cycle, use passthru.tests.withChecks instead

    postInstall = ''
      ln -s bash "$out/bin/sh"
@@ -160,6 +163,96 @@ lib.warnIf (withDocs != null)
    passthru = {
      shellPath = "/bin/bash";
      tests.static = pkgsStatic.bash;
      tests.withChecks = fa.finalPackage.overrideAttrs (attrs: {
        doCheck = true;

        nativeCheckInputs = attrs.nativeCheckInputs or [ ] ++ [
          util-linux
          libredirect.hook
          glibcLocales
          gnused
        ];

        meta = attrs.meta // {
          # Ignore Darwin for now, because the tests fail in many more ways than on Linux
          broken = attrs.meta.broken or false || stdenv.buildPlatform.isDarwin;
        };

        patches = attrs.patches or [ ] ++ [
          # See commit comment, also submitted upstream: https://lists.gnu.org/archive/html/bug-bash/2025-10/msg00054.html
          ./fail-tests.patch
          # See commit comment, also submitted upstream: https://lists.gnu.org/archive/html/bug-bash/2025-10/msg00055.html
          ./failed-tests-output.patch
          # The run-builtins test _almost_ succeeds, only has a bit of PATH trouble
          # and some odd terminal column mismatch
          ./fix-builtins-tests.patch
          # The run-invocation test _almost_ succeeds, only has a bit of PATH trouble
          ./fix-invocation-tests.patch
        ];

        preCheck = attrs.preCheck or "" + ''
          # Allows looking at actual outputs for failed tests
          export BASH_TSTOUT_KEEPDIR=$(mktemp -d)
          export HOME=$(mktemp -d)
          export NIX_REDIRECTS=${
            lib.concatMapAttrsStringSep ":" (name: value: "${name}=${value}") {
              "/bin/echo" = lib.getExe' coreutils "echo";
              "/bin/cat" = lib.getExe' coreutils "cat";
              "/bin/rm" = lib.getExe' coreutils "rm";
              "/usr" = "$(mktemp -d)";
            }
          }

          disabled_checks=(
            # Unsets PATH and breaks, not clear
            run-execscript

            # Fails on ZFS & needs a ja_JP.SJIS locale, which glibcLocales doesn't have
            run-intl

            # These error with "echo: write error: Broken pipe"
            run-histexpand
            run-lastpipe
            run-comsub
            run-comsub2

            # For some reason has an extra 'declare -x version="5.2p37"'
            run-nameref

            # These print some extra 'trap -- ''' SIGPIPE'
            run-trap
            run-varenv

            # These rely on /dev/tty
            run-read
            run-test
            run-vredir

            # Might also be related to not having a tty: "Inappropriate ioctl for device"
            run-history

            # Can be enabled in 5.4
            run-printf

            # This is probably fixable without too much trouble, but just not having a hardcoded PATH in type5.sub doesn't cut it
            # 142,143c142,147
            # < type5.sub: line 23: mkdir: command not found
            # < type5.sub: line 24: cd: /build/type-23722: No such file or directory
            # ---
            # > cat is /bin/cat
            # > cat is aliased to `echo cat'
            # > /bin/cat
            # > break is a shell builtin
            # > break is a special shell builtin
            # > ./e
            run-type
          )
          for check in "''${disabled_checks[@]}"; do
            # Exit before running the test script
            sed -i "1iecho 'Skipping test $check' >&2 && exit 0" "tests/$check"
          done
        '';
      });
    };

    meta = with lib; {
@@ -181,7 +274,7 @@ lib.warnIf (withDocs != null)
      platforms = platforms.all;
      # https://github.com/NixOS/nixpkgs/issues/333338
      badPlatforms = [ lib.systems.inspect.patterns.isMinGW ];
      maintainers = [ ];
      maintainers = with lib.maintainers; [ infinisil ];
      mainProgram = "bash";
      identifiers.cpeParts =
        let
@@ -194,4 +287,4 @@ lib.warnIf (withDocs != null)
          update = lib.elemAt versionSplit 2;
        };
    };
  }
  })
+58 −0
Original line number Diff line number Diff line
From 750cfd9af2174e1fa8164e433be1379e3e367c78 Mon Sep 17 00:00:00 2001
From: Silvan Mosberger <silvan.mosberger@moduscreate.com>
Date: Tue, 7 Oct 2025 15:01:00 +0200
Subject: [PATCH 1/2] Fail run-all on non-zero exit codes and summarise the
 results

Previously it was hard to determine whether the tests passed or not.
With this change, run-all exiting with 0 means that no tests failed.

This allows us to set up CI for our bash package in the NixOS distribution:
https://github.com/NixOS/nixpkgs/pull/435033

We hope that it will also be useful for other distributions to make sure
their packaging is correct.

Co-Authored-By: Silvan Mosberger <silvan.mosberger@tweag.io>
Co-Authored-By: Aleksander Bantyev <alexander.bantyev@tweag.io>
---
 tests/run-all | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git tests/run-all tests/run-all
index f9dfa604..c8f503a2 100644
--- tests/run-all
+++ tests/run-all
@@ -58,13 +58,28 @@ rm -f ${BASH_TSTOUT}
 
 echo Any output from any test, unless otherwise noted, indicates a possible anomaly
 
+passed=0
+total=0
 for x in run-*
 do
 	case $x in
 	$0|run-minimal|run-gprof)	;;
 	*.orig|*~) ;;
-	*)	echo $x ; sh $x ; rm -f ${BASH_TSTOUT} ;;
+	*)
+		echo $x
+		total=$(( total + 1 ))
+		if sh $x; then
+			passed=$(( passed + 1 ))
+		else
+			echo "$x EXITED NON-ZERO ($?) - possible anomaly unless otherwise noted"
+		fi
+		rm -f "${BASH_TSTOUT}"
+		;;
 	esac
 done
 
+echo "$passed/$total tests had exit code zero"
+if [ "$passed" -ne "$total" ]; then
+	exit 1
+fi
 exit 0
-- 
2.49.0
+40 −0
Original line number Diff line number Diff line
From 30c4d7fb34a09cf0a711c43231bd13fc380844d0 Mon Sep 17 00:00:00 2001
From: Silvan Mosberger <silvan.mosberger@moduscreate.com>
Date: Tue, 7 Oct 2025 15:08:49 +0200
Subject: [PATCH 2/2] allow keeping output of potentially failed tests

Previously if a test produced a diff, the only way to fix it was to
inspect the test output.

We introduce the BASH_TSTOUT_KEEPDIR environment variable which is
expected to point to a directory.
If set, that directory will be populated with output from all failed
tests, with test names as file names.

This makes debugging or updating the expected test outputs easier.

Co-Authored-By: Silvan Mosberger <silvan.mosberger@tweag.io>
Co-Authored-By: Aleksander Bantyev <alexander.bantyev@tweag.io>
---
 tests/run-all | 5 +++++
 1 file changed, 5 insertions(+)

diff --git tests/run-all tests/run-all
index c8f503a2..5d769d0f 100644
--- tests/run-all
+++ tests/run-all
@@ -72,6 +72,11 @@ do
 			passed=$(( passed + 1 ))
 		else
 			echo "$x EXITED NON-ZERO ($?) - possible anomaly unless otherwise noted"
+			if [ "${BASH_TSTOUT_KEEPDIR+set}" = "set" ]; then
+				cp "${BASH_TSTOUT}" "${BASH_TSTOUT_KEEPDIR}/$x"
+			else
+				echo "Hint: set BASH_TSTOUT_KEEPDIR environment variable to a directory to keep test output there"
+			fi
 		fi
 		rm -f "${BASH_TSTOUT}"
 		;;
-- 
2.49.0
+26 −0
Original line number Diff line number Diff line
diff --git tests/builtins.right tests/builtins.right
index d708c18..32b2cfd 100644
--- tests/builtins.right
+++ tests/builtins.right
@@ -362,7 +362,7 @@ A star (*) next to a name means that the command is disabled.
  complete [-abcdefgjksuv] [-pr] [-DEI]>  set [-abefhkmnptuvxBCEHPT] [-o optio>
  compopt [-o|+o option] [-DEI] [name .>  shift [n]
  continue [n]                            shopt [-pqsu] [-o] [optname ...]
- coproc [NAME] command [redirections]    source [-p path] filename [argument>
+ coproc [NAME] command [redirections]    source [-p path] filename [arguments>
  declare [-aAfFgiIlnrtux] [name[=value>  suspend [-f]
  dirs [-clpv] [+N] [-N]                  test [expr]
  disown [-h] [-ar] [jobspec ... | pid >  time [-p] pipeline
diff --git tests/source8.sub tests/source8.sub
index a6826da..fc4ae8c 100644
--- tests/source8.sub
+++ tests/source8.sub
@@ -15,7 +15,7 @@
 # test various uses of source -p
 
 : ${THIS_SH:=./bash}
-PATH=/bin:/usr/bin:/sbin:/usr/sbin
+PATH="$(dirname "$(type -P mkdir)")"
 
 : ${TMPDIR:=/var/tmp}
 export TDIR=${TMPDIR}/cwd-$$
+13 −0
Original line number Diff line number Diff line
diff --git tests/invocation.tests tests/invocation.tests
index c629c29..537e470 100644
--- tests/invocation.tests
+++ tests/invocation.tests
@@ -71,7 +71,7 @@ command cd -L $TDIR
 ./x23
 
 # this should result in a cannot execute binary file error since ls is in $PATH
-PATH=/bin:/usr/bin
+#PATH=/bin:/usr/bin
 ${THIS_SH} ls |& sed 's|^.*: ||'
 
 cd $SAVEPWD