629083 - Create new views in the Mozmill dashboard for the Endurance Tests

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Result Dashboard, defect)

Description

[Dave Hunt [:davehunt] [he/him] ⌚BST]

In order to review results from the Endurance Tests we need to create new views in the Mozmill dashboard to present the metrics gathered during each testrun.

Comment 1

[Dave Hunt [:davehunt] [he/him] ⌚BST]

I have forked the mozmill-dashboard project and created a branch at https://github.com/davehunt/mozmill-dashboard/tree/endurance. At this time it works with the format of the results produced by the patches in bug 629069 and bug 629077. I have also set up a couchone instance at http://mozmill.blargon7.com/#/endurance/reports

Comment 2

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: Add endurance reports to dashboard. v1

Initial version of endurance reports.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 518847 [details] [diff] [review]
Add endurance reports to dashboard. v1

> _attachments/js/highcharts.js                     |  157 ++++++++++++
> _attachments/js/themes/dark-blue.js               |  170 +++++++++++++
> _attachments/js/themes/dark-green.js              |  170 +++++++++++++
> _attachments/js/themes/gray.js                    |  164 +++++++++++++
> _attachments/js/themes/grid.js                    |   97 ++++++++

Are those themes for highcharts? If yes we should stick all files under a separate subfolder.

>+    <script language="javascript" type="text/javascript" src="js/highcharts.js"></script>

What are the performance costs in always including the charts? Not sure if we want to do that by default.

>+  Array.max = function(array) {
>+      return Math.max.apply(Math, array);
>+  };
>+
>+  Array.min = function(array) {
>+      return Math.min.apply(Math, array);
>+  };
>+
>+  Array.prototype.average = function() {
>+      var av = 0;
>+      var cnt = 0;
>+      var len = this.length;
>+      for (var i = 0; i < len; i++) {
>+          var e = +this[i];
>+          if(!e && this[i] !== 0 && this[i] !== '0') e--;
>+          if (this[i] == e) {av += e; cnt++;}
>+      }
>+      return av/cnt;
>+  }

Those definitions should be moved into a separate file, like what we did for the dates.

>             'mozmill-test' : 'firefox',
>             'mozmill-restart-test' : 'firefox',
>             'firefox-update' : 'softwareUpdate',
>-            'firefox-addons' : 'addons'
>+            'firefox-addons' : 'addons',
>+            'firefox-endurance' : 'endurance'
>           };
> 
>           var type = types[resp.report_type];
>@@ -759,7 +780,8 @@
>             'mozmill-test' : 'firefox',
>             'mozmill-restart-test' : 'firefox',
>             'firefox-update' : 'softwareUpdate',
>-            'firefox-addons' : 'addons'
>+            'firefox-addons' : 'addons',
>+            'firefox-endurance' : 'endurance'

Please check my updated code on bug 623744. You will have to incorporate it.

>+    var endurance_reports = function() {
>+      var branch = this.params.branch ? this.params.branch : 'All';
>+      var platform = this.params.platform ? this.params.platform : 'All';

Also add the handling of the dates.

>+          value.time = new Date(value.time).format("yyyy/mm/dd HH:MM:ss");

This augmented date method has been changed.

>+        for (var i=0; i < iterations[0].checkpoints.length; i++) {

Can we ensure to always have members in the iterations array?

>+            for (var ii=0; ii < iterationCheckpointCount; ii++) {

Use other variables like j, k, or l.

>+          var types = {
>+            'firefox-general' : 'firefox',
>+            'mozmill-test' : 'firefox',
>+            'mozmill-restart-test' : 'firefox',
>+            'firefox-update' : 'softwareUpdate',
>+            'firefox-addons' : 'addons',
>+            'firefox-endurance' : 'endurance'
>+          };

Remove everything except your testrun.

>+          var type = types[resp.report_type];
>+          var filename = result.filename.split(type)[1].replace(/\\/g, '/');

Add the try/catch clause.

>+++ b/_attachments/templates/endurance_reports.mustache
>+      <div id="os-selection">
>+        <span>OS: </span>
>+        <span>All</span>
>+        <span>Linux</span>
>+        <span>Mac</span>
>+        <span>Windows NT</span>
>+      </div>

Also add the date fields.

>+    <div id="pagination"><input class="pagination" type="text" value="20"></input><span class="pagination"> more</span></div>  

Then you can remove this line.

>From a36f5e8317d6f9aa094e6faac393eb0824f59c03 Mon Sep 17 00:00:00 2001
>From: Dave Hunt <[e-mail]>
>Date: Mon, 31 Jan 2011 13:09:42 +0000
>Subject: [PATCH 2/6] Updated to represent changes in report structure.
>
>---
> _attachments/js/dashboard.js                      |    7 ++++---
> _attachments/templates/endurance_reports.mustache |    4 ++++
> dashboard.js                                      |    4 +++-

Not sure what stuff like that is. Please submit a final patch against master. In general your code works so feedback+.

Comment 4

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: Add endurance reports to dashboard. v1.1

(In reply to comment #3)
> Comment on attachment 518847 [details] [diff] [review]
> Add endurance reports to dashboard. v1
> 
> > _attachments/js/highcharts.js                     |  157 ++++++++++++
> > _attachments/js/themes/dark-blue.js               |  170 +++++++++++++
> > _attachments/js/themes/dark-green.js              |  170 +++++++++++++
> > _attachments/js/themes/gray.js                    |  164 +++++++++++++
> > _attachments/js/themes/grid.js                    |   97 ++++++++
> 
> Are those themes for highcharts? If yes we should stick all files under a
> separate subfolder.

Done.

> >+    <script language="javascript" type="text/javascript" src="js/highcharts.js"></script>
> 
> What are the performance costs in always including the charts? Not sure if we
> want to do that by default.

Leaving here until the later refactor.

> >+  Array.max = function(array) {
> >+      return Math.max.apply(Math, array);
> >+  };
> >+
> >+  Array.min = function(array) {
> >+      return Math.min.apply(Math, array);
> >+  };
> >+
> >+  Array.prototype.average = function() {
> >+      var av = 0;
> >+      var cnt = 0;
> >+      var len = this.length;
> >+      for (var i = 0; i < len; i++) {
> >+          var e = +this[i];
> >+          if(!e && this[i] !== 0 && this[i] !== '0') e--;
> >+          if (this[i] == e) {av += e; cnt++;}
> >+      }
> >+      return av/cnt;
> >+  }
> 
> Those definitions should be moved into a separate file, like what we did for
> the dates.

Done.

> >             'mozmill-test' : 'firefox',
> >             'mozmill-restart-test' : 'firefox',
> >             'firefox-update' : 'softwareUpdate',
> >-            'firefox-addons' : 'addons'
> >+            'firefox-addons' : 'addons',
> >+            'firefox-endurance' : 'endurance'
> >           };
> > 
> >           var type = types[resp.report_type];
> >@@ -759,7 +780,8 @@
> >             'mozmill-test' : 'firefox',
> >             'mozmill-restart-test' : 'firefox',
> >             'firefox-update' : 'softwareUpdate',
> >-            'firefox-addons' : 'addons'
> >+            'firefox-addons' : 'addons',
> >+            'firefox-endurance' : 'endurance'
> 
> Please check my updated code on bug 623744. You will have to incorporate it.

Done.
 
> >+    var endurance_reports = function() {
> >+      var branch = this.params.branch ? this.params.branch : 'All';
> >+      var platform = this.params.platform ? this.params.platform : 'All';
> 
> Also add the handling of the dates.

Done.

> >+          value.time = new Date(value.time).format("yyyy/mm/dd HH:MM:ss");
> 
> This augmented date method has been changed.

Done.

> >+        for (var i=0; i < iterations[0].checkpoints.length; i++) {
> 
> Can we ensure to always have members in the iterations array?

There should always be at least one iteration.

> >+            for (var ii=0; ii < iterationCheckpointCount; ii++) {
> 
> Use other variables like j, k, or l.

Done.

> >+          var types = {
> >+            'firefox-general' : 'firefox',
> >+            'mozmill-test' : 'firefox',
> >+            'mozmill-restart-test' : 'firefox',
> >+            'firefox-update' : 'softwareUpdate',
> >+            'firefox-addons' : 'addons',
> >+            'firefox-endurance' : 'endurance'
> >+          };
> 
> Remove everything except your testrun.

Done.

> >+          var type = types[resp.report_type];
> >+          var filename = result.filename.split(type)[1].replace(/\\/g, '/');
> 
> Add the try/catch clause.

Done.

> >+++ b/_attachments/templates/endurance_reports.mustache
> >+      <div id="os-selection">
> >+        <span>OS: </span>
> >+        <span>All</span>
> >+        <span>Linux</span>
> >+        <span>Mac</span>
> >+        <span>Windows NT</span>
> >+      </div>
> 
> Also add the date fields.

Done.

> >+    <div id="pagination"><input class="pagination" type="text" value="20"></input><span class="pagination"> more</span></div>  
> 
> Then you can remove this line.

Done.

> >From a36f5e8317d6f9aa094e6faac393eb0824f59c03 Mon Sep 17 00:00:00 2001
> >From: Dave Hunt <[e-mail]>
> >Date: Mon, 31 Jan 2011 13:09:42 +0000
> >Subject: [PATCH 2/6] Updated to represent changes in report structure.
> >
> >---
> > _attachments/js/dashboard.js                      |    7 ++++---
> > _attachments/templates/endurance_reports.mustache |    4 ++++
> > dashboard.js                                      |    4 +++-
> 
> Not sure what stuff like that is. Please submit a final patch against master.
> In general your code works so feedback+.

Hopefully new patch is a bit cleaner. Thanks for the tip for creating it.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 519799 [details] [diff] [review]
Add endurance reports to dashboard. v1.1

>+    <script language="javascript" type="text/javascript" src="js/highcharts.js"></script>
>+++ b/_attachments/js/highcharts/highcharts.js

Looks like you forgot to update the index.html for the moved highcharts js file.

>+Array.max = function(array) {

Please define a name for all those math methods.

>+  return Math.max.apply(Math, array);

I would prefer .max.call here because you push a single argument and not a list to this method.

>+Array.prototype.average = function() {
>+  var av = 0;
>+  var cnt = 0;
>+  var len = this.length;
>+  for (var i = 0; i < len; i++) {
>+    var e = +this[i];

Not sure what this last line is doing. Can you please clarify?

>+    if(!e && this[i] !== 0 && this[i] !== '0') e--;
>+      if (this[i] == e) {av += e; cnt++;}

Please use brackets around comparisons and also the correct indentations. 

>+  return av/cnt;

This will raise an exception if the array is empty.

>+++ b/_attachments/templates/endurance_report.mustache
>@@ -0,0 +1,203 @@
>+    <style>
>+        .chart { margin-bottom: 20px; margin-right: 20px; width:450px; height:300px; }
>+    </style>

Move this to the general style sheet.

>+    <script>
>+        chart1 = new Highcharts.Chart({
>+            chart: {
>+                borderColor: '#AAAAAA',
>+                borderRadius: 10,
[..]

Can we move this into its own file and simply include? We will change the templating in the near future and you should be able to inject it into the head section then. In general it will remove the clutter from the template.

>+++ b/_attachments/templates/endurance_reports.mustache
>+    <table id="results">
>+      <thead>
>+        <tr>
>+          <th>Date</th>
>+          <th>Version</th>
>+          <th>Build</th>
>+          <th>Locale</th>
>+          <th>Platform</th>
>+          <th>Version</th>
>+          <th>CPU</th>
>+          <th>Delay</th>
>+          <th># Iterations</th>
>+          <th># Pass</th>
>+          <th># Skip</th>
>+          <th># Fail</th>

I still think that it would be very helpful to have the max and min value in this list. Pass, Skip, and Fail wouldn't be that important for this testrun. Changing that later would require to completely rebuild the views again, so I would kindly see it already with the initial checkin if possible. Does the result have a computed value for min/max already? I wonder if we even should move the computation to the python script so it can insert those values and we don't have to do it each time the report gets opened.

Comment 6

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: Add endurance reports to dashboard. v2

(In reply to comment #5)
> Comment on attachment 519799 [details] [diff] [review]
> Add endurance reports to dashboard. v1.1
> 
> >+    <script language="javascript" type="text/javascript" src="js/highcharts.js"></script>
> >+++ b/_attachments/js/highcharts/highcharts.js
> 
> Looks like you forgot to update the index.html for the moved highcharts js
> file.

Done.

> >+Array.max = function(array) {
> 
> Please define a name for all those math methods.

Done.

> >+  return Math.max.apply(Math, array);
> 
> I would prefer .max.call here because you push a single argument and not a list
> to this method.

As discussed on IRC, this needs to be apply as I'm passing an array, which becomes the parameters.

> >+Array.prototype.average = function() {
> >+  var av = 0;
> >+  var cnt = 0;
> >+  var len = this.length;
> >+  for (var i = 0; i < len; i++) {
> >+    var e = +this[i];
> 
> Not sure what this last line is doing. Can you please clarify?
> 
> >+    if(!e && this[i] !== 0 && this[i] !== '0') e--;
> >+      if (this[i] == e) {av += e; cnt++;}
> 
> Please use brackets around comparisons and also the correct indentations. 
> 
> >+  return av/cnt;
> This will raise an exception if the array is empty.

I've rewritten this function to be much clearer.

> >+++ b/_attachments/templates/endurance_report.mustache
> >@@ -0,0 +1,203 @@
> >+    <style>
> >+        .chart { margin-bottom: 20px; margin-right: 20px; width:450px; height:300px; }
> >+    </style>
> 
> Move this to the general style sheet.

Done.

> >+    <script>
> >+        chart1 = new Highcharts.Chart({
> >+            chart: {
> >+                borderColor: '#AAAAAA',
> >+                borderRadius: 10,
> [..]
> 
> Can we move this into its own file and simply include? We will change the
> templating in the near future and you should be able to inject it into the head
> section then. In general it will remove the clutter from the template.

Agreed that this would be a good move, but not sure how we'll do this. If you could make some suggestions that would be great.

> >+++ b/_attachments/templates/endurance_reports.mustache
> >+    <table id="results">
> >+      <thead>
> >+        <tr>
> >+          <th>Date</th>
> >+          <th>Version</th>
> >+          <th>Build</th>
> >+          <th>Locale</th>
> >+          <th>Platform</th>
> >+          <th>Version</th>
> >+          <th>CPU</th>
> >+          <th>Delay</th>
> >+          <th># Iterations</th>
> >+          <th># Pass</th>
> >+          <th># Skip</th>
> >+          <th># Fail</th>
> 
> I still think that it would be very helpful to have the max and min value in
> this list. Pass, Skip, and Fail wouldn't be that important for this testrun.
> Changing that later would require to completely rebuild the views again, so I
> would kindly see it already with the initial checkin if possible. Does the
> result have a computed value for min/max already? I wonder if we even should
> move the computation to the python script so it can insert those values and we
> don't have to do it each time the report gets opened.

Done. Allocated memory is displayed in the summary report. Also, the memory on all reports is now displayed in MB.

I also upgraded tablesorter to support sorting on columns with headers that span multiple columns.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Hurray for inter-diffs not working. :( So lets comment first on your reply.

(In reply to comment #6)
> > >+    <script>
> > >+        chart1 = new Highcharts.Chart({
> > >+            chart: {
> > >+                borderColor: '#AAAAAA',
> > >+                borderRadius: 10,
> > [..]
> > 
> > Can we move this into its own file and simply include? We will change the
> > templating in the near future and you should be able to inject it into the head
> > section then. In general it will remove the clutter from the template.
> 
> Agreed that this would be a good move, but not sure how we'll do this. If you
> could make some suggestions that would be great.

Lets do it later. Not a blocker to get this landed and used.

> > would kindly see it already with the initial checkin if possible. Does the
> > result have a computed value for min/max already? I wonder if we even should
> > move the computation to the python script so it can insert those values and we
> > don't have to do it each time the report gets opened.

Can you please give me an answer to that comment? It's kinda important if we could have those values already pre-computed.

More in the review comment.

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 521179 [details] [diff] [review]
Add endurance reports to dashboard. v2

This patch look wrong. Dave, please upload a version which is a diff between your branch's head and master. It looks like you attached the latest diff between head and the former commit.

Comment 9

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: Add endurance reports to dashboard. v2.1

(In reply to comment #7)
> Hurray for inter-diffs not working. :( So lets comment first on your reply.
> 
> (In reply to comment #6)
> > > would kindly see it already with the initial checkin if possible. Does the
> > > result have a computed value for min/max already? I wonder if we even should
> > > move the computation to the python script so it can insert those values and we
> > > don't have to do it each time the report gets opened.
> 
> Can you please give me an answer to that comment? It's kinda important if we
> could have those values already pre-computed.

I do like your suggestion, and will raise a bug and start to look into this soon.

(In reply to comment #8)
> Comment on attachment 521179 [details] [diff] [review]
> Add endurance reports to dashboard. v2
> 
> This patch look wrong. Dave, please upload a version which is a diff between
> your branch's head and master. It looks like you attached the latest diff
> between head and the former commit.

Sorry for wasting your time with the last patch. Hopefully this one is an improvement. I'm still learning git. :/

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 521361 [details] [diff] [review]
Add endurance reports to dashboard. v2.1

>+          value.min_allocated_memory = Math.round(min(value.allocated_memory)/1048576);
>+          value.max_allocated_memory = Math.round(max(value.allocated_memory)/1048576);

You are using these number very often across the different views. Just make it a global const, i.e. BYTE_TO_MEGABYTE

>--- a/_attachments/js/jquery/jquery.tablesorter.min.js
>+++ b/_attachments/js/jquery/jquery.tablesorter.min.js

What have you changed for tablesorter?

>+++ b/_attachments/js/math.js

>@@ -0,0 +1,14 @@
>+function max(array) {
>+  return Math.max.apply(Math, array);
>+};
>+
>+function min(array) {
>+  return Math.min.apply(Math, array);
>+};
>+
>+function average(array) {
>+  var length = array.length;
>+  for (var i = 0, sum = 0; i < length;
>+    sum+=Number(array[i++]));
>+  return sum/length;

So no more Function augmenting anymore? Instead we have global methods? I liked the previous way.

>+++ b/_attachments/templates/endurance_report.mustache
>+          <th>Allocated memory:</th>
>+          <td>Minimum: <strong>{{minAllocatedMemory}} MB</strong> / Maximum: <strong>{{maxAllocatedMemory}} MB</strong> / Average: <strong>{{averageAllocatedMemory}} MB</strong></td>
>+        </tr><tr>
>+          <th>Mapped memory:</th>
>+          <td>Minimum: <strong>{{minMappedMemory}} MB</strong> / Maximum: <strong>{{maxMappedMemory}} MB</strong> / Average: <strong>{{averageMappedMemory}} MB</strong></td>
[..]
>+          <td>{{minAllocatedMemory}} MB</td>
>+          <td>{{maxAllocatedMemory}} MB</td>
>+          <td>{{averageAllocatedMemory}} MB</td>
>+          <td>{{minMappedMemory}} MB</td>
>+          <td>{{maxMappedMemory}} MB</td>
>+          <td>{{averageMappedMemory}} MB</td>

Please add a <nobr> between the memory value and 'MB', so it doesn't break the unit into a new row.

>+++ b/_attachments/templates/endurance_reports.mustache
>+          <th rowspan="2">Delay</th>
>+          <th rowspan="2"># Iterations</th>
>+          <th colspan="2">Allocated Memory</th>

Shouldn't the colspan="2" heading be listed first? Not sure if this would fix the problem we currently have.

>+          <td>{{min_allocated_memory}} MB</td>
>+          <td>{{max_allocated_memory}} MB</td>

<nobr> here too.

r- because of the not clear tablesorter changes. If that's not necessary leave those out. Otherwise just fix the nits and we are good to go with the first round.

Comment 11

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to comment #10)
> Comment on attachment 521361 [details] [diff] [review]
> Add endurance reports to dashboard. v2.1
> >--- a/_attachments/js/jquery/jquery.tablesorter.min.js
> >+++ b/_attachments/js/jquery/jquery.tablesorter.min.js
> 
> What have you changed for tablesorter?

tablesorter was upgraded to support sorting in columns with headers that span multiple columns.

> >+++ b/_attachments/js/math.js
> 
> >@@ -0,0 +1,14 @@
> >+function max(array) {
> >+  return Math.max.apply(Math, array);
> >+};
> >+
> >+function min(array) {
> >+  return Math.min.apply(Math, array);
> >+};
> >+
> >+function average(array) {
> >+  var length = array.length;
> >+  for (var i = 0, sum = 0; i < length;
> >+    sum+=Number(array[i++]));
> >+  return sum/length;
> 
> So no more Function augmenting anymore? Instead we have global methods? I liked
> the previous way.

I'm not sure I understand. Could you give an example? From you previous comment I thought this is what you were suggesting.

> >+++ b/_attachments/templates/endurance_reports.mustache
> >+          <th rowspan="2">Delay</th>
> >+          <th rowspan="2"># Iterations</th>
> >+          <th colspan="2">Allocated Memory</th>
> 
> Shouldn't the colspan="2" heading be listed first? Not sure if this would fix
> the problem we currently have.

Which problem are you referring to? If we put this as the first heading then it will move it to the first column.

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to comment #11)
> > >+++ b/_attachments/js/math.js
> > 
> > >@@ -0,0 +1,14 @@
> > >+function max(array) {
> > >+  return Math.max.apply(Math, array);
> > >+};
> > >+
> > >+function min(array) {
> > >+  return Math.min.apply(Math, array);
> > >+};
> > >+
> > >+function average(array) {
> > >+  var length = array.length;
> > >+  for (var i = 0, sum = 0; i < length;
> > >+    sum+=Number(array[i++]));
> > >+  return sum/length;
> > 
> > So no more Function augmenting anymore? Instead we have global methods? I liked
> > the previous way.
> 
> I'm not sure I understand. Could you give an example? From you previous comment
> I thought this is what you were suggesting.

Not sure which comment you mean but I expected to see something like:

Array.average = function average(array)

> > >+++ b/_attachments/templates/endurance_reports.mustache
> > >+          <th rowspan="2">Delay</th>
> > >+          <th rowspan="2"># Iterations</th>
> > >+          <th colspan="2">Allocated Memory</th>
> > 
> > Shouldn't the colspan="2" heading be listed first? Not sure if this would fix
> > the problem we currently have.
> 
> Which problem are you referring to? If we put this as the first heading then it
> will move it to the first column.

The issue we talked about yesterday on IRC. The vertical line which is missing in the header after the allocated memory columns. But that's really minor.

Comment 13

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: Add endurance reports to dashboard. v2.2

(In reply to comment #10)
> Comment on attachment 521361 [details] [diff] [review]
> Add endurance reports to dashboard. v2.1
> 
> >+          value.min_allocated_memory = Math.round(min(value.allocated_memory)/1048576);
> >+          value.max_allocated_memory = Math.round(max(value.allocated_memory)/1048576);
> 
> You are using these number very often across the different views. Just make it
> a global const, i.e. BYTE_TO_MEGABYTE

Done.

> >+++ b/_attachments/templates/endurance_report.mustache
> >+          <th>Allocated memory:</th>
> >+          <td>Minimum: <strong>{{minAllocatedMemory}} MB</strong> / Maximum: <strong>{{maxAllocatedMemory}} MB</strong> / Average: <strong>{{averageAllocatedMemory}} MB</strong></td>
> >+        </tr><tr>
> >+          <th>Mapped memory:</th>
> >+          <td>Minimum: <strong>{{minMappedMemory}} MB</strong> / Maximum: <strong>{{maxMappedMemory}} MB</strong> / Average: <strong>{{averageMappedMemory}} MB</strong></td>
> [..]
> >+          <td>{{minAllocatedMemory}} MB</td>
> >+          <td>{{maxAllocatedMemory}} MB</td>
> >+          <td>{{averageAllocatedMemory}} MB</td>
> >+          <td>{{minMappedMemory}} MB</td>
> >+          <td>{{maxMappedMemory}} MB</td>
> >+          <td>{{averageMappedMemory}} MB</td>
> 
> Please add a <nobr> between the memory value and 'MB', so it doesn't break the
> unit into a new row.

Done. I used a non-breaking space instead.

> >+++ b/_attachments/templates/endurance_reports.mustache
> >+          <td>{{min_allocated_memory}} MB</td>
> >+          <td>{{max_allocated_memory}} MB</td>
> 
> <nobr> here too.

Done.

(In reply to comment #12)
> (In reply to comment #11)
> > > >+++ b/_attachments/js/math.js
> > > 
> > > >@@ -0,0 +1,14 @@
> > > >+function max(array) {
> > > >+  return Math.max.apply(Math, array);
> > > >+};
> > > >+
> > > >+function min(array) {
> > > >+  return Math.min.apply(Math, array);
> > > >+};
> > > >+
> > > >+function average(array) {
> > > >+  var length = array.length;
> > > >+  for (var i = 0, sum = 0; i < length;
> > > >+    sum+=Number(array[i++]));
> > > >+  return sum/length;
> > > 
> > > So no more Function augmenting anymore? Instead we have global methods? I liked
> > > the previous way.
> > 
> > I'm not sure I understand. Could you give an example? From you previous comment
> > I thought this is what you were suggesting.
> 
> Not sure which comment you mean but I expected to see something like:
> 
> Array.average = function average(array)

Done.

> > > >+++ b/_attachments/templates/endurance_reports.mustache
> > > >+          <th rowspan="2">Delay</th>
> > > >+          <th rowspan="2"># Iterations</th>
> > > >+          <th colspan="2">Allocated Memory</th>
> > > 
> > > Shouldn't the colspan="2" heading be listed first? Not sure if this would fix
> > > the problem we currently have.
> > 
> > Which problem are you referring to? If we put this as the first heading then it
> > will move it to the first column.
> 
> The issue we talked about yesterday on IRC. The vertical line which is missing
> in the header after the allocated memory columns. But that's really minor.

This problem was resolved by upgrading tablesorter and allowing the colspan cell to be sortable.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 521538 [details] [diff] [review]
Add endurance reports to dashboard. v2.2

>+var BYTE_TO_MEGABYTE = 1048576;

It should be a value <1, means 1/1048576. For all usages you will do a multiplication which is faster than a division.

>+Array.max = function max(array) {
>+  return Math.max.apply(Math, array);
>+};
>+
>+Array.min = function min(array) {
>+  return Math.min.apply(Math, array);
>+};

Array.prototype and no array param necessary. See the average method below.

Otherwise looks fine. r=me with those changes and please upload to your couch instance, so I can check.

Comment 15

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: Add endurance reports to dashboard. v2.3

(In reply to comment #14)
> Comment on attachment 521538 [details] [diff] [review]
> Add endurance reports to dashboard. v2.2
> 
> >+var BYTE_TO_MEGABYTE = 1048576;
> 
> It should be a value <1, means 1/1048576. For all usages you will do a
> multiplication which is faster than a division.

Done.

> >+Array.max = function max(array) {
> >+  return Math.max.apply(Math, array);
> >+};
> >+
> >+Array.min = function min(array) {
> >+  return Math.min.apply(Math, array);
> >+};
> 
> Array.prototype and no array param necessary. See the average method below.

Done.

> Otherwise looks fine. r=me with those changes and please upload to your couch
> instance, so I can check.

I also added conversion of delay form milliseconds to seconds for the reports.

Pushed to: http://mozmill.blargon7.com/#/endurance/reports

Comment 16

[u279076]

Just some superficial feedback:

Delay
 * make heading "Delay (s)"
 * make value simply numeric ("0.1" instead of "0.1 s")
 * right align

Iterations
 * right align

Allocated Memory
 * make heading "Allocated Memory (MB)" or just "Memory (MB)"
 * make value simply numeric ("34" instead of "34 MB")
 * right align

Similar tweaks can be applied to the report view.  These are just suggestions though; don't block check-in on these. Looks good, overall.

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Valid points. Thanks Anthony. Dave, please file a new bug and take care about it over there. I will land your code as is so we are ready for the testday tomorrow.

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Landed as:
https://github.com/whimboo/mozmill-dashboard/commit/be22e0c8bb22fa14d998530b0b3e5b3c4adee2b2 (tablesorter)
https://github.com/whimboo/mozmill-dashboard/commit/e9e5d5e19d29529fca18b667cfbd3eb07cc74b0a (views)