Page MenuHomePhabricator

Aggregate lint, unit information in HarbormasterBuildable
AbandonedPublic

Authored by avivey on Aug 17 2016, 10:55 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Aug 11, 2:05 AM
Unknown Object (File)
Wed, Aug 10, 12:55 PM
Unknown Object (File)
Tue, Aug 2, 7:51 AM
Unknown Object (File)
Thu, Jul 21, 12:16 PM
Unknown Object (File)
Thu, Jul 21, 7:06 AM
Unknown Object (File)
Tue, Jul 19, 7:46 PM
Unknown Object (File)
Mon, Jul 18, 11:37 PM
Unknown Object (File)
Mon, Jul 18, 11:37 PM
Subscribers
Restricted Owners Package

Details

Summary

We currently store each test/lint as Message, and then calculate the sum of this when loading a revision; This change moves the calculations into the Engine, running it in a daemon when there's a change, instead of in the web tier when a user is waiting.

This isn't making it all the way to "don't load all unit results when displaying revision," but does go a long way towards making that easy (And marks most places that will need to change for that).

In terms of user-facing-changes, this change shows the correct stars for lint/unit in the Revision History tab, and shows more correct/less confusing information in the Diff Details Section. It also shows 2 items under "Unit" and "Lint" in the Details pane - one only shown if the user skipped, and the other shows HM status. Also showing unit excuse + count.

pasted_file (93×434 px, 13 KB)

Next steps / revisions:

  • Add "Summary" or "Meta" Harbormaster Lint/Unit messages - important for Lints to show "Lint OK", and useful for saying "100K unit tests passed" without actually storing the details. Maybe also get the Excuse in there.
  • Migration: This basically boils down to running ./bin/harbormaster update --force lots of times, which is slow, but safe to run while the system is up. Also possibly not very interesting for old diffs, so some installs might want to skip it.

    Alternatively, restarting a remote build from the revision page will also update the Build to be displayed correctly.

See T9365, T10635.

Test Plan

Built "remote CI" jobs that wait for messages, and send messages with all sorts of conditions.
Look at details, history stars, harbormaster_buildable.details in the db.

Diff Detail

Repository
rP Phabricator
Branch
stars-no-api
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 13450
Build 17286: Run Core Tests
Build 17285: arc lint + arc unit

Event Timeline

avivey retitled this revision from to WIP: Aggregate lint, unit information in buildTarget.
avivey edited the test plan for this revision. (Show Details)
Owners added a subscriber: Restricted Owners Package.Aug 17 2016, 10:55 PM
avivey removed subscribers: epriestley, Restricted Owners Package.
Owners added a subscriber: Restricted Owners Package.Aug 19 2016, 12:01 AM

I almost want to use HarbormasterBuildLintMessage to also "send message" about "lint status": "lint skipped (by user)" or "lint ok". Calling it "message" kinda makes it reasonable...

avivey removed a subscriber: Restricted Owners Package.Aug 24 2016, 5:59 PM
  • aggregate in Buildable instead of BuildTarget
Owners added a subscriber: Restricted Owners Package.Aug 24 2016, 5:59 PM
avivey removed a subscriber: Restricted Owners Package.Aug 25 2016, 1:09 AM
  • move getSummaryUnitStatus to DifferntialDiff
  • Show count of tests in Details
  • remove unused param
Owners added a subscriber: Restricted Owners Package.Aug 25 2016, 1:11 AM
avivey retitled this revision from WIP: Aggregate lint, unit information in buildTarget to Aggregate lint, unit information in HarbormasterBuildable.Aug 25 2016, 6:50 PM
avivey updated this object.
avivey edited the test plan for this revision. (Show Details)
avivey removed a subscriber: Restricted Owners Package.
Owners added a subscriber: Restricted Owners Package.Aug 25 2016, 6:51 PM

See D16483; It will take a while before T9365 is ready for fixing, so abandoning this revision for now.