[dbscripts,v2,2/3] test: Fixup glob matching

Message ID 20180223021541.15416-3-lukeshu@lukeshu.com
State Accepted
Headers show
Series Misc touchup | expand

Commit Message

Luke Shumaker Feb. 23, 2018, 2:15 a.m. UTC
From: Luke Shumaker <lukeshu@parabola.nu>

 - ftpdir-cleanup.bats: Glob expansion does not occur in [[ -f ]] tests.
   The [[ ! -f .../${pkgname}-*${PKGEXT} ]] checks were checking that there
   were no files containing a literal '*' for that part of their name.
   Obviously, this isn't what was intended.

 - sourceballs.bats: [ -r ] checks explode if the glob returns >1 file.
   Avoid using them if the path being checked contains a glob.

 - common.bash: Globbing happens on the RHS of a [[ = ]] test.
   This means that we must quote variables on the RHS that are to be taken
   verbatim.  This is surprising, because we don't need to quote the LHS.
---
 test/cases/ftpdir-cleanup.bats |  4 ++--
 test/cases/sourceballs.bats    |  4 ++--
 test/lib/common.bash           | 12 +++++++++++-
 3 files changed, 15 insertions(+), 5 deletions(-)

Comments

Emil Velikov via arch-projects Feb. 26, 2018, 5:46 a.m. UTC | #1
On 02/22/2018 09:15 PM, Luke Shumaker wrote:
>  - common.bash: Globbing happens on the RHS of a [[ = ]] test.
>    This means that we must quote variables on the RHS that are to be taken
>    verbatim.  This is surprising, because we don't need to quote the LHS.

Unless we intend to do a general style cleanup, it is unnecessary to
"fix" cases where variables with known static content aren't quoted.

Patch

diff --git a/test/cases/ftpdir-cleanup.bats b/test/cases/ftpdir-cleanup.bats
index fd485f3..8c713c6 100644
--- a/test/cases/ftpdir-cleanup.bats
+++ b/test/cases/ftpdir-cleanup.bats
@@ -13,8 +13,8 @@  __checkRepoRemovedPackage() {
 	local pkgname
 
 	for pkgname in $(__getPackageNamesFromPackageBase ${pkgbase}); do
-		[[ ! -f ${FTP_BASE}/${PKGPOOL}/${pkgname}-*${PKGEXT} ]]
-		[[ ! -f ${FTP_BASE}/${repo}/os/${repoarch}/${pkgname}-*${PKGEXT} ]]
+		! __isGlobfile "${FTP_BASE}/${PKGPOOL}/${pkgname}"-*"${PKGEXT}"
+		! __isGlobfile "${FTP_BASE}/${repo}/os/${repoarch}/${pkgname}"-*"${PKGEXT}"
 	done
 }
 
diff --git a/test/cases/sourceballs.bats b/test/cases/sourceballs.bats
index a0a2999..df7ddd4 100644
--- a/test/cases/sourceballs.bats
+++ b/test/cases/sourceballs.bats
@@ -2,12 +2,12 @@  load ../lib/common
 
 __checkSourcePackage() {
 	local pkgbase=$1
-	[ -r ${FTP_BASE}/${SRCPOOL}/${pkgbase}-*${SRCEXT} ]
+	__isGlobfile "${FTP_BASE}/${SRCPOOL}/${pkgbase}"-*"${SRCEXT}"
 }
 
 __checkRemovedSourcePackage() {
 	local pkgbase=$1
-	[ ! -r ${FTP_BASE}/${SRCPOOL}/${pkgbase}-*${SRCEXT} ]
+	! __isGlobfile "${FTP_BASE}/${SRCPOOL}/${pkgbase}"-*"${SRCEXT}"
 }
 
 @test "create simple package sourceballs" {
diff --git a/test/lib/common.bash b/test/lib/common.bash
index 5411641..6e2b3df 100644
--- a/test/lib/common.bash
+++ b/test/lib/common.bash
@@ -14,6 +14,16 @@  __getCheckSum() {
 	echo "${result%% *}"
 }
 
+# Proxy function to check if a file exists. Using [[ -f ... ]] directly is not
+# always wanted because we might want to expand bash globs first. This way we
+# can pass unquoted globs to __isGlobfile() and have them expanded as function
+# arguments before being checked.
+#
+# This is a copy of db-functions is_globfile
+__isGlobfile() {
+	[[ -f $1 ]]
+}
+
 __buildPackage() {
 	local pkgdest=${1:-.}
 	local p
@@ -203,7 +213,7 @@  checkPackageDB() {
 			for repoarch in ${repoarches[@]}; do
 				# Only 'any' packages can be found in repos of both arches
 				if [[ $pkgarch != any ]]; then
-					if [[ $pkgarch != ${repoarch} ]]; then
+					if [[ $pkgarch != "$repoarch" ]]; then
 						continue
 					fi
 				fi