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

2018-02-22 Thread Eli Schwartz via arch-projects 
On 02/22/2018 03:43 PM, Luke Shumaker wrote:
> From: Luke Shumaker 
> 
>  - 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



signature.asc
Description: OpenPGP digital signature

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

2018-02-22 Thread Luke Shumaker 
From: Luke Shumaker 

 - 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(-)

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