Merge lp:~laney/ubuntu-test-cases/touch-boottest-no-download-orig into lp:ubuntu-test-cases/touch

[Iain Lane]

oxide-qt's boottest fails. psivaa says this is because getpkgsrc can't download the source package due to the device not having enough space.

Looking at the test script it seems as if the whole orig isn't needed - you only want the debian/ part to run the tests, and in "getpkgsrc" itself (outputs the binary packages from a source that are already installed) you can get the necessary information from apt's database directly without needing any part of the source.

We can refactor both parts

  - Make getpkgsrc use grep-aptavail from dctrl-tools to do the source to binary mapping. Then it isn't getting the source any more, so I rename it to getinstalledpkgs.
  - Use apt-get source --diff-only in the test runner to download the debian directory.

I can't test this, but if someone can and it works then it should be good to land in this state as a drop in replacement.

(I previously said something about getpkgsrc being unnecessary but now I see that it is used later on, so disregard that)

[lp:~laney/ubuntu-test-cases/touch-boottest-no-download-orig]

396. By Iain Lane on 2015-05-29

Rename in tests/getinstalledpkgs/debian/tests/control too

[Evan (ev)]

I'm not convinced --diff-only will work here.

review: Needs Information

[Evan (ev)]

Having getpkgsrc use grep-aptavail works. This block is here to calculate the binary packages generated by the source package under test so that they can be fed in as test dependencies of the DEP8 boot test.

[Iain Lane (laney)]

On Fri, May 29, 2015 at 05:42:37PM -0000, Evan Dandrea wrote:
> Isn't this going to fail for native packages? Also, ${SRC_PKG_NAME} :)

Yes. Good catch (on both). Can you think of any other case? Otherwise,
we could try --diff-only and if this doesn't download any files then do
it again without.

--
Iain Lane [ <email address hidden> ]
Debian Developer [ <email address hidden> ]
Ubuntu Developer [ <email address hidden> ]

[lp:~laney/ubuntu-test-cases/touch-boottest-no-download-orig]

397. By Iain Lane on 2015-06-01

Make package extraction work with native packages and format 1.0 packages

[Iain Lane (laney)]

On Sat, May 30, 2015 at 05:51:21PM -0000, Iain Lane wrote:
> On Fri, May 29, 2015 at 05:42:37PM -0000, Evan Dandrea wrote:
> > Isn't this going to fail for native packages? Also, ${SRC_PKG_NAME} :)
>
> Yes. Good catch (on both). Can you think of any other case? Otherwise,
> we could try --diff-only and if this doesn't download any files then do
> it again without.

Pushed something like that, with fixes for other source formats too.
Could you re-review?

--
Iain Lane [ <email address hidden> ]
Debian Developer [ <email address hidden> ]
Ubuntu Developer [ <email address hidden> ]

[lp:~laney/ubuntu-test-cases/touch-boottest-no-download-orig]

398. By Iain Lane on 2015-06-01

Exclude debian at any level

[Francis Ginther (fginther)]

I'm currently testing this and will have some more input later. Right now, I don't see any reason why the code that runs on the host system needs to change. The fix to stop downloading the entire source package in getinstalledpkgs should be all that is needed.

[Iain Lane (laney)]

On Mon, Jun 01, 2015 at 03:23:35PM -0000, Francis Ginther wrote:
> I'm currently testing this and will have some more input later. Right now, I don't see any reason why the code that runs on the host system needs to change. The fix to stop downloading the entire source package in getinstalledpkgs should be all that is needed.

I thought that adt copies the source tree over to the target system.

If it doesn't, then that is right and this becomes just a space
optimisation so we can drop it if you want.

(Actually, I don't see why the package under test's debian/ directory is
needed at all at this step - it seems to me like it could just be a
skeleton whose debian/tests/control we generate from
needs_install.packages. But I didn't know for sure so went for this
route.

Actually², I don't get the reason why we generate this list at all.
Surely the boottest infrastructure has the list of packages it cares
about because it needs to know which packages to trigger a boottest for
in the first place and this script could consult a mapping into that
list directly.

Those are asides that don't need to block this review.)

p.s. still couldn't test it so it might break in some boneheaded way;
feel free to fix up directly and then merge yourself if it's easier once
you see what's wrong

--
Iain Lane [ <email address hidden> ]
Debian Developer [ <email address hidden> ]
Ubuntu Developer [ <email address hidden> ]

[Francis Ginther (fginther)]

Thanks for the response. Looking at this deeper, I have to agree with you. This section of code appears to be doing more then it needs to. The boottest is setup using the dep8 format, but it is completely independent of any dep8 test that the source package under test may have defined, but this separation appears to be confused.

From my testing so far, the changes to tests/getinstalledpkgs/* are doing the right thing. If This is the case for a few more test cases. I'll pull this set of changes as you suggest and at least fix the issue caused by large source packages.

Unmerged revisions

=== modified file 'scripts/boottest.sh'
--- scripts/boottest.sh	2015-03-16 16:20:32 +0000
+++ scripts/boottest.sh	2015-06-01 11:23:15 +0000
@@ -146,16 +146,16 @@
     --- ${ADT_VIRT}"
 
 
-# Inject the package name into getpkgsrc DEP8 test
-FROM=${TESTS}/getpkgsrc/debian/tests/getpkgsrc.template
-TARGET=${TESTS}/getpkgsrc/debian/tests/getpkgsrc
+# Inject the package name into getinstalledpkgs DEP8 test
+FROM=${TESTS}/getinstalledpkgs/debian/tests/getinstalledpkgs.template
+TARGET=${TESTS}/getinstalledpkgs/debian/tests/getinstalledpkgs
 sed -e "s/{{ source_package }}/${SRC_PKG_NAME}/" ${FROM} > ${TARGET}
 
 # Execute a first test to get the package source tree from the testbed.
 PKG_SRC_DIR=pkgsrc
 rm -fr ${PKG_SRC_DIR} || true
 set +e
-${ADT_CMD} --unbuilt-tree ${TESTS}/getpkgsrc -o ${PKG_SRC_DIR} ${ADT_OPTS}
+${ADT_CMD} --unbuilt-tree ${TESTS}/getinstalledpkgs -o ${PKG_SRC_DIR} ${ADT_OPTS}
 RET=$?
 set -e
 
@@ -179,13 +179,29 @@
 	RET=$?
 	set -e
 else
-	SOURCE_DIR=$(ls -d ${PKG_SRC_DIR}/artifacts/${SRC_PKG_NAME}-*/debian)
-        # Get the debian dir from the source package (involving the whole
-        # source tree can fail with 'No space left on device' on the phone).
+        # Get the debian dir *only* from the archive. orig tarballs can be too
+        # big for the target system.
         TARGET_BASE=work
         rm -fr ${TARGET_BASE}
         mkdir -p ${TARGET_BASE}
-        cp -rd ${SOURCE_DIR} ${TARGET_BASE}
+        apt-get source --diff-only ${SRC_PKG_NAME}
+
+        shopt -s nullglob
+        allfiles=(*)
+
+        # If this is a 3.0 (native) package then that wouldn't have downloaded
+        # anything, so try the whole thing.
+        if [ ${#allfiles[@]} -eq 0 ]; then
+            apt-get source --tar-only ${SRC_PKG_NAME}
+            # Extract debian/ but not */debian/
+            tar --strip-components=1 --no-anchored --exclude "*/**/debian" --extract --file * "debian"
+        elif [ ! -z *.diff.gz ]; then # this is a source format 1.0 package
+            zcat * | patch -p1
+        else # 3.0 (quilt)
+            tar xf *
+        fi
+        shopt -u nullglob
+
 	# Inject the boot DEP8 test into the debian dir from the package
 	# source tree
 	FROM=${TESTS}/boottest/debian/tests

=== renamed directory 'tests/getpkgsrc' => 'tests/getinstalledpkgs'
=== modified file 'tests/getinstalledpkgs/debian/changelog'
--- tests/getpkgsrc/debian/changelog	2015-01-24 21:31:54 +0000
+++ tests/getinstalledpkgs/debian/changelog	2015-06-01 11:23:15 +0000
@@ -1,4 +1,4 @@
-getpkgsrc (0.1) vivid; urgency=medium
+getinstalledpkgs (0.1) vivid; urgency=medium
 
   * Initial release.
 

=== modified file 'tests/getinstalledpkgs/debian/control'
--- tests/getpkgsrc/debian/control	2015-01-24 21:31:54 +0000
+++ tests/getinstalledpkgs/debian/control	2015-06-01 11:23:15 +0000
@@ -1,9 +1,9 @@
-Source: getpkgsrc
+Source: getinstalledpkgs
 Section: misc
 Maintainer: Canonical CI Engineering <[e-mail]>
 Build-Depends: debhelper (>= 9),
 Standards-Version: 3.9.4
 
-Package: getpkgsrc
+Package: getinstalledpkgs
 Architecture: all
 Description: A dep8 test providing a package source from inside the testbed.

=== modified file 'tests/getinstalledpkgs/debian/tests/control'
--- tests/getpkgsrc/debian/tests/control	2015-01-24 21:31:54 +0000
+++ tests/getinstalledpkgs/debian/tests/control	2015-06-01 11:23:15 +0000
@@ -1,2 +1,2 @@
-Tests: getpkgsrc
-Depends: dpkg-dev
+Tests: getinstalledpkgs
+Depends: dpkg-dev, dctrl-tools

=== renamed file 'tests/getpkgsrc/debian/tests/getpkgsrc.template' => 'tests/getinstalledpkgs/debian/tests/getinstalledpkgs.template'
--- tests/getpkgsrc/debian/tests/getpkgsrc.template	2015-01-28 14:30:28 +0000
+++ tests/getinstalledpkgs/debian/tests/getinstalledpkgs.template	2015-06-01 11:23:15 +0000
@@ -4,10 +4,7 @@
 cd ${ADT_ARTIFACTS}
 # List installed packages, removing the ':{arch}' suffix
 dpkg-query -f '${binary:Package}\n' -W | sed -e 's/:.*$//' > installed.packages
-# adt-run complains and fails the test if sdterr is not empty
-# redirect stderr to preserve the output for reference
-apt-get source {{ source_package }} 2> apt-get-source.stderr
 # Get the defined binary packages
-grep ^Package: {{ source_package }}*/debian/control | sed -e 's/^Package: //' | sort > binary.packages
+grep-aptavail -X -n -S -sPackage {{ source_package }} | sort | uniq > binary.packages
 # Get the binary packages already installed
 comm  -1 -2 binary.packages installed.packages > needs_install.packages