Closed Bug 583227 Opened 14 years ago Closed 14 years ago

performance test suite reports build number as 0 when vm version check fails

Categories

(Tamarin Graveyard :: Tools, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: dschaffe, Assigned: dschaffe)

Details

Attachments

(1 file, 1 obsolete file)

in the performance database results are stored with the build number 0 when the version check is unable to detect a build number.

From checking the test harness code a 0 build number will be logged in the database in a few cases:
- if a ':' is not in the version string, we should just return the version string.  in some older milestone builds or builds from perforce branches we may not want to list a mercurial build number and keep a number without a ':'.  We should not return 0 since results in useless data in the database.
- if the version cannot be detected we should report error, return -1, and not put any data in the database
Attached patch patch (obsolete) — Splinter Review
should fix android and winmo to stop buildnumber being set to 0 if version check does not work in shell.
Attachment #462421 - Flags: review?(brbaker)
Comment on attachment 462421 [details] [diff] [review]
patch

Looks like the idea was to ensure that --vmversion was being passed to all performance/runtests.py calls but that is NOT the case with this patch (there are still plenty of calls that are not passing the switch, check all of the android-performance scripts and also all of the --memory calls)

Make sure to not only update the echo of the command but also what is actually being executed, this only update what is echoed and not the actual call

diff -r c4d4ff1c8698 build/buildbot/slaves/all/run-performance-release-arm.sh
--- a/build/buildbot/slaves/all/run-performance-release-arm.sh	Fri Jul 30 12:50:58 2010 -0400
+++ b/build/buildbot/slaves/all/run-performance-release-arm.sh	Tue Aug 03 11:39:02 2010 -0400
@@ -132,7 +132,7 @@
         vmargs=""
         iter=3
     fi
-    echo "python ./runtests.py -r $branch -k -f -i $iter --vmargs='$vmargs' $2"
+    echo "python ./runtests.py -r $branch -k -f -i $iter --vmversion=$change --vmargs='$vmargs' $2"
     python ./runtests.py -r $branch -k -f -i $iter --vmargs="$vmargs" $2
     test "$?" = "0" || { 
         result="1"; 


Should also stick to using $change instead of $1 for the version number:
-    python ./runtests.py -r $branch -k -f -i $iter --vmargs="$interp $vmargs" $2
+    python ./runtests.py -r $branch -k -f -i $iter --vmversion=$1 --vmargs="$interp $vmargs" $2
Attachment #462421 - Flags: review?(brbaker) → review-
the 0 in the winmo and android performance is caused by actual version not matching expected.  On android and milestone builds the patch below plus the --vmversion addition fixed the problem for release.  
diff -r 9cc3f30a7a5a test/util/runtestBase.py
--- a/test/util/runtestBase.py	Tue Aug 03 11:52:45 2010 -0400
+++ b/test/util/runtestBase.py	Wed Aug 04 10:01:13 2010 -0400
@@ -1329,7 +1328,7 @@
         try:
             return re.compile('(\d+):').search(avmVerStr).group(1)
         except:
-            return '0'
+            return avmVerStr
(In reply to comment #3)
> the 0 in the winmo and android performance is caused by actual version not
> matching expected.  On android and milestone builds the patch below plus the
> --vmversion addition fixed the problem for release.  
> diff -r 9cc3f30a7a5a test/util/runtestBase.py
> --- a/test/util/runtestBase.py    Tue Aug 03 11:52:45 2010 -0400
> +++ b/test/util/runtestBase.py    Wed Aug 04 10:01:13 2010 -0400
> @@ -1329,7 +1328,7 @@
>          try:
>              return re.compile('(\d+):').search(avmVerStr).group(1)
>          except:
> -            return '0'
> +            return avmVerStr

Android should now be returning proper version string information from the shell instead of always being 'cyclone'.
(In reply to comment #4)
> (In reply to comment #3)
> > the 0 in the winmo and android performance is caused by actual version not
> > matching expected.  On android and milestone builds the patch below plus the
> > --vmversion addition fixed the problem for release.  
> > diff -r 9cc3f30a7a5a test/util/runtestBase.py
> > --- a/test/util/runtestBase.py    Tue Aug 03 11:52:45 2010 -0400
> > +++ b/test/util/runtestBase.py    Wed Aug 04 10:01:13 2010 -0400
> > @@ -1329,7 +1328,7 @@
> >          try:
> >              return re.compile('(\d+):').search(avmVerStr).group(1)
> >          except:
> > -            return '0'
> > +            return avmVerStr
> 
> Android should now be returning proper version string information from the
> shell instead of always being 'cyclone'.

we should not assume builds to have ':'.  all of the milestone builds do not.  also having '0' as the buildnumber is difficult to debug rather than the version string.  I propose pushing just this 1 line change.
Attached patch patchSplinter Review
patches fixes android appearing as 0 in the perf db and also when -vmversion=<n> is specified without ':' full rev the build is entered in the perf db as 0.
Attachment #462421 - Attachment is obsolete: true
Attachment #463614 - Flags: review?(brbaker)
Attachment #463614 - Flags: review?(brbaker) → review+
pushed into tamarin-redux 4982:bede40c7ce45
Status: NEW → ASSIGNED
Assignee: nobody → dschaffe
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: