[pacman-dev] [PATCH 01/11] Add testrunner for makepkg-template
Signed-off-by: Florian Pritz
Signed-off-by: Florian Pritz
Signed-off-by: Florian Pritz
Signed-off-by: Florian Pritz
Signed-off-by: Florian Pritz
Signed-off-by: Florian Pritz
Signed-off-by: Florian Pritz
Signed-off-by: Florian Pritz
Signed-off-by: Florian Pritz
Signed-off-by: Florian Pritz
Signed-off-by: Florian Pritz
On 21.12.2014 16:11, Florian Pritz wrote:
+ "$scriptdir/makepkg-template" \ + --template-dir "$testdir/$testcase/templates" \ + -p "$testdir/$testcase/PKGBUILD" \ + -o "$testdir/$testcase/result" \ + &> "$testdir/$testcase/output_full" "${arguments[@]}"
Keeping those files in the testcase directory is actually a bad idea since they will be left over if you check out an older commit and they will confuse the test suite. I'll switch this to a tempdir.
+ exitcode=$? + + sed "$testdir/$testcase/output_full" >"$testdir/$testcase/output" \ + -e 's|./test/scripts/|./|' \ + -e 's| at '$scriptdir'/makepkg-template line [0-9]\+\.$||' + printf "%s\n" "$exitcode" >> "$testdir/$testcase/output"
This leads to a magic number in the expected_output file. I'll switch this and all testcases to use "exitcode: $exitcode" for more clarity. (won't resubmit them unless anything else changes)
Signed-off-by: Florian Pritz
On 12/21/14 at 04:43pm, Florian Pritz wrote:
Signed-off-by: Florian Pritz
--- v2: - use a temp dir for output files - mark the exit code in the expected_output file with "exitcode: " (tests are adjusted on my working branch)
Makefile.am | 2 + test/scripts/Makefile.am | 1 + test/scripts/makepkg-template_test.sh | 89 +++++++++++++++++++++++++++++++++++ 3 files changed, 92 insertions(+) create mode 100755 test/scripts/makepkg-template_test.sh
diff --git a/Makefile.am b/Makefile.am index e9b3dfa..ab669ef 100644 --- a/Makefile.am +++ b/Makefile.am @@ -28,6 +28,7 @@ $(top_srcdir)/test/pacman/tests/TESTS: $(wildcard test/pacman/tests/*.py*)
TESTS = test/scripts/parseopts_test.sh \ test/scripts/human_to_size_test.sh \ + test/scripts/makepkg-template_test.sh \ test/scripts/pacman-db-upgrade-v9.py \ test/util/pacsorttest.sh \ test/util/vercmptest.sh @@ -37,6 +38,7 @@ TEST_SUITE_LOG = test/test-suite.log TEST_EXTENSIONS = .py AM_TESTS_ENVIRONMENT = \ PMTEST_UTIL_DIR=$(top_builddir)/src/util/; export PMTEST_UTIL_DIR; \ + PMTEST_SCRIPT_DIR=$(top_srcdir)/scripts/; export PMTEST_SCRIPT_DIR; \ PMTEST_SCRIPTLIB_DIR=$(top_srcdir)/scripts/library/; export PMTEST_SCRIPTLIB_DIR; LOG_DRIVER = env AM_TAP_AWK='$(AWK)' $(SHELL) \ $(top_srcdir)/build-aux/tap-driver.sh diff --git a/test/scripts/Makefile.am b/test/scripts/Makefile.am index 8d6bc84..ed1a951 100644 --- a/test/scripts/Makefile.am +++ b/test/scripts/Makefile.am @@ -1,6 +1,7 @@ check_SCRIPTS = \ parseopts_test.sh \ pacman-db-upgrade-v9.py \ + makepkg-template_test.sh \ human_to_size_test.sh
noinst_SCRIPTS = $(check_SCRIPTS) diff --git a/test/scripts/makepkg-template_test.sh b/test/scripts/makepkg-template_test.sh new file mode 100755 index 0000000..5242f2c --- /dev/null +++ b/test/scripts/makepkg-template_test.sh @@ -0,0 +1,89 @@ +#!/bin/bash + +declare -i testcount=0 fail=0 pass=0 total=0 + +# source the library function +scriptdir=${1:-${PMTEST_SCRIPT_DIR}} +if [[ -z $scriptdir || ! -f $scriptdir/makepkg-template ]]; then + printf "Bail out! makepkg-template executable (%s) could not be located\n" "${scriptdir}/makepkg-template" + exit 1 +fi
This does not work the same as our other tests which allow passing the executable as the first argument.
+ +testdir="${0%/*}/makepkg-template-tests" + +total=$(find "$testdir" -maxdepth 1 -mindepth 1 -type d | wc -l) + +run_test() { + local testcase=$1 exitcode + local -i failed=0 + + (( ++testcount )) + + local -a arguments + + if [[ -f "$testdir/$testcase/arguments" ]]; then + arguments=($(cat "$testdir/$testcase/arguments")) + fi + + mkdir "$TMPDIR/$testcase" + + "$scriptdir/makepkg-template" \
makepkg-template uses translated messages; if you're going to check its output you need to set the locale.
+ --template-dir "$testdir/$testcase/templates" \ + -p "$testdir/$testcase/PKGBUILD" \ + -o "$TMPDIR/$testcase/result" \ + &> "$TMPDIR/$testcase/output_full" "${arguments[@]}" + exitcode=$? + + sed "$TMPDIR/$testcase/output_full" >"$TMPDIR/$testcase/output" \ + -e 's|./test/scripts/|./|' \
That should be $testdir or some variation of it.
+ -e 's| at '$scriptdir'/makepkg-template line [0-9]\+\.$||'
makepkg-template should be updated to not include line numbers in error messages rather than trying to filter them out.
+ printf "exitcode: %s\n" "$exitcode" >> "$TMPDIR/$testcase/output"
It would be better if we could test the exit code, output, and result independently.
+ + if [[ -f "$TMPDIR/$testcase/result" ]]; then + if ! diff -u "$TMPDIR/$testcase/result" "$testdir/$testcase/expected_result"; then
The diff output needs to be marked as diagnostic output. I'm working on a TAP generator for bash that we can use that will do that for us as well as taking care of all the counting.
+ failed=1 + fi + else + if [[ -f "$testdir/$testcase/expected_result" ]]; then + echo "$testcase: Found expected_result file but no result file" + failed=1 + fi + fi + + if ! diff -u "$TMPDIR/$testcase/output" "$testdir/$testcase/expected_output"; then + failed=1 + fi + + if ((!failed)); then + (( ++pass )) + printf "ok %d - %s\n" "$testcount" "$input"
I think you mean s/input/testcase/
+ else + (( ++fail )) + printf "not ok %d - %s\n" "$testcount" "$input" + printf '# [TEST %3s]: FAIL\n' "$testcount"
Redundant failure message.
+ fi +} + +summarize() { + rm -rf "$TMPDIR" + + if (( !fail )); then + printf '# All %s tests successful\n\n' "$testcount" + exit 0 + else + printf '# %s of %s tests failed\n\n' "$fail" "$testcount" + exit 1 + fi +} +TMPDIR="$(mktemp -d "/tmp/${0##*/}.XXXXXX")" +trap 'summarize' EXIT + +printf '# Beginning makepkg-template tests\n'
I know we have these messages in other tests, but they are completely useless, let's not add any more.
+ +echo "1..$total" + +for dir in "$testdir/"*; do + if [[ -d "$dir" ]]; then + run_test "${dir##*/}" + fi +done -- 2.2.1
Signed-off-by: Florian Pritz
+ -e 's| at '$scriptdir'/makepkg-template line [0-9]\+\.$||'
makepkg-template should be updated to not include line numbers in error messages rather than trying to filter them out.
I find line numbers rather helpful when debugging (which is also why I keep output-full around even though I currently provide no way to keep the tempdir), but I agree it's not the cleanest check.
+ printf "exitcode: %s\n" "$exitcode" >> "$TMPDIR/$testcase/output"
It would be better if we could test the exit code, output, and result independently.
result and output are already tested independently and adding the exit code to the output means I need less files per test case. Since I sometimes expect exit codes != 0 I need a way to check for a specific one and this seemed easiest. Would you want a dedicated file with just one number inside or maybe some kind of "config" file per testcase? That could also store additional arguments (currently "arguments" file, used only once I believe). It would add an additional file to every test case though.
+ + if [[ -f "$TMPDIR/$testcase/result" ]]; then + if ! diff -u "$TMPDIR/$testcase/result" "$testdir/$testcase/expected_result"; then
The diff output needs to be marked as diagnostic output. I'm working on a TAP generator for bash that we can use that will do that for us as well as taking care of all the counting.
Any ETA? I have not yet changed that. That said the test suite seems to put this output into the log file just fine AFAICT.
+ else + (( ++fail )) + printf "not ok %d - %s\n" "$testcount" "$input" + printf '# [TEST %3s]: FAIL\n' "$testcount"
Redundant failure message.
I copied that from human_to_size_test.sh. I have no idea how the testsuite actually works and why this was there initially. Makefile.am | 2 + test/scripts/Makefile.am | 1 + test/scripts/makepkg-template_test.sh | 91 +++++++++++++++++++++++++++++++++++ 3 files changed, 94 insertions(+) create mode 100755 test/scripts/makepkg-template_test.sh diff --git a/Makefile.am b/Makefile.am index e9b3dfa..ab669ef 100644 --- a/Makefile.am +++ b/Makefile.am @@ -28,6 +28,7 @@ $(top_srcdir)/test/pacman/tests/TESTS: $(wildcard test/pacman/tests/*.py*) TESTS = test/scripts/parseopts_test.sh \ test/scripts/human_to_size_test.sh \ + test/scripts/makepkg-template_test.sh \ test/scripts/pacman-db-upgrade-v9.py \ test/util/pacsorttest.sh \ test/util/vercmptest.sh @@ -37,6 +38,7 @@ TEST_SUITE_LOG = test/test-suite.log TEST_EXTENSIONS = .py AM_TESTS_ENVIRONMENT = \ PMTEST_UTIL_DIR=$(top_builddir)/src/util/; export PMTEST_UTIL_DIR; \ + PMTEST_SCRIPT_DIR=$(top_srcdir)/scripts/; export PMTEST_SCRIPT_DIR; \ PMTEST_SCRIPTLIB_DIR=$(top_srcdir)/scripts/library/; export PMTEST_SCRIPTLIB_DIR; LOG_DRIVER = env AM_TAP_AWK='$(AWK)' $(SHELL) \ $(top_srcdir)/build-aux/tap-driver.sh diff --git a/test/scripts/Makefile.am b/test/scripts/Makefile.am index 8d6bc84..ed1a951 100644 --- a/test/scripts/Makefile.am +++ b/test/scripts/Makefile.am @@ -1,6 +1,7 @@ check_SCRIPTS = \ parseopts_test.sh \ pacman-db-upgrade-v9.py \ + makepkg-template_test.sh \ human_to_size_test.sh noinst_SCRIPTS = $(check_SCRIPTS) diff --git a/test/scripts/makepkg-template_test.sh b/test/scripts/makepkg-template_test.sh new file mode 100755 index 0000000..46f5929 --- /dev/null +++ b/test/scripts/makepkg-template_test.sh @@ -0,0 +1,91 @@ +#!/bin/bash + +declare -i testcount=0 fail=0 pass=0 total=0 + +# source the library function +script=${1:-${PMTEST_SCRIPT_DIR}/makepkg-template} +if [[ -z $script || ! -f $script ]]; then + printf "Bail out! makepkg-template executable (%s) could not be located\n" "${script}" + exit 1 +fi + +testdir="${0%/*}/makepkg-template-tests" + +total=$(find "$testdir" -maxdepth 1 -mindepth 1 -type d | wc -l) + +run_test() { + local testcase=$1 exitcode + local -i failed=0 + + (( ++testcount )) + + local -a arguments + + if [[ -f "$testdir/$testcase/arguments" ]]; then + arguments=($(cat "$testdir/$testcase/arguments")) + fi + + mkdir "$TMPDIR/$testcase" + + LC_ALL=C "$script" \ + --template-dir "$testdir/$testcase/templates" \ + -p "$testdir/$testcase/PKGBUILD" \ + -o "$TMPDIR/$testcase/result" \ + &> "$TMPDIR/$testcase/output_full" "${arguments[@]}" + exitcode=$? + + perl -p <"$TMPDIR/$testcase/output_full" >"$TMPDIR/$testcase/output" \ + -e 's| at \Q'"$script"'\E line [0-9]+\.$||;' \ + + if [[ "${testdir%/*}" != "." ]]; then + perl -pi -e 's|\Q'"${testdir%/*}"'\E|.|g;' "$TMPDIR/$testcase/output" + fi + + printf "exitcode: %s\n" "$exitcode" >> "$TMPDIR/$testcase/output" + + if [[ -f "$TMPDIR/$testcase/result" ]]; then + if ! diff -u "$testdir/$testcase/expected_result" "$TMPDIR/$testcase/result"; then + failed=1 + fi + else + if [[ -f "$testdir/$testcase/expected_result" ]]; then + echo "$testcase: Found expected_result file but no result file" + failed=1 + fi + fi + + if ! diff -u "$testdir/$testcase/expected_output" "$TMPDIR/$testcase/output"; then + failed=1 + fi + + if ((!failed)); then + (( ++pass )) + printf "ok %d - %s\n" "$testcount" "$testcase" + else + (( ++fail )) + printf "not ok %d - %s\n" "$testcount" "$input" + printf '# [TEST %3s]: FAIL\n' "$testcount" + fi +} + +summarize() { + rm -rf "$TMPDIR" + + if (( !fail )); then + printf '# All %s tests successful\n\n' "$testcount" + exit 0 + else + printf '# %s of %s tests failed\n\n' "$fail" "$testcount" + exit 1 + fi +} +TMPDIR="$(mktemp -d "/tmp/${0##*/}.XXXXXX")" +trap 'summarize' EXIT + +echo "1..$total" + +for dir in "$testdir/"*; do + if [[ -d "$dir" ]]; then + run_test "${dir##*/}" + fi +done -- 2.2.1
Signed-off-by: Florian Pritz
Signed-off-by: Florian Pritz
Signed-off-by: Florian Pritz
Signed-off-by: Florian Pritz
Signed-off-by: Florian Pritz
Signed-off-by: Florian Pritz
Signed-off-by: Florian Pritz
Signed-off-by: Florian Pritz
Signed-off-by: Florian Pritz
Signed-off-by: Florian Pritz
Signed-off-by: Florian Pritz
On 12/28/14 at 04:23pm, Florian Pritz wrote:
Signed-off-by: Florian Pritz
--- v4: - adjusted to tap.sh - move expected output/result/exitcode/arguments into a single file called testcase-config - removed line number removal code (depends on "[PATCH] makepkg-template: Remove linenumber/file from errors" to be merged) - normalize paths by cd'ing into the testdir rather than using sed
The test files are simply adjusted to the changes to the main script, but I hope this is the last iteration so I'm posting them again for context.
Makefile.am | 2 + test/scripts/Makefile.am | 1 + .../testcase-config-template | 9 ++++ test/scripts/makepkg-template_test.sh | 52 ++++++++++++++++++++++ 4 files changed, 64 insertions(+) create mode 100644 test/scripts/makepkg-template-tests/testcase-config-template create mode 100755 test/scripts/makepkg-template_test.sh
diff --git a/Makefile.am b/Makefile.am index 01ea4f1..9dbacca 100644 --- a/Makefile.am +++ b/Makefile.am @@ -28,6 +28,7 @@ $(top_srcdir)/test/pacman/tests/TESTS: $(wildcard test/pacman/tests/*.py*)
TESTS = test/scripts/parseopts_test.sh \ test/scripts/human_to_size_test.sh \ + test/scripts/makepkg-template_test.sh \ test/scripts/pacman-db-upgrade-v9.py \ test/util/pacsorttest.sh \ test/util/vercmptest.sh @@ -37,6 +38,7 @@ TEST_SUITE_LOG = test/test-suite.log TEST_EXTENSIONS = .py AM_TESTS_ENVIRONMENT = \ PMTEST_UTIL_DIR=$(top_builddir)/src/util/; export PMTEST_UTIL_DIR; \ + PMTEST_SCRIPT_DIR=$(top_srcdir)/scripts/; export PMTEST_SCRIPT_DIR; \ PMTEST_SCRIPTLIB_DIR=$(top_srcdir)/scripts/library/; export PMTEST_SCRIPTLIB_DIR; LOG_DRIVER = env AM_TAP_AWK='$(AWK)' $(SHELL) \ $(top_srcdir)/build-aux/tap-driver.sh diff --git a/test/scripts/Makefile.am b/test/scripts/Makefile.am index 8d6bc84..ed1a951 100644 --- a/test/scripts/Makefile.am +++ b/test/scripts/Makefile.am @@ -1,6 +1,7 @@ check_SCRIPTS = \ parseopts_test.sh \ pacman-db-upgrade-v9.py \ + makepkg-template_test.sh \ human_to_size_test.sh
noinst_SCRIPTS = $(check_SCRIPTS) diff --git a/test/scripts/makepkg-template-tests/testcase-config-template b/test/scripts/makepkg-template-tests/testcase-config-template new file mode 100644 index 0000000..66267d3 --- /dev/null +++ b/test/scripts/makepkg-template-tests/testcase-config-template @@ -0,0 +1,9 @@ +arguments+=() +expected_exitcode=0 + +# set IFS="" if you want trailing new line(s), otherwise remove it +IFS="" read -d '' expected_output <<'EOF' +EOF + +IFS="" read -d '' expected_result <<'EOF' +EOF diff --git a/test/scripts/makepkg-template_test.sh b/test/scripts/makepkg-template_test.sh new file mode 100755 index 0000000..34185db --- /dev/null +++ b/test/scripts/makepkg-template_test.sh @@ -0,0 +1,52 @@ +#!/bin/bash + +source "$(dirname "$0")"/../tap.sh || exit 1 + +# source the library function
This comment does not apply here.
+script=$(readlink -f "${1:-${PMTEST_SCRIPT_DIR}/makepkg-template}")
This doesn't match our other test scripts which all fall back to looking for the program in PATH.
+if [[ -z $script || ! -f $script ]]; then + tap_bail "makepkg-template executable (%s) could not be located" "${script}" + exit 1 +fi + +# normalize paths +cd "${0%/*}" +testdir="./makepkg-template-tests" + +TMPDIR="$(mktemp -d "/tmp/${0##*/}.XXXXXX")" +trap "rm -rf '${TMPDIR}'" EXIT TERM + +total=$(find "$testdir" -maxdepth 1 -mindepth 1 -type d | wc -l) +[[ -n "$total" ]] || tap_bail "unable to determine total number of tests"
You need to exit after tap_bail.
+tap_plan "$((total*3))" + +run_test() { + local testcase=$1 exitcode expected_result expected_output + local -a arguments + local -i expected_exitcode=-1 + + [[ -f "$testdir/$testcase/testcase-config" ]] || continue + source "$testdir/$testcase/testcase-config" + + mkdir "$TMPDIR/$testcase" + touch "$TMPDIR/$testcase/result" + + LC_ALL=C "$script" \ + --template-dir "$testdir/$testcase/templates" \ + -p "$testdir/$testcase/PKGBUILD" \ + -o "$TMPDIR/$testcase/result" \ + &> "$TMPDIR/$testcase/output" "${arguments[@]}" + exitcode=$? + + tap_is_int "$exitcode" "$expected_exitcode" "$testcase exitcode" + tap_diff "$TMPDIR/$testcase/output" <(printf "%s" "$expected_output") "$testcase output" + tap_diff "$TMPDIR/$testcase/result" <(printf "%s" "$expected_result") "$testcase resulting PKGBUILD" +} + +for dir in "$testdir/"*; do + if [[ -d "$dir" ]]; then + run_test "${dir##*/}" + fi +done + +tap_finish -- 2.2.1
Signed-off-by: Florian Pritz
On 12/28/14 at 06:17pm, Florian Pritz wrote:
Signed-off-by: Florian Pritz
--- v5: - remove wrong comment - look for the executable in PATH - exit after tap_bail
Makefile.am | 2 + test/scripts/Makefile.am | 1 + .../testcase-config-template | 9 ++++ test/scripts/makepkg-template_test.sh | 56 ++++++++++++++++++++++ 4 files changed, 68 insertions(+) create mode 100644 test/scripts/makepkg-template-tests/testcase-config-template create mode 100755 test/scripts/makepkg-template_test.sh
...
diff --git a/test/scripts/makepkg-template_test.sh b/test/scripts/makepkg-template_test.sh new file mode 100755 index 0000000..60cbb0c --- /dev/null +++ b/test/scripts/makepkg-template_test.sh @@ -0,0 +1,56 @@ +#!/bin/bash + +source "$(dirname "$0")"/../tap.sh || exit 1 + +script=${1:-${PMTEST_SCRIPT_DIR}/makepkg-template}
That slash needs to be removed so that when PMTEST_SCRIPT_DIR isn't defined we'll look for "makepkg-template" not "/makepkg-template".
On 29.12.2014 04:50, Andrew Gregory wrote:
diff --git a/test/scripts/makepkg-template_test.sh b/test/scripts/makepkg-template_test.sh new file mode 100755 index 0000000..60cbb0c --- /dev/null +++ b/test/scripts/makepkg-template_test.sh @@ -0,0 +1,56 @@ +#!/bin/bash + +source "$(dirname "$0")"/../tap.sh || exit 1 + +script=${1:-${PMTEST_SCRIPT_DIR}/makepkg-template}
That slash needs to be removed so that when PMTEST_SCRIPT_DIR isn't defined we'll look for "makepkg-template" not "/makepkg-template".
Fixed on my working branch
On 29/12/14 19:52, Florian Pritz wrote:
On 29.12.2014 04:50, Andrew Gregory wrote:
diff --git a/test/scripts/makepkg-template_test.sh b/test/scripts/makepkg-template_test.sh new file mode 100755 index 0000000..60cbb0c --- /dev/null +++ b/test/scripts/makepkg-template_test.sh @@ -0,0 +1,56 @@ +#!/bin/bash + +source "$(dirname "$0")"/../tap.sh || exit 1 + +script=${1:-${PMTEST_SCRIPT_DIR}/makepkg-template}
That slash needs to be removed so that when PMTEST_SCRIPT_DIR isn't defined we'll look for "makepkg-template" not "/makepkg-template".
Fixed on my working branch
make distcheck: ERROR: test/scripts/makepkg-template_test.sh ============================================ Bail out! makepkg-template executable (../scripts/makepkg-template) could not be located ERROR: test/scripts/makepkg-template_test.sh - Bail out! makepkg-template executable (../scripts/makepkg-template) could not be located
autotools dislikes having symlinks in the tarball so create those
during testing. Also fix the value of PMTEST_SCRIPT_DIR to actually
point to the correct dir in the distcheck case (different build dir).
Signed-off-by: Florian Pritz
On 09/01/15 08:52, Florian Pritz wrote:
autotools dislikes having symlinks in the tarball so create those during testing. Also fix the value of PMTEST_SCRIPT_DIR to actually point to the correct dir in the distcheck case (different build dir).
Signed-off-by: Florian Pritz
Most of this patch looks like it should be integrated into the original patchset. Allan
participants (3)
-
Allan McRae
-
Andrew Gregory
-
Florian Pritz