[dbscripts,1/1] test: db-update: @test "update same any package to same repository fails": change PKGEXT

Message ID 20180216040410.21444-2-lukeshu@lukeshu.com
State Superseded
Headers show
Series Update tests to check for glob regression | expand

Commit Message

Luke Shumaker Feb. 16, 2018, 4:04 a.m. UTC
From: Luke Shumaker <lukeshu@parabola.nu>

This has the test change PKGEXT the second time it tries to release the
package.  Currently, this causes the tests to fail.  That's a good thing;
it's checking for the regression where db-functions:check_pkgrepos isn't
treating PKGEXT as a glob.

Without this, that regression didn't cause test failure because the checks
right after it were tripping anyway.

https://lists.archlinux.org/pipermail/arch-projects/2018-February/004742.html
---
 test/cases/db-update.bats | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Comments

Emil Velikov via arch-projects Feb. 16, 2018, 4:21 a.m. UTC | #1
On 02/15/2018 11:04 PM, Luke Shumaker wrote:
> From: Luke Shumaker <lukeshu@parabola.nu>
> 
> This has the test change PKGEXT the second time it tries to release the
> package.  Currently, this causes the tests to fail.  That's a good thing;
> it's checking for the regression where db-functions:check_pkgrepos isn't
> treating PKGEXT as a glob.
> 
> Without this, that regression didn't cause test failure because the checks
> right after it were tripping anyway.
> 
> https://lists.archlinux.org/pipermail/arch-projects/2018-February/004742.html

This looks reasonable, thanks. BTW no need to send a cover letter for
one patch. :)

> ---
>  test/cases/db-update.bats | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/test/cases/db-update.bats b/test/cases/db-update.bats
> index 1da7eef..6604841 100644
> --- a/test/cases/db-update.bats
> +++ b/test/cases/db-update.bats
> @@ -92,7 +92,16 @@ load ../lib/common
>  	db-update
>  	checkPackage extra pkg-any-a
>  
> -	releasePackage extra pkg-any-a
> +	# don't let __buildPackage use the cached build; we want to
> +	# force a new build with a different PKGEXT.
> +	if [[ -n ${BUILDDIR} ]]; then
> +		mv -T "${BUILDDIR}/$(__getCheckSum "${TMP}/svn-packages-copy/pkg-any-a/trunk/PKGBUILD")"{,.bak}
> +	fi
> +	PKGEXT=.pkg.tar.gz releasePackage extra pkg-any-a
> +	if [[ -n ${BUILDDIR} ]]; then
> +		rm -rf "${BUILDDIR}/$(__getCheckSum "${TMP}/svn-packages-copy/pkg-any-a/trunk/PKGBUILD")"
> +		mv -T "${BUILDDIR}/$(__getCheckSum "${TMP}/svn-packages-copy/pkg-any-a/trunk/PKGBUILD")"{.bak,}
> +	fi
>  	run db-update
>  	[ "$status" -ne 0 ]
>  }

I'm guessing you restore the old version because the new version will
throw off other tests using the cached version with the wrong $PKGEXT?
It might make more sense to change __buildPackage() to use is_globfile
${cache}*${PKGEXT} rather than [[ -d ${cache} ]]

This should prevent needing to move anything, leading to cleaner code on
both sides.
Luke Shumaker Feb. 16, 2018, 9:43 p.m. UTC | #2
On Thu, 15 Feb 2018 23:21:42 -0500,
Eli Schwartz via arch-projects wrote:
> 
> This looks reasonable, thanks. BTW no need to send a cover letter for
> one patch. :)

I felt silly sending it, but I wanted to note that your fixes looked
good to me for this, but I didn't want to put that in the commit
message. :)

> > ---
> >  test/cases/db-update.bats | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/test/cases/db-update.bats b/test/cases/db-update.bats
> > index 1da7eef..6604841 100644
> > --- a/test/cases/db-update.bats
> > +++ b/test/cases/db-update.bats
> > @@ -92,7 +92,16 @@ load ../lib/common
> >  	db-update
> >  	checkPackage extra pkg-any-a
> >  
> > -	releasePackage extra pkg-any-a
> > +	# don't let __buildPackage use the cached build; we want to
> > +	# force a new build with a different PKGEXT.
> > +	if [[ -n ${BUILDDIR} ]]; then
> > +		mv -T "${BUILDDIR}/$(__getCheckSum "${TMP}/svn-packages-copy/pkg-any-a/trunk/PKGBUILD")"{,.bak}
> > +	fi
> > +	PKGEXT=.pkg.tar.gz releasePackage extra pkg-any-a
> > +	if [[ -n ${BUILDDIR} ]]; then
> > +		rm -rf "${BUILDDIR}/$(__getCheckSum "${TMP}/svn-packages-copy/pkg-any-a/trunk/PKGBUILD")"
> > +		mv -T "${BUILDDIR}/$(__getCheckSum "${TMP}/svn-packages-copy/pkg-any-a/trunk/PKGBUILD")"{.bak,}
> > +	fi
> >  	run db-update
> >  	[ "$status" -ne 0 ]
> >  }
> 
> I'm guessing you restore the old version because the new version will
> throw off other tests using the cached version with the wrong $PKGEXT?
> It might make more sense to change __buildPackage() to use is_globfile
> ${cache}*${PKGEXT} rather than [[ -d ${cache} ]]
> 
> This should prevent needing to move anything, leading to cleaner code on
> both sides.

That's a good idea.  I'll do that!
Emil Velikov via arch-projects Feb. 16, 2018, 9:57 p.m. UTC | #3
On 02/16/2018 04:43 PM, Luke Shumaker wrote:
> On Thu, 15 Feb 2018 23:21:42 -0500,
> Eli Schwartz via arch-projects wrote:
>>
>> This looks reasonable, thanks. BTW no need to send a cover letter for
>> one patch. :)
> 
> I felt silly sending it, but I wanted to note that your fixes looked
> good to me for this, but I didn't want to put that in the commit
> message. :)

git send-email --annotate can be used to insert additional messages that
are not part of the commit, after the

---

and before this:

 test/cases/db-update.bats | 11 ++++++++++-

Patch

diff --git a/test/cases/db-update.bats b/test/cases/db-update.bats
index 1da7eef..6604841 100644
--- a/test/cases/db-update.bats
+++ b/test/cases/db-update.bats
@@ -92,7 +92,16 @@  load ../lib/common
 	db-update
 	checkPackage extra pkg-any-a
 
-	releasePackage extra pkg-any-a
+	# don't let __buildPackage use the cached build; we want to
+	# force a new build with a different PKGEXT.
+	if [[ -n ${BUILDDIR} ]]; then
+		mv -T "${BUILDDIR}/$(__getCheckSum "${TMP}/svn-packages-copy/pkg-any-a/trunk/PKGBUILD")"{,.bak}
+	fi
+	PKGEXT=.pkg.tar.gz releasePackage extra pkg-any-a
+	if [[ -n ${BUILDDIR} ]]; then
+		rm -rf "${BUILDDIR}/$(__getCheckSum "${TMP}/svn-packages-copy/pkg-any-a/trunk/PKGBUILD")"
+		mv -T "${BUILDDIR}/$(__getCheckSum "${TMP}/svn-packages-copy/pkg-any-a/trunk/PKGBUILD")"{.bak,}
+	fi
 	run db-update
 	[ "$status" -ne 0 ]
 }