594373 - BuildAPI testrun efficiency reporting

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Syed Albiz [:salbiz]]

Attachment: initial patch for adding test efficiency reporting

Tracking ongoing work on adding reporting mechanism to display and visualize the time taken to complete test-runs (both unitttests and talos) to buildapi.

Comment 1

[Chris AtLee [:catlee]]

Comment on attachment 473046 [details] [diff] [review]
initial patch for adding test efficiency reporting

Looks pretty good.  Can you add some more comments in TestRunsQuery to explain what the different bits of the query are doing?

Comment 2

[Syed Albiz [:salbiz]]

(In reply to comment #1)
> Comment on attachment 473046 [details] [diff] [review]
> initial patch for adding test efficiency reporting
> 
> Looks pretty good.  Can you add some more comments in TestRunsQuery to explain
> what the different bits of the query are doing?

Yes, definitely. When I have a little more time, I'd also like to re-visit it entirely and see if it can't be sped up a bit, as it crawls along quite slowly at the moment.

Comment 3

[Syed Albiz [:salbiz]]

Attachment: added_clarification_to_TestRunQuery

Added a few comments to TestRunQuery which should hopefully make it a bit easier to see what's going on. Do you have any suggestions on how to structure that code to make it inherently more readable even without the added comments?

Comment 4

[Syed Albiz [:salbiz]]

Comment on attachment 474695 [details] [diff] [review]
added_clarification_to_TestRunQuery

Spent some time experimenting with ways to optimize the query. Moving some of the more complex query logic into the model/controller actually seemed to hurt initial load time (which is the real killer, since the beaker cache reduces subsequent load of groupings, etc immensely). We could try to optimize the database structure by adding some indexes to join on. That is probably out of scope for this bug though, I can file another one for that.

Comment 5

[Syed Albiz [:salbiz]]

Attachment: [tested]testrun_patch

Just re-based patch against updated templates, which required some reworking of display logic to handle groupings properly.

Comment 6

[Chris AtLee [:catlee]]

Comment on attachment 479220 [details] [diff] [review]
[tested]testrun_patch

Looks good except for the hardcoded limit of 300 in TestRunsQuery.  Can we get rid of that, or make it an argument to the function?

Comment 7

[Syed Albiz [:salbiz]]

Attachment: [tested]testrun_patch

That should have been dropped, it was only there for testing purposes. I've now removed it entirely. Over a date range of a few days we have less than 300 unique builders anyway, which is probably why it didn't show up during testing.

Comment 8

[Syed Albiz [:salbiz]]

Attachment: [tested]testrun_patch

re-applied against latest changeset on default branch

Comment 9

[Syed Albiz [:salbiz]]

Attachment: [tested]patch_fix

fixed JSON output and validated request headers using newly implemented controller schema. Previous patches were based against a revision that used a different header method of getting request parameters.

Comment 10

[Syed Albiz [:salbiz]]

closing this out, since the fix has landed and seems to work well