Merge pull request #7261 from Martchus/unhandled-output-subrepo

mergify[bot] · web-flow · commit 91af0a8b3b44 · 2026-04-13T18:03:54.000Z
test: Check for unhandled output by default
diff --git a/Makefile b/Makefile
@@ -17,7 +17,7 @@ TESTS ?=
 # EXTRA_PROVE_ARGS: Additional prove arguments to pass
 EXTRA_PROVE_ARGS ?=
 # PROVE: Test application for Perl tests
-PROVE ?= prove
+PROVE ?= tools/prove_wrapper
 # Number of parallel jobs for prove
 PROVE_JOBS ?= $(shell nproc 2>/dev/null || echo 1)
 PROVE_JOBS_ARGS ?= -j$(PROVE_JOBS)
diff --git a/docs/Contributing.asciidoc b/docs/Contributing.asciidoc
@@ -328,16 +328,14 @@ We use annotations in some places to mark "uncoverable" code such as this:
 
 See the docs for details https://metacpan.org/pod/Devel::Cover
 
-To run the tests with a different command than `prove` you can set the
-`PROVE` variable in the Makefile. For example, in our CircleCI tests we
-use `tools/prove_wrapper` to run the tests. This wrapper is a simple script that
-will run `prove` and fail the test if there is unhandled output. This is useful
-to avoid having unhandled output in tests. To run the tests with
-`tools/prove_wrapper` you can do:
+We use `tools/prove_wrapper` to run the tests. This wrapper is a simple script
+that will run `prove` and fail the test if there is unhandled output. This is
+useful to avoid having unhandled output in tests. To run tests directly with
+`prove`, set the `PROVE` variable when invoking `make`, e.g.:
 
 [source,sh]
 ----
-PROVE=tools/prove_wrapper make test TESTS=t/foo.t
+PROVE=prove make test TESTS=t/foo.t
 ----
 
 There are some ways to save some time when executing local tests:
diff --git a/external/os-autoinst-common/.github/workflows/base-commit-message-checker.yml b/external/os-autoinst-common/.github/workflows/base-commit-message-checker.yml
@@ -7,8 +7,10 @@ jobs:
   base-check-commit-message:
     name: Check commit message
     runs-on: ubuntu-latest
+    permissions:
+      contents: read
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v6
         with:
           fetch-depth: 0
       - name: Install gitlint
diff --git a/external/os-autoinst-common/.github/workflows/commit-message-checker.yml b/external/os-autoinst-common/.github/workflows/commit-message-checker.yml
@@ -10,4 +10,6 @@ on:
 
 jobs:
   check-commit-message:
+    permissions:
+      contents: read
     uses: ./.github/workflows/base-commit-message-checker.yml
diff --git a/external/os-autoinst-common/.github/workflows/perl-author-tests.yml b/external/os-autoinst-common/.github/workflows/perl-author-tests.yml
@@ -5,10 +5,14 @@ on: [push, pull_request]
 
 jobs:
   perl-author-tests:
+    permissions:
+      contents: read
     runs-on: ubuntu-latest
     name: Perl author tests
     container:
       image: registry.opensuse.org/devel/openqa/containers/os-autoinst_dev
     steps:
-      - uses: actions/checkout@v4
-      - run: make test-author
+      - uses: actions/checkout@v6
+      - run: |
+          git config --system --add safe.directory "$GITHUB_WORKSPACE"
+          make test-author
diff --git a/external/os-autoinst-common/.github/workflows/perl-lint-checks.yml b/external/os-autoinst-common/.github/workflows/perl-lint-checks.yml
@@ -5,10 +5,12 @@ on: [push, pull_request]
 
 jobs:
   perl-lint-checks:
+    permissions:
+      contents: read
     runs-on: ubuntu-latest
     name: "Perltidy"
     container:
       image: registry.opensuse.org/devel/openqa/containers/os-autoinst_dev
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v6
       - run: GITHUB_ACTIONS=1 make test-tidy
diff --git a/external/os-autoinst-common/.github/workflows/test.yaml b/external/os-autoinst-common/.github/workflows/test.yaml
@@ -5,9 +5,11 @@ on: [push, pull_request]
 
 jobs:
   test:
+    permissions:
+      contents: read
     runs-on: ubuntu-latest
     container:
       image: perldocker/perl-tester
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v6
       - run: make test-t
diff --git a/external/os-autoinst-common/.github/workflows/yamllint.yml b/external/os-autoinst-common/.github/workflows/yamllint.yml
@@ -6,9 +6,11 @@ on: [push, pull_request]
 jobs:
   yaml-lint:
     runs-on: ubuntu-latest
+    permissions:
+      contents: read
     name: "YAML-lint"
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v6
       - uses: docker://registry.opensuse.org/home/okurz/container/containers/tumbleweed:yamllint
         with:
           entrypoint: make
diff --git a/external/os-autoinst-common/.gitrepo b/external/os-autoinst-common/.gitrepo
@@ -6,7 +6,7 @@
 [subrepo]
 	remote = git@github.com:os-autoinst/os-autoinst-common.git
 	branch = master
-	commit = 7bbf40822c49031520d5d287daf89b8fdfe21fb8
-	parent = 1377209de4a732645afc526176fd4a65dc4d60b6
+	commit = 4e71e20292cd76298e53e9726e4d2b49be0eb7e0
+	parent = e5bc742416a9bcb03b24716c89c3a23c380625df
 	method = merge
 	cmdver = 0.4.6
diff --git a/external/os-autoinst-common/.perlcriticrc b/external/os-autoinst-common/.perlcriticrc
@@ -24,10 +24,6 @@ add_themes = community
 max_nests = 4
 
 # == Community Policies
-# -- Test::Most brings in strict & warnings
-[Freenode::StrictWarnings]
-extra_importers = Test::Most
-
 # -- Test::Most brings in strict & warnings
 [Community::StrictWarnings]
 extra_importers = Test::Most
diff --git a/external/os-autoinst-common/Makefile b/external/os-autoinst-common/Makefile
@@ -1,3 +1,11 @@
+# PROVE: Test application for Perl tests
+PROVE ?= tools/prove_wrapper
+PROVE_JOBS ?= $(shell nproc 2>/dev/null || echo 1)
+PROVE_JOBS_ARGS ?= -j$(PROVE_JOBS)
+
+SH_FILES ?= $(shell file --mime-type $$(git ls-files) test/*.t | sed -n 's/^\(.*\):.*text\/x-shellscript.*$$/\1/p')
+SH_SHELLCHECK_FILES ?= $(shell file --mime-type * | sed -n 's/^\(.*\):.*text\/x-shellscript.*$$/\1/p')
+
 all: help
 
 .PHONY: help
@@ -12,8 +20,17 @@ update-deps: ## Update dependencies
 setup-hooks: ## Install pre-commit git hooks
 	pre-commit install --install-hooks -t commit-msg -t pre-commit
 
+shfmt: ## Format shell scripts via shfmt
+	shfmt -w ${SH_FILES}
+
+test-shellcheck: ## Run shfmt and shellcheck tests
+	@which shfmt >/dev/null 2>&1 || echo "Command 'shfmt' not found, can not execute shell script formating checks"
+	shfmt -d ${SH_FILES}
+	@which shellcheck >/dev/null 2>&1 || echo "Command 'shellcheck' not found, can not execute shell script checks"
+	if [ -n "${SH_SHELLCHECK_FILES}" ]; then shellcheck -x ${SH_SHELLCHECK_FILES}; fi
+
 .PHONY: test-checkstyle
-test-checkstyle: test-tidy test-yaml test-gitlint ## Run checkstyle checks
+test-checkstyle: test-tidy test-yaml test-gitlint test-shellcheck ## Run checkstyle checks
 
 .PHONY: test
 test: test-checkstyle test-author test-t ## Run all tests
@@ -35,8 +52,8 @@ test-gitlint: ## Run gitlint checks
 
 .PHONY: test-author
 test-author: ## Run author tests
-	prove -l -r xt/
+	"${PROVE}" $(PROVE_JOBS_ARGS) -l -r xt/
 
 .PHONY: test-t
 test-t: ## Run unit tests
-	prove -l -r t/
+	"${PROVE}" $(PROVE_JOBS_ARGS) -l -r t/
diff --git a/external/os-autoinst-common/dependencies.yaml b/external/os-autoinst-common/dependencies.yaml
@@ -23,6 +23,8 @@ develop_requires:
   perl(Perl::Critic::Community):
   perl(Test::Perl::Critic):
   python3-gitlint:
+  expect:  # for unbuffer
+  shfmt:
 
 cover_requires:
   perl(Devel::Cover):
diff --git a/external/os-autoinst-common/lib/OpenQA/Test/TimeLimit.pm b/external/os-autoinst-common/lib/OpenQA/Test/TimeLimit.pm
@@ -13,6 +13,7 @@ sub import {
     return if ($ENV{OPENQA_TEST_TIMEOUT_DISABLE} or $INC{'perl5db.pl'});
     $SCALE_FACTOR *= $ENV{OPENQA_TEST_TIMEOUT_SCALE_COVER} // 3 if Devel::Cover->can('report');
     $SCALE_FACTOR *= $ENV{OPENQA_TEST_TIMEOUT_SCALE_CI} // 2 if $ENV{CI};
+    $SCALE_FACTOR *= 5 if $ENV{HARNESS_IS_PARALLEL};
     $limit *= $SCALE_FACTOR;
     $SIG{ALRM} = sub { BAIL_OUT "test '$0' exceeds runtime limit of '$limit' seconds\n" };
     alarm $limit;
diff --git a/external/os-autoinst-common/t/01-compile-check-all.t b/external/os-autoinst-common/t/01-compile-check-all.t
@@ -0,0 +1,12 @@
+# Copyright SUSE LLC
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+use Test::Most;
+# We need :no_end_test here because otherwise it would output a no warnings
+# test for each of the modules, but with the same test number
+use Test::Warnings qw(:no_end_test :report_warnings);
+
+use Test::Compile;
+my $test = Test::Compile->new();
+$test->all_files_ok();
+$test->done_testing();
diff --git a/external/os-autoinst-common/tools/prove_wrapper b/external/os-autoinst-common/tools/prove_wrapper
@@ -4,28 +4,41 @@
 
 set -euo pipefail
 
-OUTPUT=$(mktemp)
+prove_cmd=(prove -I .)
+
+# skip the check for unhandled output when running in verbose mode
+for arg in "$@"; do [[ $arg =~ (-v|--verbose) ]] && TEST_VERBOSE=1 && break; done
+[[ ${TEST_VERBOSE:-} && $TEST_VERBOSE != 0 ]] && exec "${prove_cmd[@]}" "$@"
 
 echo "Running prove with TAP output check ..."
 
-prove -I . "$@" 2>&1 | tee "$OUTPUT"
+# use unbuffer if available and stdout is a terminal for immediate output
+[[ -t 1 ]] && unbuffer_cmd=("$(which unbuffer 2> /dev/null)") || unbuffer_cmd=()
+
+OUTPUT=$(mktemp)
+"${unbuffer_cmd[@]}" "${prove_cmd[@]}" "$@" 2>&1 | tee "$OUTPUT"
+
 STATUS=${PIPESTATUS[0]}
 
 if [ "$STATUS" -ne 0 ]; then
     echo "not ok - prove failed"
     exit "$STATUS"
 fi
 
-sed -E '
-  s/^\[[0-9]{2}:[0-9]{2}:[0-9]{2}\][[:space:]]*//
-  /^All tests successful\./,$d
-' "$OUTPUT" > "$OUTPUT.processed"
-
-UNHANDLED=$(grep -vE '^(x?t|t/compile)/.*([0-9]{2}-|)([[:alnum:]]|_|-)+\.t \.+' "$OUTPUT.processed" || true)
+UNHANDLED=$(sed --regexp-extended \
+    -e 's/^\[[0-9]{2}:[0-9]{2}:[0-9]{2}\][[:space:]]*//' \
+    -e '/x?t\/.*\.t\s*\.+/d' \
+    -e '/\s*[0-9]+\.\.[0-9]+.*/d' \
+    -e '/^# /d' \
+    -e '/Files=.*Tests=/d' \
+    -e '/Result: /d' \
+    -e '/All tests successful\./d' \
+    "$OUTPUT")
 
 if [ -n "$UNHANDLED" ]; then
-    echo "not ok - unhandled output found"
-    echo "Run with PROVE_COMMAND=tools/prove_wrapper to reproduce locally"
+    echo "not ok - unhandled output found:"
+    echo "$UNHANDLED"
+    echo "Run with PROVE=tools/prove_wrapper to reproduce locally"
     exit 1
 else
     echo "ok - no unhandled output found"
diff --git a/external/os-autoinst-common/xt/01-compile-check-all.t b/external/os-autoinst-common/xt/01-compile-check-all.t
diff --git a/tools/ci/run_unit_tests.sh b/tools/ci/run_unit_tests.sh
@@ -24,5 +24,4 @@ sudo chown -R $USER test-results
 export OPENQA_TEST_TIMEOUT_SCALE_CI=3
 export EXTRA_PROVE_ARGS="-v"
 export OPENQA_FULLSTACK_TEMP_DIR=$PWD/test-results/fullstack
-export PROVE="tools/prove_wrapper"
 make test-$target
