Merge remote-tracking branch 'origin/topic/awelzel/force-still-fails-tests'

* origin/topic/awelzel/force-still-fails-tests:
  Reduce to 2 CPUs per task for CI tasks
  Fix upgrade using test_command of old version
  Change --force to fail on failing tests

Author: awelzel

.cirrus.yml

@@ -1,4 +1,4 @@
-cpus: &CPUS 8
+cpus: &CPUS 2
 
 resources_template: &RESOURCES_TEMPLATE
   cpu: *CPUS

CHANGES

@@ -1,3 +1,35 @@
+2.13.0-30 | 2023-04-21 20:37:27 +0200
+
+  * Reduce to 2 CPUs per task for CI tasks (Arne Welzel, Corelight)
+
+    Specifying 8 CPUs and two tasks immediately allocates as many resources
+    as are available for free for a single user on Cirrus. Depending on what
+    else a user is using Cirrus for, execution of zkg tasks may be delayed
+    for significant amounts of time.
+
+    The test-suite does not run such a long duration that the 4x reduction
+    in CPUs should have a practical impact.
+
+  * Fix upgrade using test_command of old version (Arne Welzel, Corelight)
+
+  * GH-137: Change --force to fail on failing tests (Arne Welzel, Corelight)
+
+    This is a user-visible change in behavior, but to something sane, so
+    hopefully that's acceptable.
+
+    Currently, `--force` is oftentimes used in scenarios where it should mean
+    "non-interactive". At the same time, `--force` also implies to ignore
+    failing test commands. This is not reasonable behavior, particularly as
+    `--skiptests` is available. This patch changes the behavior and causes
+    install/upgrade to exit with a failure code when tests fail even with
+    `--force` provided.
+
+    If someone intentionally wants to install a package with broken tests,
+    they can/should use `--skiptests` explicitly instead. That is less
+    surprising and stands out more.
+
+    Fixes #137
+
 2.13.0-26 | 2023-02-03 09:23:59 +0100
 
   * Fix zkg bundled usage (Arne Welzel, Corelight)

VERSION

@@ -1 +1 @@
-2.13.0-26
+2.13.0-30

doc/man/zkg.1

@@ -27,7 +27,7 @@ level margin: \\n[rst2man-indent\\n[rst2man-indent-level]]
 .\" new: \\n[rst2man-indent\\n[rst2man-indent-level]]
 .in \\n[rst2man-indent\\n[rst2man-indent-level]]u
 ..
-.TH "ZKG" "1" "Jan 17, 2023" "2.13.0-24" "Zeek Package Manager"
+.TH "ZKG" "1" "Apr 21, 2023" "2.13.0-30" "Zeek Package Manager"
 .SH NAME
 zkg \- Zeek Package Manager
 .sp
@@ -38,10 +38,7 @@ A command\-line package manager for Zeek.
 .sp
 .nf
 .ft C
-usage: zkg [\-h] [\-\-version] [\-\-configfile FILE | \-\-user] [\-\-verbose]
-           [\-\-extra\-source NAME=URL]
-           {test,install,bundle,unbundle,remove,uninstall,purge,refresh,upgrade,load,unload,pin,unpin,list,search,info,config,autoconfig,env,create,template}
-           ...
+usage: zkg [\-h] [\-\-version] [\-\-configfile FILE | \-\-user] [\-\-verbose] [\-\-extra\-source NAME=URL] {test,install,bundle,unbundle,remove,uninstall,purge,refresh,upgrade,load,unload,pin,unpin,list,search,info,config,autoconfig,env,create,template} ...
 .ft P
 .fi
 .UNINDENT
@@ -119,9 +116,7 @@ Installs packages from a configured package source or directly from a git URL.
 .sp
 .nf
 .ft C
-usage: zkg install [\-h] [\-\-skiptests] [\-\-nodeps] [\-\-nosuggestions]
-                   [\-\-version VERSION] [\-\-force] [\-\-user\-var NAME=VAL]
-                   package [package ...]
+usage: zkg install [\-h] [\-\-skiptests] [\-\-nodeps] [\-\-nosuggestions] [\-\-version VERSION] [\-\-force] [\-\-user\-var NAME=VAL] package [package ...]
 .ft P
 .fi
 .UNINDENT
@@ -229,9 +224,7 @@ This command creates a bundle file containing a collection of Zeek packages.  If
 .sp
 .nf
 .ft C
-usage: zkg bundle [\-h] [\-\-force] [\-\-nodeps] [\-\-nosuggestions]
-                  [\-\-manifest MANIFEST [MANIFEST ...] \-\-]
-                  filename.bundle
+usage: zkg bundle [\-h] [\-\-force] [\-\-nodeps] [\-\-nosuggestions] [\-\-manifest MANIFEST [MANIFEST ...] \-\-] filename.bundle
 .ft P
 .fi
 .UNINDENT
@@ -270,8 +263,7 @@ This command unpacks a bundle file formerly created by the \fBbundle\fP command
 .sp
 .nf
 .ft C
-usage: zkg unbundle [\-h] [\-\-replace] [\-\-force] [\-\-user\-var NAME=VAL]
-                    filename.bundle
+usage: zkg unbundle [\-h] [\-\-replace] [\-\-force] [\-\-user\-var NAME=VAL] filename.bundle
 .ft P
 .fi
 .UNINDENT
@@ -307,8 +299,7 @@ Retrieve latest package metadata from sources and checks whether any installed p
 .sp
 .nf
 .ft C
-usage: zkg refresh [\-h] [\-\-aggregate] [\-\-fail\-on\-aggregate\-problems] [\-\-push]
-                   [\-\-sources SOURCES [SOURCES ...]]
+usage: zkg refresh [\-h] [\-\-aggregate] [\-\-fail\-on\-aggregate\-problems] [\-\-push] [\-\-sources SOURCES [SOURCES ...]]
 .ft P
 .fi
 .UNINDENT
@@ -340,9 +331,7 @@ Uprades the specified package(s) to latest available version.  If no specific pa
 .sp
 .nf
 .ft C
-usage: zkg upgrade [\-h] [\-\-skiptests] [\-\-nodeps] [\-\-nosuggestions] [\-\-force]
-                   [\-\-user\-var NAME=VAL]
-                   [package ...]
+usage: zkg upgrade [\-h] [\-\-skiptests] [\-\-nodeps] [\-\-nosuggestions] [\-\-force] [\-\-user\-var NAME=VAL] [package ...]
 .ft P
 .fi
 .UNINDENT
@@ -493,8 +482,7 @@ Outputs a list of packages that match a given category.
 .sp
 .nf
 .ft C
-usage: zkg list [\-h] [\-\-nodesc]
-                [{all,installed,not_installed,loaded,unloaded,outdated}]
+usage: zkg list [\-h] [\-\-nodesc] [{all,installed,not_installed,loaded,unloaded,outdated}]
 .ft P
 .fi
 .UNINDENT
@@ -549,9 +537,7 @@ Shows detailed information/metadata for given packages. If the package is curren
 .sp
 .nf
 .ft C
-usage: zkg info [\-h] [\-\-version VERSION] [\-\-nolocal] [\-\-json]
-                [\-\-jsonpretty SPACES] [\-\-allvers]
-                package [package ...]
+usage: zkg info [\-h] [\-\-version VERSION] [\-\-nolocal] [\-\-json] [\-\-jsonpretty SPACES] [\-\-allvers] package [package ...]
 .ft P
 .fi
 .UNINDENT
@@ -593,8 +579,7 @@ The default output of this command is a valid package manager config file that c
 .sp
 .nf
 .ft C
-usage: zkg config [\-h]
-                  [{all,sources,user_vars,state_dir,script_dir,plugin_dir,bin_dir,zeek_dist,bro_dist}]
+usage: zkg config [\-h] [{all,sources,user_vars,state_dir,script_dir,plugin_dir,bin_dir,zeek_dist,bro_dist}]
 .ft P
 .fi
 .UNINDENT
@@ -658,9 +643,7 @@ This command creates a new Zeek package in the directory provided via \-\-packag
 .sp
 .nf
 .ft C
-usage: zkg create [\-h] \-\-packagedir DIR [\-\-version VERSION]
-                  [\-\-features FEATURE [FEATURE ...]] [\-\-template URL]
-                  [\-\-force] [\-\-user\-var NAME=VAL]
+usage: zkg create [\-h] \-\-packagedir DIR [\-\-version VERSION] [\-\-features FEATURE [FEATURE ...]] [\-\-template URL] [\-\-force] [\-\-user\-var NAME=VAL]
 .ft P
 .fi
 .UNINDENT
@@ -698,9 +681,7 @@ This command shows versions and supported features for a given package.
 .sp
 .nf
 .ft C
-usage: zkg template info [\-h] [\-\-json] [\-\-jsonpretty SPACES]
-                         [\-\-version VERSION]
-                         [URL]
+usage: zkg template info [\-h] [\-\-json] [\-\-jsonpretty SPACES] [\-\-version VERSION] [URL]
 .ft P
 .fi
 .UNINDENT
@@ -730,7 +711,7 @@ The template version to report on.  A version tag, branch name, or commit hash m
 .SH CONFIG FILE
 .sp
 The \fBzkg\fP command\-line tool uses an INI\-format config file to allow
-users to customize their Package Sources, Package installation paths, Zeek executable/source paths, and other
+users to customize their \fI\%Package Sources\fP, \fI\%Package\fP installation paths, Zeek executable/source paths, and other
 \fBzkg\fP options.
 .sp
 See the default/example config file below for explanations of the

testing/baselines/tests.install-force-build-fail/out

@@ -0,0 +1,7 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+Installing "one<...>/foo"
+error: incomplete installation, the follow packages failed to be installed:
+  one<...>/foo (main)
+Failed installing "one<...>/foo": package build_command failed, see log in <...>/foo-build.log
+have_load_bar=1
+have_load_foo=0

testing/baselines/tests.install-force-test-fail/out

@@ -0,0 +1,10 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+=== install without skiptests
+error: failed to run tests for one<...>/foo: test_command failed with exit code 1
+Running unit tests for "one<...>/foo"
+have_load_foo=0
+=== install with skiptests
+Installing "one<...>/foo"
+Installed "one<...>/foo" (main)
+Loaded "one<...>/foo"
+have_load_foo=1

testing/baselines/tests.upgrade-test-fail/out

@@ -0,0 +1,10 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+Installing "one<...>/foo"
+Installed "one<...>/foo" (1.0.2)
+Loaded "one<...>/foo"
+=== upgrade without skiptests
+error: failed to run tests for one<...>/foo: test_command failed with exit code 1
+Running unit tests for "one<...>/foo"
+=== upgrade with skiptest
+Upgraded "one<...>/foo" (1.0.3)
+print "foo 1.0.3";

testing/tests/install-force-build-fail

@@ -0,0 +1,16 @@
+# @TEST-DOC: Test that a failing build_command fails the package installation, no matter what.
+# @TEST-REQUIRES: type zeek-config
+#
+# @TEST-EXEC: bash %INPUT
+
+# @TEST-EXEC: zkg install bar
+#
+# @TEST-EXEC-FAIL: zkg -v install --force --skiptests foo >>out 2>&1
+# @TEST-EXEC: echo "have_load_bar=$(grep -c '@load.*bar' ./scripts/packages/packages.zeek)" >>out
+# @TEST-EXEC: echo "have_load_foo=$(grep -c '@load.*foo' ./scripts/packages/packages.zeek)" >>out
+#
+# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff out
+
+cd packages/foo
+echo 'build_command = exit 1' >> zkg.meta
+git commit -am 'build fails'

testing/tests/install-force-test-fail

@@ -0,0 +1,19 @@
+# @TEST-DOC: Test that a failing test_command with --force causes zkg install to fail and the package is not loaded. Also, verify that --skiptests can override that.
+# @TEST-REQUIRES: type zeek-config
+#
+# @TEST-EXEC: bash %INPUT
+
+# @TEST-EXEC: echo "=== install without skiptests" >>out
+# @TEST-EXEC-FAIL: zkg -v install --force foo >>out 2>&1
+# @TEST-EXEC: echo "have_load_foo=$(grep -c '@load.*foo' ./scripts/packages/packages.zeek)" >>out
+#
+# @TEST-EXEC: rm -rf ./state
+# @TEST-EXEC: echo "=== install with skiptests" >>out
+# @TEST-EXEC: zkg -v install --force --skiptests foo >>out 2>&1
+# @TEST-EXEC: echo "have_load_foo=$(grep -c '@load.*foo' ./scripts/packages/packages.zeek)" >>out
+#
+# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff out
+
+cd packages/foo
+echo 'test_command = exit 1' >> zkg.meta
+git commit -am 'add a test'

testing/tests/upgrade-test-fail

@@ -0,0 +1,31 @@
+# @TEST-DOC: Upgrading fails when a test_comamnd fails, even with --force.
+# @TEST-REQUIRES: type zeek-config
+# @TEST-EXEC: (cd packages/foo && echo 'print "foo 1.0.2";' >> __load__.zeek && git tag -a 1.0.2 -m 1.0.2 )
+
+# @TEST-EXEC: zkg install foo >>out 2>&1
+
+# @TEST-EXEC: bash improve-foo-tag-1.0.3
+
+# @TEST-EXEC: zkg refresh
+#
+# @TEST-EXEC: echo "=== upgrade without skiptests" >>out
+# @TEST-EXEC-FAIL: zkg upgrade --force >>out 2>&1
+
+# The upgrade failed, no 1.0.3 to be found in the __load__.zeek file
+# @TEST-EXEC-FAIL: grep -F '1.0.3' scripts/packages/foo/__load__.zeek >>out
+#
+# @TEST-EXEC: echo "=== upgrade with skiptest" >>out
+# @TEST-EXEC: zkg upgrade --force --skiptests >>out 2>&1
+# @TEST-EXEC: grep -F '1.0.3' scripts/packages/foo/__load__.zeek >>out
+#
+# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff out
+
+@TEST-START-FILE improve-foo-tag-1.0.3
+set -e
+cd packages/foo
+echo 'test_command = exit 1' >> zkg.meta
+echo 'print "foo 1.0.3";' > __load__.zeek
+git add zkg.meta
+git commit -am 'add a test'
+git tag -a 1.0.3 -m 1.0.3
+@TEST-END-FILE