From d734c09f64c1320722e28ca84c6d5940cecfba4a Mon Sep 17 00:00:00 2001 From: Sarah Clements Date: Thu, 28 Jan 2021 16:55:56 -0800 Subject: [PATCH] Bug 1678760 - Push Health status progress fixes (#6995) * Bug 1678760 - Fix push health status progress indicator * override testfailed value from push.get_status in both health APIs * use updated failure counts for tab summaries * UI fixes - remove spinner in push health indicators since it was causing pushes with data older than 4 months to continuously spin * the percentage complete in the progress status indicator should default to 100% because data older than 4 months will not have any data about task status --- treeherder/webapp/api/push.py | 20 ++++++++++++++++++-- ui/helpers/display.js | 7 ++++--- ui/job-view/pushes/Push.jsx | 2 +- ui/push-health/Health.jsx | 7 +++---- ui/push-health/MyPushes.jsx | 8 ++++++-- ui/shared/PushHealthStatus.jsx | 6 ++---- 6 files changed, 34 insertions(+), 16 deletions(-) diff --git a/treeherder/webapp/api/push.py b/treeherder/webapp/api/push.py index 8867e5a7b..1cdcf193e 100644 --- a/treeherder/webapp/api/push.py +++ b/treeherder/webapp/api/push.py @@ -272,6 +272,12 @@ class PushViewSet(viewsets.ViewSet): lint_failure_count = len(push_health_lint_failures) test_in_progress_count = None + status = push.get_status() + total_failures = test_failure_count + build_failure_count + lint_failure_count + # Override the testfailed value added in push.get_status so that it aligns with how we detect lint, build and test failures + # for the push health API's (total_failures doesn't include known intermittent failures) + status['testfailed'] = total_failures + if with_history: serializer = PushSerializer([push], many=True) commit_history = serializer.data @@ -291,7 +297,7 @@ class PushViewSet(viewsets.ViewSet): 'needInvestigation': test_failure_count + build_failure_count + lint_failure_count, - 'status': push.get_status(), + 'status': status, 'history': commit_history, 'metrics': { 'linting': { @@ -360,6 +366,16 @@ class PushViewSet(viewsets.ViewSet): elif metric_result == 'fail': push_result = metric_result + status = push.get_status() + total_failures = ( + len(push_health_test_failures['needInvestigation']) + + len(build_failures) + + len(lint_failures) + ) + # Override the testfailed value added in push.get_status so that it aligns with how we detect lint, build and test failures + # for the push health API's (total_failures doesn't include known intermittent failures) + status['testfailed'] = total_failures + newrelic.agent.record_custom_event( 'push_health_need_investigation', { @@ -398,7 +414,7 @@ class PushViewSet(viewsets.ViewSet): 'details': build_failures, }, }, - 'status': push.get_status(), + 'status': status, } ) diff --git a/ui/helpers/display.js b/ui/helpers/display.js index 338af3d87..790639b98 100644 --- a/ui/helpers/display.js +++ b/ui/helpers/display.js @@ -44,10 +44,11 @@ export const getSearchWords = function getHighlighterArray(text) { export const getPercentComplete = function getPercentComplete(counts) { const { pending, running, completed } = counts; - const inProgress = pending + running; - const total = completed + inProgress; + const total = completed + pending + running; - return total > 0 ? Math.floor((completed / total) * 100) : 0; + // pushes older than our 4-month data cutoff we want to display as 100% complete + // in the status progress indicator even though the total counts will be 0 + return total > 0 ? Math.floor((completed / total) * 100) : 100; }; export const formatArtifacts = function formatArtifacts(data, artifactParams) { diff --git a/ui/job-view/pushes/Push.jsx b/ui/job-view/pushes/Push.jsx index c0d3cc1af..ab61fa9b4 100644 --- a/ui/job-view/pushes/Push.jsx +++ b/ui/job-view/pushes/Push.jsx @@ -681,7 +681,7 @@ class Push extends React.PureComponent { widthClass="ml-4 mb-3" commitShaClass="text-monospace" > - {showPushHealthSummary && ( + {showPushHealthSummary && pushHealthStatus && (
- {`[${needInvestigationCount} failures] Push Health`} + {`[${ + (status && status.testfailed) || 0 + } failures] Push Health`} {!!tests && !!currentRepo && ( diff --git a/ui/push-health/MyPushes.jsx b/ui/push-health/MyPushes.jsx index 1a1d7589a..cc830eae2 100644 --- a/ui/push-health/MyPushes.jsx +++ b/ui/push-health/MyPushes.jsx @@ -96,8 +96,12 @@ class MyPushes extends React.Component { if (!failureStatus && data.length) { stateChanges.pushMetrics = data; - } else { + } else if (failureStatus) { notify(`There was a problem retrieving push metrics: ${data}`, 'danger'); + } else { + notify( + `Didn't find push data for you in ${selectedRepo}. Try selecting a different option.`, + ); } this.setState(stateChanges); @@ -120,7 +124,7 @@ class MyPushes extends React.Component { const totalNeedInvestigation = pushMetrics.length ? pushMetrics - .map((push) => push.needInvestigation) + .map((push) => push.status.testfailed) .reduce((total, count) => total + count) : 0; diff --git a/ui/shared/PushHealthStatus.jsx b/ui/shared/PushHealthStatus.jsx index 5f73a06d0..58cb8a83e 100644 --- a/ui/shared/PushHealthStatus.jsx +++ b/ui/shared/PushHealthStatus.jsx @@ -1,6 +1,6 @@ import React, { Component } from 'react'; import PropTypes from 'prop-types'; -import { Badge, Spinner } from 'reactstrap'; +import { Badge } from 'reactstrap'; import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; import { faCheck, @@ -85,7 +85,7 @@ class PushHealthStatus extends Component { return ( - {needInvestigation !== null ? ( + {needInvestigation !== null && ( - ) : ( - )} );