619506 - zeros should be displayed for a bug view

Status: RESOLVED FIXED

(Tree Management Graveyard :: OrangeFactor, defect, P1)

Description

[Jeff Hammel]

Currently, if a bug does not occur, the detail view for the bug does
show or act like there is a zero, but instead shows a line for when it
is non-zero.  Zeros should be displayed and math should be done with
zeros when the bug does not occur.

Comment 1

[Jeff Hammel]

preferably, don't make the zeros visible data points, but the graph should display the line as if it went through the data points.

Currently the zeros are used in calculations (e.g. 7day moving average).  They shouldn't be

Comment 2

[Mark Côté [:mcote]]

Attachment: Include zero points (no occurrences) in graphs and calculations in bug details view

This adds zero points to the graph in the bug-details view.  The zeros are used in the frequency & moving-average calculations.  They are not included in the table of occurrences.

The tooltips will be changed in bug 619823, so I've left them there for the zero points for the time being.

I'm thinking that zero points should be used in the OF calculations on the other graphs (OrangeFactor, TestRun, Simulator) too, even if it's incredibly unlikely that there will be no oranges at all in a current day.  What do you think?

Comment 3

[Mark Côté [:mcote]]

Attachment: Include zero points (no occurrences) in graphs and calculations in bug details view

Removing pointless whitespace change.

Comment 4

[Jeff Hammel]

> I'm thinking that zero points should be used in the OF calculations on the
> other graphs (OrangeFactor, TestRun, Simulator) too, even if it's incredibly
> unlikely that there will be no oranges at all in a current day.  What do you
> think?

Yes, sorry, I misspoke.  The zeros should be accounted for correctly in the calculations.

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 501850 [details] [diff] [review]
Include zero points (no occurrences) in graphs and calculations in bug details view

>diff --git a/_attachments/scripts/woo.dataparser.js b/_attachments/scripts/woo.dataparser.js
>     for (row in results) {
>       results[row] = results[row].sort(function(a, b) { return a["udate"] - b["udate"]});
>+      if (includeZeros) {
>+        var allDayResults = [];
>+        var oneDay = 24*60*60*1000;
>+        var lastDay = results[row][0].udate;
>+        allDayResults.push(results[row][0])
>+        for (var i = 1; i < results[row].length; i++) {
>+          while (results[row][i].udate - lastDay > oneDay) {
>+            lastDay += oneDay;
>+            allDayResults.push({date: getTboxDate(new Date(lastDay)), udate: lastDay, pushcount: 1, data: null});
>+          }
>+          allDayResults.push(results[row][i]);
>+          lastDay = results[row][i].udate;
>+        }
>+        while (enddate.getTime() > lastDay) {
>+          lastDay += oneDay;
>+          allDayResults.push({date: getTboxDate(new Date(lastDay)), udate: lastDay, pushcount: 1, data: null});
>+        }
>+        results[row] = allDayResults;
>+      }

I really don't understand what you are doing here.  Where is zero added to the results array?

Comment 6

[Mark Côté [:mcote]]

The for loop there adds entries for each day in which there are no results with data == null (and an arbitrary pushcount of 1, just so we don't divide by zero.  We would have to be careful here if we were calculating a complete average over time, since the pushcounts would be incorrectly increased, but we aren't doing that here, so the pushcount doesn't matter).

Later, we initialize datecount to 0 for every day but only increment it if data != null:

      for (item in results[row]) {
        var it = results[row][item]
        date = it["udate"];
        if (datecount[date] == null) {
          datecount[date] = 0;
          pushcount[date] = it["pushcount"];
        }
        if (it["data"] != null) {
          datecount[date] += 1;
        }
      }

And then a little bit later, we put the data into the three arrays (dateresults, pushresults, and movingresults), and since datecount for the null dates is 0, the entries for those dates in the three arrays are also zero:

        dateArray.push([d, datecount[d]]);
        pushArray.push([d, datecount[d]/pushcount[d]]);
        movingSum.push(datecount[d]/pushcount[d]);

And those are the three arrays that are plotted in displayBug():

                  showLineGraphMultiple([results[1][bugid], results[2][bugid], results[3][bugid]],

The results array (the first array returned by parseBugs()) is only used by displayBug() when populating the occurrence-details table, where it skips results with data == null:

                    for (item in b) {
                      if (b[item].data != null) {
                        var dat = b[item].data;
                        occurrences.push([b[item].date, dat.platform, dat.buildtype]);
                      }
                    }

Makes sense?

Comment 7

[Mark Côté [:mcote]]

Looking back over this, I might be able to skip a step in there, but I think it would require rewriting a lot of parseBugs().  I tried to fix this bug without changing too much of the general flow of this function.  But if you'd like me to rework the whole thing, I can try that, though that might be best left until your changes are in (if they affect this function much).

Comment 8

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 501850 [details] [diff] [review]
Include zero points (no occurrences) in graphs and calculations in bug details view

thanks for the explanation.

Comment 9

[Mark Côté [:mcote]]

Pushed as http://hg.mozilla.org/automation/orangefactor/rev/98f0ab138f06