661372 - Add OrangeFactor API to return the orange history of a given test failure

Status: RESOLVED FIXED

(Tree Management Graveyard :: OrangeFactor, defect)

Description

[Jonathan Griffin (:jgriffin)]

sdwilsh would like an API that uses the OrangeFactor database to return the orange history of a given failure

Comment 1

[Jonathan Griffin (:jgriffin)]

Shawn, I've made an API (not deployed yet), that takes a list of test names/errors and returns a breakdown of their occurrences in the OF database, and the associated bugs.

For example, you might pass this data to the API (two failures from a TBPL test run today):

[
  ["test_timeupdate_small_files.html", "Test timed out."],
  ["test_bug493251.html", "Wrong number events (16) - got 0, expected 1"]
]

The return would be:

{
  "test_bug493251.html": {
    "Wrong number events (16) - got 0, expected 1": {
      "mozilla-central": 23,
      "bugs": {
        "493251": 1,
        "565245": 9
      },
      "all_trees": 32,
      "tracemonkey": 2,
      "cedar": 6,
      "mozilla-beta": 1
    }
  },
  "test_timeupdate_small_files.html": {
    "Test timed out.": {
      "jaegermonkey": 2,
      "mozilla-central": 31,
      "bugs": {
        "620721": 10
      },
      "all_trees": 45,
      "mozilla-aurora": 3,
      "cedar": 6,
      "mozilla-beta": 2,
      "tracemonkey": 1
    }
  }
}

Does seem useful for your purposes?  Would you like to tweak the input/output?

Comment 2

[Shawn Wilsher :sdwilsh]

(In reply to comment #1)
> Does seem useful for your purposes?  Would you like to tweak the input/output?
I think that would help.  It does mean I'll need to parse lines like "TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser-window/browser_bug625465.js | Test timed out" into what you want.

I also know the tree, and I'm trying to decide if I should be able to pass that in and have you filter it or not.

Comment 3

[Jonathan Griffin (:jgriffin)]

Attachment: woo_server.py patch v0.1

First version of patch to serve this API.  The output is now slightly different:

{
  "test_bug493251.html": {
    "Wrong number events (16) - got 0, expected 1": {
      "count": {
        "places": 1,
        "mozilla-central": 31,
        "tracemonkey": 3,
        "cedar": 8,
        "mozilla-beta": 1,
        "total": 44
      },
      "bugs": {
        "493251": 1,
        "658802": 1,
        "655648": 2,
        "655630": 2,
        "565245": 41
      }
    }
  },
  "test_timeupdate_small_files.html": {
    "Test timed out.": {
      "count": {
        "mozilla-central": 39,
        "jaegermonkey": 2,
        "tracemonkey": 1,
        "mozilla-aurora": 4,
        "cedar": 10,
        "mozilla-beta": 6,
        "total": 62
      },
      "bugs": {
        "620721": 58
      }
    }
  }
}

You can pass a ?tree= parameter in the url, which will restrict it to the given tree, otherwise all trees are included.

Comment 4

[Jonathan Griffin (:jgriffin)]

To test this API, you can do something like this:

curl -XGET 'http://127.0.0.1:8080/api/orangequery/?tree=mozilla-central' -d '
> [
>   ["test_timeupdate_small_files.html", "Test timed out."],
>   ["test_bug493251.html", "Wrong number events (16) - got 0, expected 1"]
> ]
> '

Comment 5

[Jonathan Griffin (:jgriffin)]

Attachment: woo_server.py patch v0.2

I've integrated the backend work for bug 646749 into this same API, since they use similar queries.  I can split into separate queries later, if it turns out there is a need.

With this update, the returned data contains test failure correlations, which can allow a caller to determine which of the set of test failures usually occurs together.

Sample input:

curl -XGET 'http://127.0.0.1:8080/api/orangequery/?tree=mozilla-central' -d '
[
  ["test_timeupdate_small_files.html", "Test timed out."],
  ["test_bug493251.html", "Wrong number events (16) - got 0, expected 1"],
  ["test_bug493251.html", "Wrong number events (17) - got 0, expected 1"]
]
'

Sample output:

{
  "test_bug493251.html": {
    "Wrong number events (17) - got 0, expected 1": {
      "count": {
        "total": 31,
        "mozilla-central": 31
      },
      "correlations": {
        "test_bug493251.html": {
          "Wrong number events (16) - got 0, expected 1": 31
        },
        "test_timeupdate_small_files.html": {
          "Test timed out.": 0
        }
      },
      "bugs": {
        "493251": 1,
        "655630": 2,
        "565245": 30
      }
    },
    "Wrong number events (16) - got 0, expected 1": {
      "count": {
        "total": 31,
        "mozilla-central": 31
      },
      "correlations": {
        "test_bug493251.html": {
          "Wrong number events (17) - got 0, expected 1": 31
        },
        "test_timeupdate_small_files.html": {
          "Test timed out.": 0
        }
      },
      "bugs": {
        "493251": 1,
        "655630": 2,
        "565245": 30
      }
    }
  },
  "test_timeupdate_small_files.html": {
    "Test timed out.": {
      "count": {
        "total": 39,
        "mozilla-central": 39
      },
      "correlations": {
        "test_bug493251.html": {
          "Wrong number events (17) - got 0, expected 1": 0,
          "Wrong number events (16) - got 0, expected 1": 0
        }
      },
      "bugs": {
        "620721": 38
      }
    }
  }
}

Comment 6

[Mark Côté [:mcote]]

Comment on attachment 537912 [details] [diff] [review]
woo_server.py patch v0.2

Looks good, just a few things:

>+    '/orangequery/?', 'OrangeQuery',
> )

Heh "orangequery" is kind of funny and vague...maybe "testerrors" or something like that? Although I don't care that much.

> 
> class JsonHandler:
> 
>     def GET(self):
>-        results = json.dumps(self._GET(urlparse.parse_qs(web.ctx.query[1:], True)))
>+        try:
>+          body = json.loads(web.data())
>+        except:
>+          body = None
>+        results = json.dumps(self._GET(urlparse.parse_qs(web.ctx.query[1:], True), body))

Hm bare except clauses aren't great because they can mask real problems. Is it really needed here?

>+            # generate some generic structure in our response object
>+            if not test in response:
>+                response[test] = {}
>+            if not error in response[test]:
>+                response[test][error] = {}
>+                response[test][error]['bugs'] = {}
>+                response[test][error]['count'] = {}
>+                response[test][error]['correlations'] = {}
>+                response[test][error]['count']['total'] = 0

Might as well initialize that all at once, e.g.

response[test][error] = {
    'bugs': {},
    'count': { 'total': 0 },
    'correlations': {}
}

Also, not sure if "total" should be in the "count" object.  Seems a bit funny to me because it has a special meaning--it includes all the other numbers. So code iterating over those values would have to handle the special case. And it's easy to tally them up if needed, so I would drop it unless there's something I'm missing.

Wondering if maybe we can have a separate query that will return bug correlations by bug ID, maybe by checking failure messages where that bug occurs and then using that to look for correlations.  Does that make sense?  If so, I would do that as a separate patch so we can deploy this for sdwilsh.

Comment 7

[Jonathan Griffin (:jgriffin)]

Pushed as http://hg.mozilla.org/automation/orangefactor/rev/46b6977e9c46.  I addressed all your comments.  For the 'total' issue, I think some callers may find the total more useful than the individual tree counts, so I didn't want to remove it completely, thus forcing them to iterate over all the individual counts just to get a sum.  Instead, I changed the output a little, e.g., 

  "test_timeupdate_small_files.html": {
    "Test timed out.": {
      "counts": {
        "total": 62,
        "trees": {
          "mozilla-central": 39,
          "jaegermonkey": 2,
          "mozilla-aurora": 4,
          "cedar": 10,
          "mozilla-beta": 6,
          "tracemonkey": 1
        }
      },
      "correlations": {
        "test_bug493251.html": {
          "Wrong number events (17) - got 0, expected 1": 0,
          "Wrong number events (16) - got 0, expected 1": 0
        }
      },
      "bugs": {
        "620721": 58
      }
    }
  }

I'll file a separate but for bug correlations.

Comment 8

[Jonathan Griffin (:jgriffin)]

sdwilsh, I'm closing this, please reopen if you'd like any revisions.

Comment 9

[Mark Côté [:mcote]]

Re: totals, makes sense to me.

Comment 10

[Jonathan Griffin (:jgriffin)]

(In reply to comment #8)
> sdwilsh, I'm closing this, please reopen if you'd like any revisions.

and I forgot to mention this is now available at http://brasstacks.mozilla.com/orangefactor/api/testerrors/, with an optional ?tree= argument, if you want to restrict to a specific tree.  The body of the GET should contain JSON as described in comment #2.

Comment 11

[(no longer active)]

I think you meant comment 4.  This works perfectly:

ehsanakhgari:~/moz/mozilla-central [10:27:40]$ curl -XGET 'http://brasstacks.mozilla.com/orangefactor/api/testerrors/?tree=mozilla-central' -d '
> [
>   ["test_timeupdate_small_files.html", "Test timed out."],
>   ["test_bug493251.html", "Wrong number events (16) - got 0, expected 1"]
> ]
> '
{"test_bug493251.html": {"Wrong number events (16) - got 0, expected 1": {"counts": {"total": 31, "trees": {"mozilla-central": 31}}, "correlations": {"test_timeupdate_small_files.html": {"Test timed out.": 0}}, "bugs": {"493251": 1, "655630": 2, "565245": 30}}}, "test_timeupdate_small_files.html": {"Test timed out.": {"counts": {"total": 39, "trees": {"mozilla-central": 39}}, "correlations": {"test_bug493251.html": {"Wrong number events (16) - got 0, expected 1": 0}}, "bugs": {"620721": 38}}}}

But is there any reason why we can't just pass the JSON input as a GET parameter?  Or as a POST parameter?

Comment 12

[Jonathan Griffin (:jgriffin)]

> 
> But is there any reason why we can't just pass the JSON input as a GET
> parameter?  Or as a POST parameter?

No particular reason, it just seemed to make more sense to me to put it in the body.  But I can allow it as a parameter too if you prefer.

Comment 13

[(no longer active)]

(In reply to comment #12)
> > 
> > But is there any reason why we can't just pass the JSON input as a GET
> > parameter?  Or as a POST parameter?
> 
> No particular reason, it just seemed to make more sense to me to put it in the
> body.  But I can allow it as a parameter too if you prefer.

I think that would make it much easier to use this both from the location bar and XHR.

Comment 14

[Jonathan Griffin (:jgriffin)]

Attachment: woo_server.py patch v0.3

New patch which allows the querydata to be specified as part of the url, e.g.,

http://127.0.0.1:8080/api/testerrors/?querydata=[%20%20[%22test_timeupdate_small_files.html%22%2C%20%22Test%20timed%20out.%22]%2C%20%20[%22test_bug493251.html%22%2C%20%22Wrong%20number%20events%20%2816%29%20-%20got%200%2C%20expected%201%22]%2C%20%20[%22test_bug493251.html%22%2C%20%22Wrong%20number%20events%20%2817%29%20-%20got%200%2C%20expected%201%22]]

(same as:
[
  ["test_timeupdate_small_files.html", "Test timed out."],
  ["test_bug493251.html", "Wrong number events (16) - got 0, expected 1"],
  ["test_bug493251.html", "Wrong number events (17) - got 0, expected 1"]
]
)

Comment 15

[Shawn Wilsher :sdwilsh]

(In reply to comment #3)
> You can pass a ?tree= parameter in the url, which will restrict it to the given
> tree, otherwise all trees are included.
Hmm, it looks like tree doesn't account for where it lives in the hg repo, but rather the last thing after the last '/', right?

Comment 16

[Jonathan Griffin (:jgriffin)]

(In reply to comment #15)
> (In reply to comment #3)
> > You can pass a ?tree= parameter in the url, which will restrict it to the given
> > tree, otherwise all trees are included.
> Hmm, it looks like tree doesn't account for where it lives in the hg repo,
> but rather the last thing after the last '/', right?

Right, it just uses the last segment of the repo's path, e.g., ?tree=services-central, not ?tree=services/services-central.

Comment 17

[Shawn Wilsher :sdwilsh]

(In reply to comment #16)
> Right, it just uses the last segment of the repo's path, e.g.,
> ?tree=services-central, not ?tree=services/services-central.
That's fine now, I'm just worried that we might be digging ourselves a hole for the distant future.  Probably me being paranoid though :)

Comment 18

[Mark Côté [:mcote]]

Comment on attachment 538380 [details] [diff] [review]
woo_server.py patch v0.3

Looks good and works fine.

Comment 19

[Jonathan Griffin (:jgriffin)]

Deployed.  See e.g., 

http://brasstacks.mozilla.com/orangefactor/api/testerrors/?tree=mozilla-central&querydata=[%20%20[%22test_timeupdate_small_files.html%22%2C%20%22Test%20timed%20out.%22]%2C%20%20[%22test_bug493251.html%22%2C%20%22Wrong%20number%20events%20%2816%29%20-%20got%200%2C%20expected%201%22]%2C%20%20[%22test_bug493251.html%22%2C%20%22Wrong%20number%20events%20%2817%29%20-%20got%200%2C%20expected%201%22]]