Auto merge of #61655 - RalfJung:checktools, r=kennytm

checktools: unify grepping the TOOLSTATE file

The file was grepped twice but in a different way. This unifies the code to make sure it is consistent. Or were these deliberately not doing the same thing? That seems strange though.

I wouldn't know how to test these changes.

src/ci/docker/x86_64-gnu-tools/checktools.sh

@@ -35,12 +35,17 @@ set -e
 cat "$TOOLSTATE_FILE"
 echo
 
+# This function checks if a particular tool is *not* in status "test-pass".
+check_tool_failed() {
+    grep -vq '"'"$1"'":"test-pass"' "$TOOLSTATE_FILE"
+}
+
 # This function checks that if a tool's submodule changed, the tool's state must improve
 verify_status() {
     echo "Verifying status of $1..."
     if echo "$CHANGED_FILES" | grep -q "^M[[:blank:]]$2$"; then
         echo "This PR updated '$2', verifying if status is 'test-pass'..."
-        if grep -vq '"'"$1"'":"test-pass"' "$TOOLSTATE_FILE"; then
+        if check_tool_failed "$1"; then
             echo
             echo "⚠️ We detected that this PR updated '$1', but its tests failed."
             echo
@@ -55,14 +60,16 @@ verify_status() {
     fi
 }
 
-# deduplicates the submodule check and the assertion that on beta some tools MUST be passing
+# deduplicates the submodule check and the assertion that on beta some tools MUST be passing.
+# $1 should be "submodule_changed" to only check tools that got changed by this PR,
+# or "beta_required" to check all tools that have $2 set to "beta".
 check_dispatch() {
     if [ "$1" = submodule_changed ]; then
         # ignore $2 (branch id)
         verify_status $3 $4
     elif [ "$2" = beta ]; then
         echo "Requiring test passing for $3..."
-        if grep -q '"'"$3"'":"\(test\|build\)-fail"' "$TOOLSTATE_FILE"; then
+        if check_tool_failed "$3"; then
             exit 4
         fi
     fi