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

Proposed by Iain Lane
Status: Superseded
Proposed branch: lp:~laney/ubuntu-test-cases/touch-boottest-no-download-orig
Merge into: lp:ubuntu-test-cases/touch
Diff against target: 110 lines (+30/-17)
5 files modified
scripts/boottest.sh (+24/-8)
tests/getinstalledpkgs/debian/changelog (+1/-1)
tests/getinstalledpkgs/debian/control (+2/-2)
tests/getinstalledpkgs/debian/tests/control (+2/-2)
tests/getinstalledpkgs/debian/tests/getinstalledpkgs.template (+1/-4)
To merge this branch: bzr merge lp:~laney/ubuntu-test-cases/touch-boottest-no-download-orig
Reviewer Review Type Date Requested Status
Evan (community) Needs Information
Review via email: mp+260584@code.launchpad.net

This proposal has been superseded by a proposal from 2015-06-02.

Description of the change

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)

To post a comment you must log in.
396. By Iain Lane

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

Revision history for this message
Evan (ev) wrote :

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

review: Needs Information
Revision history for this message
Evan (ev) wrote :

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.

Revision history for this message
Iain Lane (laney) 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.

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

397. By Iain Lane

Make package extraction work with native packages and format 1.0 packages

Revision history for this message
Iain Lane (laney) wrote :

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

398. By Iain Lane

Exclude debian at any level

Revision history for this message
Francis Ginther (fginther) 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.

Revision history for this message
Iain Lane (laney) wrote :

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

Revision history for this message
Francis Ginther (fginther) wrote :

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'scripts/boottest.sh'
2--- scripts/boottest.sh 2015-03-16 16:20:32 +0000
3+++ scripts/boottest.sh 2015-06-01 11:23:15 +0000
4@@ -146,16 +146,16 @@
5 --- ${ADT_VIRT}"
6
7
8-# Inject the package name into getpkgsrc DEP8 test
9-FROM=${TESTS}/getpkgsrc/debian/tests/getpkgsrc.template
10-TARGET=${TESTS}/getpkgsrc/debian/tests/getpkgsrc
11+# Inject the package name into getinstalledpkgs DEP8 test
12+FROM=${TESTS}/getinstalledpkgs/debian/tests/getinstalledpkgs.template
13+TARGET=${TESTS}/getinstalledpkgs/debian/tests/getinstalledpkgs
14 sed -e "s/{{ source_package }}/${SRC_PKG_NAME}/" ${FROM} > ${TARGET}
15
16 # Execute a first test to get the package source tree from the testbed.
17 PKG_SRC_DIR=pkgsrc
18 rm -fr ${PKG_SRC_DIR} || true
19 set +e
20-${ADT_CMD} --unbuilt-tree ${TESTS}/getpkgsrc -o ${PKG_SRC_DIR} ${ADT_OPTS}
21+${ADT_CMD} --unbuilt-tree ${TESTS}/getinstalledpkgs -o ${PKG_SRC_DIR} ${ADT_OPTS}
22 RET=$?
23 set -e
24
25@@ -179,13 +179,29 @@
26 RET=$?
27 set -e
28 else
29- SOURCE_DIR=$(ls -d ${PKG_SRC_DIR}/artifacts/${SRC_PKG_NAME}-*/debian)
30- # Get the debian dir from the source package (involving the whole
31- # source tree can fail with 'No space left on device' on the phone).
32+ # Get the debian dir *only* from the archive. orig tarballs can be too
33+ # big for the target system.
34 TARGET_BASE=work
35 rm -fr ${TARGET_BASE}
36 mkdir -p ${TARGET_BASE}
37- cp -rd ${SOURCE_DIR} ${TARGET_BASE}
38+ apt-get source --diff-only ${SRC_PKG_NAME}
39+
40+ shopt -s nullglob
41+ allfiles=(*)
42+
43+ # If this is a 3.0 (native) package then that wouldn't have downloaded
44+ # anything, so try the whole thing.
45+ if [ ${#allfiles[@]} -eq 0 ]; then
46+ apt-get source --tar-only ${SRC_PKG_NAME}
47+ # Extract debian/ but not */debian/
48+ tar --strip-components=1 --no-anchored --exclude "*/**/debian" --extract --file * "debian"
49+ elif [ ! -z *.diff.gz ]; then # this is a source format 1.0 package
50+ zcat * | patch -p1
51+ else # 3.0 (quilt)
52+ tar xf *
53+ fi
54+ shopt -u nullglob
55+
56 # Inject the boot DEP8 test into the debian dir from the package
57 # source tree
58 FROM=${TESTS}/boottest/debian/tests
59
60=== renamed directory 'tests/getpkgsrc' => 'tests/getinstalledpkgs'
61=== modified file 'tests/getinstalledpkgs/debian/changelog'
62--- tests/getpkgsrc/debian/changelog 2015-01-24 21:31:54 +0000
63+++ tests/getinstalledpkgs/debian/changelog 2015-06-01 11:23:15 +0000
64@@ -1,4 +1,4 @@
65-getpkgsrc (0.1) vivid; urgency=medium
66+getinstalledpkgs (0.1) vivid; urgency=medium
67
68 * Initial release.
69
70
71=== modified file 'tests/getinstalledpkgs/debian/control'
72--- tests/getpkgsrc/debian/control 2015-01-24 21:31:54 +0000
73+++ tests/getinstalledpkgs/debian/control 2015-06-01 11:23:15 +0000
74@@ -1,9 +1,9 @@
75-Source: getpkgsrc
76+Source: getinstalledpkgs
77 Section: misc
78 Maintainer: Canonical CI Engineering <canonical-ci-engineering@lists.launchpad.net>
79 Build-Depends: debhelper (>= 9),
80 Standards-Version: 3.9.4
81
82-Package: getpkgsrc
83+Package: getinstalledpkgs
84 Architecture: all
85 Description: A dep8 test providing a package source from inside the testbed.
86
87=== modified file 'tests/getinstalledpkgs/debian/tests/control'
88--- tests/getpkgsrc/debian/tests/control 2015-01-24 21:31:54 +0000
89+++ tests/getinstalledpkgs/debian/tests/control 2015-06-01 11:23:15 +0000
90@@ -1,2 +1,2 @@
91-Tests: getpkgsrc
92-Depends: dpkg-dev
93+Tests: getinstalledpkgs
94+Depends: dpkg-dev, dctrl-tools
95
96=== renamed file 'tests/getpkgsrc/debian/tests/getpkgsrc.template' => 'tests/getinstalledpkgs/debian/tests/getinstalledpkgs.template'
97--- tests/getpkgsrc/debian/tests/getpkgsrc.template 2015-01-28 14:30:28 +0000
98+++ tests/getinstalledpkgs/debian/tests/getinstalledpkgs.template 2015-06-01 11:23:15 +0000
99@@ -4,10 +4,7 @@
100 cd ${ADT_ARTIFACTS}
101 # List installed packages, removing the ':{arch}' suffix
102 dpkg-query -f '${binary:Package}\n' -W | sed -e 's/:.*$//' > installed.packages
103-# adt-run complains and fails the test if sdterr is not empty
104-# redirect stderr to preserve the output for reference
105-apt-get source {{ source_package }} 2> apt-get-source.stderr
106 # Get the defined binary packages
107-grep ^Package: {{ source_package }}*/debian/control | sed -e 's/^Package: //' | sort > binary.packages
108+grep-aptavail -X -n -S -sPackage {{ source_package }} | sort | uniq > binary.packages
109 # Get the binary packages already installed
110 comm -1 -2 binary.packages installed.packages > needs_install.packages

Subscribers

People subscribed via source and target branches