[OSSTEST PATCH 0/8] improvements related to adhoc testing

[OSSTEST PATCH 0/8] improvements related to adhoc testing

[Ian Jackson]

While testing Wei Liu's XSM series, I found that the adhoc testing
arrangements less than ideal.

After these patches, I can create `daily-cron-email-liuw' containing

  To: ian.jackson@eu.citrix.com, Wei Liu <wei.liu2@citrix.com>

and (in my own account on the test controller) do this:

  OSSTEST_EMAIL_HEADER=daily-cron-email-liuw OSSTEST_USE_HEAD=y ./cr-daily-branch osstest

and (I think) get a test run which:

  - uses whatever osstest version I have checked out
  - emails Wei and me
  - generates emails that don't look like a proper production test
  - reports regressions etc. against real osstest baseline (...#incoming)

Ian.

[OSSTEST PATCH 1/8] emails: cr*: Defer setting of OSSTEST_EMAIL_HEADER

[Ian Jackson]

Defer computing this variable until it is going to be used.

The effect is that the fallback default value is assigned much later -
in start_email (cr-daily-branch) or send_bisection_email (cri-bisect).
Previously it was assigned as soon as cri-args-hostlists was read,
which is quite early.

In the next patch this will allow other hooks (eg the branch settings)
to adjust the default, rather than them having to override.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 cri-args-hostlists |    6 +++++-
 cri-bisect         |    2 ++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/cri-args-hostlists b/cri-args-hostlists
index 57958d3..3e5ef44 100644
--- a/cri-args-hostlists
+++ b/cri-args-hostlists
@@ -53,7 +53,9 @@ elif [ "x$1" = "x--like-real" ]; then
 fi	
 : ${OSSTEST_BLESSING:=play}
 
-: ${OSSTEST_EMAIL_HEADER:=daily-cron-email-$OSSTEST_BLESSING}
+prepare_for_email () {
+	: ${OSSTEST_EMAIL_HEADER:=daily-cron-email-$OSSTEST_BLESSING}
+}
 
 . ${OSSTEST_CRON_SETTINGS:-$dcs-$OSSTEST_BLESSING}
 
@@ -93,6 +95,8 @@ start_email () {
         local subject_prefix=$4
         # redirects stdout
 
+	prepare_for_email
+
 	exec >tmp/$flight.email
 	branchemailheader=$OSSTEST_EMAIL_HEADER--$branch
 	if test -e $branchemailheader; then
diff --git a/cri-bisect b/cri-bisect
index af3f23f..e7ed24b 100644
--- a/cri-bisect
+++ b/cri-bisect
@@ -84,6 +84,8 @@ perhaps_bisect_step_core () {
 send_bisection_email () {
         echo emailing "$2" "$1"
 
+	prepare_for_email
+
         exec 3>tmp/$flight.email
         cat >&3 $OSSTEST_EMAIL_HEADER$2
         cat >&3 <<END
-- 
1.7.10.4

Re: [OSSTEST PATCH 1/8] emails: cr*: Defer setting of OSSTEST_EMAIL_HEADER

[Ian Campbell]

On Wed, 2015-02-25 at 13:01 +0000, Ian Jackson wrote:
> Defer computing this variable until it is going to be used.
> 
> The effect is that the fallback default value is assigned much later -
> in start_email (cr-daily-branch) or send_bisection_email (cri-bisect).
> Previously it was assigned as soon as cri-args-hostlists was read,
> which is quite early.
> 
> In the next patch this will allow other hooks (eg the branch settings)
> to adjust the default, rather than them having to override.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

[OSSTEST PATCH 2/8] emails: branch-settings.osstest: Permit caller to specify OSSTEST_EMAIL_HEADER

[Ian Jackson]

Do not override this if the environment already has one.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 branch-settings.osstest |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/branch-settings.osstest b/branch-settings.osstest
index 060ff4b..fe9561f 100644
--- a/branch-settings.osstest
+++ b/branch-settings.osstest
@@ -1 +1 @@
-OSSTEST_EMAIL_HEADER=daily-cron-email-osstest
+: ${OSSTEST_EMAIL_HEADER:=daily-cron-email-osstest}
-- 
1.7.10.4

Re: [OSSTEST PATCH 2/8] emails: branch-settings.osstest: Permit caller to specify OSSTEST_EMAIL_HEADER

[Ian Campbell]

On Wed, 2015-02-25 at 13:01 +0000, Ian Jackson wrote:
> Do not override this if the environment already has one.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

[OSSTEST PATCH 3/8] emails: honour OSSTEST_EMAIL_SUBJECT_PREFIX

[Ian Jackson]

This is prefixed before the other computed prefixes.  It makes it
easier to distinguish an adhoc cr-daily-branch test runs for a real
branch.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 cri-args-hostlists |    2 +-
 cri-bisect         |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/cri-args-hostlists b/cri-args-hostlists
index 3e5ef44..ef4cebd 100644
--- a/cri-args-hostlists
+++ b/cri-args-hostlists
@@ -105,7 +105,7 @@ start_email () {
 		cat $OSSTEST_EMAIL_HEADER
 	fi
 	echo "Message-ID: <osstest-$flight-mainreport@xen.org>"
-	printf 'Subject: %s' "${subject_prefix:-[$branch test] }"
+	printf 'Subject: %s' "${OSSTEST_EMAIL_SUBJECT_PREFIX}${subject_prefix:-[$branch test] }"
 
 	local flight_html_dir=$OSSTEST_HTMLPUB_DIR/
 	local job_html_dir=$OSSTEST_HTML_DIR/
diff --git a/cri-bisect b/cri-bisect
index e7ed24b..2cfb49a 100644
--- a/cri-bisect
+++ b/cri-bisect
@@ -89,7 +89,7 @@ send_bisection_email () {
         exec 3>tmp/$flight.email
         cat >&3 $OSSTEST_EMAIL_HEADER$2
         cat >&3 <<END
-Subject: $subject_prefix $1 $job
+Subject: ${OSSTEST_EMAIL_SUBJECT_PREFIX}$subject_prefix $1 $job
 
 branch $xenbranch
 xen branch $xenbranch
-- 
1.7.10.4

Re: [OSSTEST PATCH 3/8] emails: honour OSSTEST_EMAIL_SUBJECT_PREFIX

[Ian Campbell]

On Wed, 2015-02-25 at 13:01 +0000, Ian Jackson wrote:
> This is prefixed before the other computed prefixes.  It makes it
> easier to distinguish an adhoc cr-daily-branch test runs for a real
> branch.

Do they not already get "adhoc" in the $subject? i.e. my commissioning
runs for the new arm create (following README.dev procedure) resulted in
mails with:

[adhoc test] 34418: trouble: blocked/broken/fail/pass

(IOW it seems $branch is replaced by adhoc somewhere along the say)

> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> ---
>  cri-args-hostlists |    2 +-
>  cri-bisect         |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/cri-args-hostlists b/cri-args-hostlists
> index 3e5ef44..ef4cebd 100644
> --- a/cri-args-hostlists
> +++ b/cri-args-hostlists
> @@ -105,7 +105,7 @@ start_email () {
>  		cat $OSSTEST_EMAIL_HEADER
>  	fi
>  	echo "Message-ID: <osstest-$flight-mainreport@xen.org>"
> -	printf 'Subject: %s' "${subject_prefix:-[$branch test] }"
> +	printf 'Subject: %s' "${OSSTEST_EMAIL_SUBJECT_PREFIX}${subject_prefix:-[$branch test] }"
>  
>  	local flight_html_dir=$OSSTEST_HTMLPUB_DIR/
>  	local job_html_dir=$OSSTEST_HTML_DIR/
> diff --git a/cri-bisect b/cri-bisect
> index e7ed24b..2cfb49a 100644
> --- a/cri-bisect
> +++ b/cri-bisect
> @@ -89,7 +89,7 @@ send_bisection_email () {
>          exec 3>tmp/$flight.email
>          cat >&3 $OSSTEST_EMAIL_HEADER$2
>          cat >&3 <<END
> -Subject: $subject_prefix $1 $job
> +Subject: ${OSSTEST_EMAIL_SUBJECT_PREFIX}$subject_prefix $1 $job
>  
>  branch $xenbranch
>  xen branch $xenbranch

Re: [OSSTEST PATCH 3/8] emails: honour OSSTEST_EMAIL_SUBJECT_PREFIX

[Ian Jackson]

Ian Campbell writes ("Re: [OSSTEST PATCH 3/8] emails: honour OSSTEST_EMAIL_SUBJECT_PREFIX"):
> On Wed, 2015-02-25 at 13:01 +0000, Ian Jackson wrote:
> > This is prefixed before the other computed prefixes.  It makes it
> > easier to distinguish an adhoc cr-daily-branch test runs for a real
> > branch.
> 
> Do they not already get "adhoc" in the $subject? i.e. my commissioning
> runs for the new arm create (following README.dev procedure) resulted in
> mails with:
> 
> [adhoc test] 34418: trouble: blocked/broken/fail/pass
> 
> (IOW it seems $branch is replaced by adhoc somewhere along the say)

That happens if you use mg-execute-flight.  If you let cr-daily-branch
run the flight for you, it uses the standard email stuff.

Ian.

Re: [OSSTEST PATCH 3/8] emails: honour OSSTEST_EMAIL_SUBJECT_PREFIX

[Ian Campbell]

On Thu, 2015-02-26 at 17:44 +0000, Ian Jackson wrote:
> Ian Campbell writes ("Re: [OSSTEST PATCH 3/8] emails: honour OSSTEST_EMAIL_SUBJECT_PREFIX"):
> > On Wed, 2015-02-25 at 13:01 +0000, Ian Jackson wrote:
> > > This is prefixed before the other computed prefixes.  It makes it
> > > easier to distinguish an adhoc cr-daily-branch test runs for a real
> > > branch.
> > 
> > Do they not already get "adhoc" in the $subject? i.e. my commissioning
> > runs for the new arm create (following README.dev procedure) resulted in
> > mails with:
> > 
> > [adhoc test] 34418: trouble: blocked/broken/fail/pass
> > 
> > (IOW it seems $branch is replaced by adhoc somewhere along the say)
> 
> That happens if you use mg-execute-flight.  If you let cr-daily-branch
> run the flight for you, it uses the standard email stuff.

Ah, OK, I didn't realise there was a difference.

So Ack to this and the next patch which I didn't ack for similar
reasons. (I think that makes the whole series acked, FWIW)

> 
> Ian.

[OSSTEST PATCH 4/8] emails: Set OSSTEST_EMAIL_SUBJECT_PREFIX if not --real

[Ian Jackson]

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 cri-args-hostlists |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/cri-args-hostlists b/cri-args-hostlists
index ef4cebd..1acecb6 100644
--- a/cri-args-hostlists
+++ b/cri-args-hostlists
@@ -50,7 +50,9 @@ elif [ "x$1" = "x--like-real" ]; then
 	: ${OSSTEST_CRON_SETTINGS:=$dcs-real}
 	: ${OSSTEST_HTML_SUFFIX:=-play}
 	shift
-fi	
+else
+	: ${OSSTEST_EMAIL_SUBJECT_PREFIX:='adhoc '}
+fi
 : ${OSSTEST_BLESSING:=play}
 
 prepare_for_email () {
-- 
1.7.10.4

[OSSTEST PATCH 5/8] standalone: Always set OSSTEST_NO_BASELINE

[Ian Jackson]

OSSTEST_NO_BASELINE disables the thing where cr-daily-branch decides
(based on information from sg-check-tested) that the baseline is
untested and therefore that it ought to do a baseline test instead of
testing the tip.

This whole feature is never correct in standalone mode, where there is
no useful test history database for sg-check-tested to work on.

In the case where the branch was osstest, not specifying --baseline
and therefore not having OSSTEST_NO_BASELINE didn't make all that much
difference because we also have OSSTEST_USE_HEAD=y and currently
ap-fetch-version-old (wrongfully) honours that, so the `baseline' test
would use the head anyway.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 standalone |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/standalone b/standalone
index caf3fd5..dd36467 100755
--- a/standalone
+++ b/standalone
@@ -72,7 +72,6 @@ flight="standalone"
 host=
 reuse=1 # Don't blow away machines by default
 lvextendmax=50 # Leave some LVM space free for running tests
-nobaseline=y
 
 while true ; do
     case "$1" in
@@ -82,7 +81,7 @@ while true ; do
 	-r|--reuse)  reuse=1;   shift 1;;
 	-R|--noreuse|--reinstall)reuse=0;shift 1;;
 	--lvextendmax)lvextendmax=$2; shift 2;;
-	--baseline)nobaseline=n; shift 1;;
+	--baseline) echo >&2 'warning: --baseline is obsolete'; shift 1;;
 	--help)      usage; exit 0;;
         --) shift ; break ;;
         *) echo "Internal error!" ; exit 1 ;;
@@ -183,7 +182,7 @@ case $op in
         BUILD_LVEXTEND_MAX="$lvextendmax" \
         OSSTEST_FLIGHT=$flight \
         OSSTEST_CONFIG=$config \
-        OSSTEST_NO_BASELINE=$nobaseline \
+        OSSTEST_NO_BASELINE=y \
             with_logging logs/$flight/make-flight.log ./cr-daily-branch $@ $branch
         ;;
 
-- 
1.7.10.4

Re: [OSSTEST PATCH 5/8] standalone: Always set OSSTEST_NO_BASELINE

[Ian Campbell]

On Wed, 2015-02-25 at 13:01 +0000, Ian Jackson wrote:
> OSSTEST_NO_BASELINE disables the thing where cr-daily-branch decides
> (based on information from sg-check-tested) that the baseline is
> untested and therefore that it ought to do a baseline test instead of
> testing the tip.
> 
> This whole feature is never correct in standalone mode, where there is
> no useful test history database for sg-check-tested to work on.
> 
> In the case where the branch was osstest, not specifying --baseline
> and therefore not having OSSTEST_NO_BASELINE didn't make all that much
> difference because we also have OSSTEST_USE_HEAD=y and currently
> ap-fetch-version-old (wrongfully) honours that, so the `baseline' test
> would use the head anyway.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Although:
> -	--baseline)nobaseline=n; shift 1;;
> +	--baseline) echo >&2 'warning: --baseline is obsolete'; shift 1;;

TBH I think you could just nuke it from a tool like this. I rather
suspect noone is using it... I can't even remember why I wanted it.

Re: [OSSTEST PATCH 5/8] standalone: Always set OSSTEST_NO_BASELINE

[Ian Jackson]

Ian Campbell writes ("Re: [OSSTEST PATCH 5/8] standalone: Always set OSSTEST_NO_BASELINE"):
> On Wed, 2015-02-25 at 13:01 +0000, Ian Jackson wrote:
> > OSSTEST_NO_BASELINE disables the thing where cr-daily-branch decides
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
> Although:
> > -	--baseline)nobaseline=n; shift 1;;
> > +	--baseline) echo >&2 'warning: --baseline is obsolete'; shift 1;;
> 
> TBH I think you could just nuke it from a tool like this. I rather
> suspect noone is using it... I can't even remember why I wanted it.

OK, I have done that and retained your ack.

Thanks,
Ian.

[OSSTEST PATCH 6/8] standalone: Always pass OLD_REVISION=NONE to cr-daily-branch

[Ian Jackson]

We don't want sg-report-flight digging through the `test history
database' (which doesn't really exist in standalone mode) looking for
a baseline.

Passing OLD_REVISION=NONE also avoids calling ./ap-fetch-version-old
for the current branch.  This is useful particularly when the current
branch is osstest, because in that case ap-fetch-version-old ought to
fail when not run on the production system.

NONE will use `badchar' entry in `case "$NEW_REVISION/$OLD_REVISION"'
so sg-report-flight will get no --this and --that arguments.  It also
avoids constructing a list of people who touched changes, etc.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 standalone |    1 +
 1 file changed, 1 insertion(+)

diff --git a/standalone b/standalone
index dd36467..d9121bf 100755
--- a/standalone
+++ b/standalone
@@ -183,6 +183,7 @@ case $op in
         OSSTEST_FLIGHT=$flight \
         OSSTEST_CONFIG=$config \
         OSSTEST_NO_BASELINE=y \
+        OLD_REVISION=NONE \
             with_logging logs/$flight/make-flight.log ./cr-daily-branch $@ $branch
         ;;
 
-- 
1.7.10.4

Re: [OSSTEST PATCH 6/8] standalone: Always pass OLD_REVISION=NONE to cr-daily-branch

[Ian Campbell]

On Wed, 2015-02-25 at 13:01 +0000, Ian Jackson wrote:
> We don't want sg-report-flight digging through the `test history
> database' (which doesn't really exist in standalone mode) looking for
> a baseline.
> 
> Passing OLD_REVISION=NONE also avoids calling ./ap-fetch-version-old
> for the current branch.  This is useful particularly when the current
> branch is osstest, because in that case ap-fetch-version-old ought to
> fail when not run on the production system.
> 
> NONE will use `badchar' entry in `case "$NEW_REVISION/$OLD_REVISION"'
> so sg-report-flight will get no --this and --that arguments.  It also
> avoids constructing a list of people who touched changes, etc.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

[OSSTEST PATCH 7/8] ap-fetch-version-old: Do not honour OSSTEST_USE_HEAD

[Ian Jackson]

When doing an OSSTEST_USE_HEAD test in infrastructure mode we would
like to use the real osstest mainline baseline as a reference for the
flight reports.  (Rather than simply using the very version under test
as the baseline.)

So revert cf805fa8 "Allow forcing the use of current osstest HEAD for
branch=osstest" as far as it touches ap-fetch-version-old.

In standalone mode: tests now set OLD_REVISION=NONE in their calls to
cr-daily-branch, so ap-fetch-version-old will not be run, and things
will continue to work as before.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 ap-fetch-version-old |    8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/ap-fetch-version-old b/ap-fetch-version-old
index 7b7f50e..8ca9468 100755
--- a/ap-fetch-version-old
+++ b/ap-fetch-version-old
@@ -98,12 +98,8 @@ ovmf)
 		$BASE_TREE_OVMF xen-tested-master $BASE_LOCALREV_OVMF
 	;;
 osstest)
-	if [ "x$OSSTEST_USE_HEAD" != "xy" ] ; then
-	    git fetch -f $HOME/testing.git incoming:ap-fetch
-            git rev-parse ap-fetch^0
-	else
-	    git rev-parse HEAD^0
-	fi
+	git fetch -f $HOME/testing.git incoming:ap-fetch
+        git rev-parse ap-fetch^0
         ;;
 *)
 	echo >&2 "branch $branch ?"
-- 
1.7.10.4

Re: [OSSTEST PATCH 7/8] ap-fetch-version-old: Do not honour OSSTEST_USE_HEAD

[Ian Campbell]

On Wed, 2015-02-25 at 13:01 +0000, Ian Jackson wrote:
> When doing an OSSTEST_USE_HEAD test in infrastructure mode we would
> like to use the real osstest mainline baseline as a reference for the
> flight reports.  (Rather than simply using the very version under test
> as the baseline.)
> 
> So revert cf805fa8 "Allow forcing the use of current osstest HEAD for
> branch=osstest" as far as it touches ap-fetch-version-old.
> 
> In standalone mode: tests now set OLD_REVISION=NONE in their calls to
> cr-daily-branch, so ap-fetch-version-old will not be run, and things
> will continue to work as before.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

[OSSTEST PATCH 8/8] ap-fetch-version: Use osstest's home to find master tree

[Ian Jackson]

When ap-fetch-version and ap-fetch-version-old are run on the osstest
controller but as a different user they should look in ~osstest, not
$HOME, for the master testing.git tree.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 ap-fetch-version     |    2 +-
 ap-fetch-version-old |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ap-fetch-version b/ap-fetch-version
index 33aaf00..8d72796 100755
--- a/ap-fetch-version
+++ b/ap-fetch-version
@@ -91,7 +91,7 @@ ovmf)
 	;;
 osstest)
 	if [ "x$OSSTEST_USE_HEAD" != "xy" ] ; then
-	    git fetch $HOME/testing.git pretest:ap-fetch >&2
+	    git fetch ~osstest/testing.git pretest:ap-fetch >&2
             git rev-parse ap-fetch^0
 	else
 	    git rev-parse HEAD^0
diff --git a/ap-fetch-version-old b/ap-fetch-version-old
index 8ca9468..8f1bc82 100755
--- a/ap-fetch-version-old
+++ b/ap-fetch-version-old
@@ -98,7 +98,7 @@ ovmf)
 		$BASE_TREE_OVMF xen-tested-master $BASE_LOCALREV_OVMF
 	;;
 osstest)
-	git fetch -f $HOME/testing.git incoming:ap-fetch
+	git fetch -f ~osstest/testing.git incoming:ap-fetch
         git rev-parse ap-fetch^0
         ;;
 *)
-- 
1.7.10.4

Re: [OSSTEST PATCH 8/8] ap-fetch-version: Use osstest's home to find master tree

[Ian Campbell]

On Wed, 2015-02-25 at 13:01 +0000, Ian Jackson wrote:
> When ap-fetch-version and ap-fetch-version-old are run on the osstest
> controller but as a different user they should look in ~osstest, not
> $HOME, for the master testing.git tree.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

But what if they are run not on the osstest controller where ~osstest
may not exist?

I think your previous changes have already arranged that standalone mode
won't get to either of these anyway, so, that being the case:

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Re: [OSSTEST PATCH 8/8] ap-fetch-version: Use osstest's home to find master tree

[Ian Jackson]

Ian Campbell writes ("Re: [OSSTEST PATCH 8/8] ap-fetch-version: Use osstest's home to find master tree"):
> On Wed, 2015-02-25 at 13:01 +0000, Ian Jackson wrote:
> > When ap-fetch-version and ap-fetch-version-old are run on the osstest
> > controller but as a different user they should look in ~osstest, not
> > $HOME, for the master testing.git tree.
...
> But what if they are run not on the osstest controller where ~osstest
> may not exist?

Then they ought not to look for the user's $HOME/testing.git, which is
unlikely to (a) exist or (b) be relevant if it does.  They ought to
fail.

> I think your previous changes have already arranged that standalone mode
> won't get to either of these anyway, so, that being the case:

Yes, that's the intent.

> Acked-by: Ian Campbell <ian.campbell@citrix.com>

I have added something about this to the commit message (and retained
your ack).

Thanks,
Ian.