[arch-projects] [dbscripts] [PATCH 2/3] test: Fixup glob matching

[Eli Schwartz]

On 02/22/2018 03:43 PM, Luke Shumaker wrote:
> From: Luke Shumaker <lukeshu at 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.

Good catch, thanks!

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

Arguably I'd like to upgrade the whole testsuite to use [[ ... ]]

Thanks.

>  - 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.

No we don't, we only "need" to quote the ones that (can) contain globs.

In the general case, there is a school of thought that says you should
only quote what you explicitly need to quote. :p

Also not really surprising. This is a bash feature, you can do regex
comparisons too.

> ---
>  test/cases/ftpdir-cleanup.bats |  4 ++--
>  test/cases/sourceballs.bats    |  4 ++--
>  test/lib/common.bash           | 12 +++++++++++-
>  3 files changed, 15 insertions(+), 5 deletions(-)
> 
> 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
> 


-- 
Eli Schwartz
Bug Wrangler and Trusted User

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.archlinux.org/pipermail/arch-projects/attachments/20180222/bf84dcee/attachment.asc>