diff --git a/treeherder/model/management/commands/cache_failure_history.py b/treeherder/model/management/commands/cache_failure_history.py index ddf1ae007..4235a513e 100644 --- a/treeherder/model/management/commands/cache_failure_history.py +++ b/treeherder/model/management/commands/cache_failure_history.py @@ -3,9 +3,9 @@ import datetime from django.core.management.base import BaseCommand from treeherder.model.models import OptionCollection -from treeherder.push_health.push_health import (fixed_by_commit_history_days, - get_history, - intermittent_history_days) +from treeherder.push_health.tests import (fixed_by_commit_history_days, + get_history, + intermittent_history_days) from treeherder.webapp.api.utils import REPO_GROUPS diff --git a/treeherder/push_health/push_health.py b/treeherder/push_health/tests.py similarity index 97% rename from treeherder/push_health/push_health.py rename to treeherder/push_health/tests.py index 3fb115937..b22f254dc 100644 --- a/treeherder/push_health/push_health.py +++ b/treeherder/push_health/tests.py @@ -79,7 +79,7 @@ def get_history(failure_classification_id, push_date, num_days, option_map, repo # though the job failed, we count it as a 'pass' for the test in question. This 'pass' is # used for the pass/fail ratio on the test to help determine if it is intermittent. # -def get_push_failures(push, option_map): +def get_current_test_failures(push, option_map): all_testfailed = Job.objects.filter( push=push, tier__lte=2, @@ -168,7 +168,7 @@ def has_line(failure_line, log_line_list): return next((find_line for find_line in log_line_list if find_line['line_number'] == failure_line.line), False) -def get_push_health_test_failures(push, repository_ids): +def get_test_failures(push, repository_ids): # query for jobs for the last two weeks excluding today # find tests that have failed in the last 14 days # this is very cache-able for reuse on other pushes. @@ -186,7 +186,7 @@ def get_push_health_test_failures(push, repository_ids): fixed_by_commit_history_days, option_map, repository_ids) - push_failures, unsupported_jobs = get_push_failures(push, option_map) + push_failures, unsupported_jobs = get_current_test_failures(push, option_map) filtered_push_failures = [ failure for failure in push_failures if filter_failure(failure) ] diff --git a/treeherder/webapp/api/push.py b/treeherder/webapp/api/push.py index fb462c3e5..632356fa0 100644 --- a/treeherder/webapp/api/push.py +++ b/treeherder/webapp/api/push.py @@ -13,7 +13,7 @@ from treeherder.model.models import (Job, JobType, Push, Repository) -from treeherder.push_health.push_health import get_push_health_test_failures +from treeherder.push_health.tests import get_test_failures from treeherder.webapp.api.serializers import PushSerializer from treeherder.webapp.api.utils import (REPO_GROUPS, to_datetime, @@ -211,7 +211,7 @@ class PushViewSet(viewsets.ViewSet): except Push.DoesNotExist: return Response("No push with id: {0}".format(pk), status=HTTP_404_NOT_FOUND) - push_health_test_failures = get_push_health_test_failures(push, REPO_GROUPS['trunk']) + push_health_test_failures = get_test_failures(push, REPO_GROUPS['trunk']) return Response({'needInvestigation': len(push_health_test_failures['needInvestigation'])}) @@ -227,7 +227,7 @@ class PushViewSet(viewsets.ViewSet): except Push.DoesNotExist: return Response("No push with revision: {0}".format(revision), status=HTTP_404_NOT_FOUND) - push_health_test_failures = get_push_health_test_failures(push, REPO_GROUPS['trunk']) + push_health_test_failures = get_test_failures(push, REPO_GROUPS['trunk']) test_result = 'pass' if len(push_health_test_failures['unsupported']): test_result = 'indeterminate' @@ -238,37 +238,37 @@ class PushViewSet(viewsets.ViewSet): 'revision': revision, 'id': push.id, 'result': test_result, - 'metrics': [ - { + 'metrics': { + 'tests': { 'name': 'Tests', 'result': test_result, - 'failures': push_health_test_failures, + 'details': push_health_test_failures, }, - { + 'linting': { + 'name': 'Linting (Not yet implemented)', + 'result': 'none', + 'details': ['lint stuff would be good'], + }, + 'builds': { 'name': 'Builds (Not yet implemented)', - 'result': 'pass', + 'result': 'none', 'details': ['Wow, everything passed!'], }, - { - 'name': 'Linting (Not yet implemented)', - 'result': 'pass', - 'details': ['Gosh, this code is really nicely formatted.'], - }, - { + 'coverage': { 'name': 'Coverage (Not yet implemented)', - 'result': 'pass', + 'result': 'none', 'details': [ 'Covered 42% of the tests that are needed for feature ``foo``.', 'Covered 100% of the tests that are needed for feature ``bar``.', 'The ratio of people to cake is too many...', ], }, - { + 'performance': { 'name': 'Performance (Not yet implemented)', - 'result': 'pass', + 'result': 'none', 'details': ['Ludicrous Speed'], }, - ], + }, }) @cache_memoize(60 * 60) diff --git a/ui/push-health/Health.jsx b/ui/push-health/Health.jsx index 6e1611b26..178aa7cb9 100644 --- a/ui/push-health/Health.jsx +++ b/ui/push-health/Health.jsx @@ -14,6 +14,7 @@ import PushModel from '../models/push'; import { resultColorMap } from './helpers'; import Metric from './Metric'; import Navigation from './Navigation'; +import TestFailures from './TestFailures'; export default class Health extends React.PureComponent { constructor(props) { @@ -25,7 +26,8 @@ export default class Health extends React.PureComponent { user: { isLoggedIn: false }, revision: params.get('revision'), repo: params.get('repo'), - healthData: null, + metrics: {}, + result: null, failureMessage: null, notifications: [], }; @@ -55,9 +57,7 @@ export default class Health extends React.PureComponent { updatePushHealth = async () => { const { repo, revision } = this.state; const { data, failureStatus } = await PushModel.getHealth(repo, revision); - const newState = !failureStatus - ? { healthData: data } - : { failureMessage: data }; + const newState = !failureStatus ? data : { failureMessage: data }; this.setState(newState); }; @@ -85,17 +85,17 @@ export default class Health extends React.PureComponent { render() { const { - healthData, + metrics, + result, user, repo, revision, failureMessage, notifications, } = this.state; + const { tests, linting, builds, coverage, performance } = metrics; const { currentRepo } = this.props; - const overallResult = healthData - ? resultColorMap[healthData.result] - : 'none'; + const overallResult = result ? resultColorMap[result] : 'none'; return ( @@ -105,7 +105,7 @@ export default class Health extends React.PureComponent { notifications={notifications} clearNotification={this.clearNotification} /> - {healthData && ( + {!!tests && !!currentRepo && (

@@ -121,20 +121,25 @@ export default class Health extends React.PureComponent {

- {healthData.metrics.map(metric => ( - - + + + + {[linting, builds, coverage, performance].map(metric => ( + + +
+ {metric.details.map(detail => ( +
{detail}
+ ))} +
+
))} @@ -142,7 +147,7 @@ export default class Health extends React.PureComponent { )} {failureMessage && } - {!failureMessage && !healthData && } + {!failureMessage && !tests && } ); @@ -151,5 +156,9 @@ export default class Health extends React.PureComponent { Health.propTypes = { location: PropTypes.object.isRequired, - currentRepo: PropTypes.object.isRequired, + currentRepo: PropTypes.object, +}; + +Health.defaultProps = { + currentRepo: null, }; diff --git a/ui/push-health/Metric.jsx b/ui/push-health/Metric.jsx index 1eb0062db..c0102c082 100644 --- a/ui/push-health/Metric.jsx +++ b/ui/push-health/Metric.jsx @@ -8,7 +8,6 @@ import { import { Button, Badge, Row, Col, Collapse, Card, CardBody } from 'reactstrap'; import { resultColorMap } from './helpers'; -import TestFailures from './TestFailures'; export default class Metric extends React.PureComponent { constructor(props) { @@ -17,7 +16,7 @@ export default class Metric extends React.PureComponent { const { result } = this.props; this.state = { - detailsShowing: result !== 'pass', + detailsShowing: !['pass', 'none'].includes(result), }; } @@ -27,17 +26,7 @@ export default class Metric extends React.PureComponent { render() { const { detailsShowing } = this.state; - const { - result, - name, - details, - failures, - repo, - revision, - user, - notify, - currentRepo, - } = this.props; + const { result, name, children } = this.props; const resultColor = resultColorMap[result]; const expandIcon = detailsShowing ? faMinusSquare : faPlusSquare; @@ -63,24 +52,7 @@ export default class Metric extends React.PureComponent { - - {name === 'Tests' && ( - - )} - {details && - details.map(detail => ( -
- {detail} -
- ))} -
+ {children}
@@ -91,18 +63,7 @@ export default class Metric extends React.PureComponent { } Metric.propTypes = { - repo: PropTypes.string.isRequired, - currentRepo: PropTypes.object.isRequired, - revision: PropTypes.string.isRequired, result: PropTypes.string.isRequired, name: PropTypes.string.isRequired, - user: PropTypes.object.isRequired, - notify: PropTypes.func.isRequired, - details: PropTypes.array, - failures: PropTypes.object, -}; - -Metric.defaultProps = { - details: null, - failures: null, + children: PropTypes.object.isRequired, }; diff --git a/ui/push-health/TestFailures.jsx b/ui/push-health/TestFailures.jsx index 392f116ee..e15da2009 100644 --- a/ui/push-health/TestFailures.jsx +++ b/ui/push-health/TestFailures.jsx @@ -3,54 +3,58 @@ import PropTypes from 'prop-types'; import ClassificationGroup from './ClassificationGroup'; import UnsupportedGroup from './UnsupportedGroup'; +import Metric from './Metric'; export default class TestFailures extends React.PureComponent { render() { - const { failures, repo, revision, user, notify, currentRepo } = this.props; - const { needInvestigation, intermittent, unsupported } = failures; + const { data, repo, revision, user, notify, currentRepo } = this.props; + const { name, result, details } = data; + const { needInvestigation, intermittent, unsupported } = details; const needInvestigationLength = Object.keys(needInvestigation).length; return ( -
- - - -
+ +
+ + + +
+
); } } TestFailures.propTypes = { - failures: PropTypes.object.isRequired, + data: PropTypes.object.isRequired, user: PropTypes.object.isRequired, repo: PropTypes.string.isRequired, currentRepo: PropTypes.object.isRequired,