Fetch pods when displaying pods resources in overview pages #7070
Conversation
openshift-ci-robot
added
the
kind/cleanup
openshift-ci-robot
added
component/core
jeff-phillips-18
force-pushed
the
pods-load-pods-overview
branch
from
5f2b616
to
ff7b5c9
Compare
|
/retest |
|
/retest |
| it('should return pods for a given Deployment', () => { | ||
| const transformedData = getPodsForDeployments(sampleDeployments.data, MockResources); | ||
| expect(transformedData).toHaveLength(3); | ||
| }); | ||
|
|
||
| it('should return pods for a given DeploymentConfig', () => { | ||
| const transformedData = getPodsForStatefulSets(sampleDeployments.data, MockResources); | ||
| expect(transformedData).toHaveLength(3); | ||
| }); | ||
|
|
||
| it('should return pods for a given DaemonSet', () => { | ||
| const transformedData = getPodsForDaemonSets(sampleDaemonSets.data, MockResources); | ||
| expect(transformedData).toHaveLength(1); | ||
| }); | ||
|
|
||
| it('should return pods for a given CronJob', () => { | ||
| const transformedData = getPodsForCronJobs(sampleCronJobs.data, MockResources); | ||
| expect(transformedData).toHaveLength(1); | ||
| }); |
There was a problem hiding this comment.
For full coverage, can we have a test for each of these functions that test a lack of data (null and/or [])? The condition inside is super simple, it's a 1:1 relationship with the test data, so I think they are okay tests as is, but if we are going with such simple tests, we should cover both paths.
| import { | ||
| getPodsForDeploymentConfigs, | ||
| getPodsForDeployments, | ||
| getPodsForStatefulSets, | ||
| getPodsForDaemonSets, | ||
| getPodsForCronJobs, | ||
| } from '../pod-resource-utils'; |
There was a problem hiding this comment.
We don't have tests for the exposed methods under each of these... can we get tests for those?
| @@ -173,22 +181,122 @@ export const PodsOverview: React.SFC<PodsOverviewProps> = ({ | |||
| </Alert> | |||
| ) : null} | |||
| {_.isEmpty(filteredPods) ? ( | |||
| <span className="text-muted">{emptyMessage}</span> | |||
| <span className="text-muted">{loaded ? loadError || emptyMessage : <LoadingBox />}</span> | |||
There was a problem hiding this comment.
Hmm... looks like if we get a gateway error because the cluster going down, this doesn't really render properly; it's stuck not loaded but with an error. Did we want to check loaded and NOT an error?
React Dev Tools & the Response (Error) Object:
jeff-phillips-18
force-pushed
the
pods-load-pods-overview
branch
from
ff7b5c9
to
1a492f0
Compare
|
@andrewballantyne Added tests and updated to display loadError if there is one. |
|
/retest Looks like connection issues |
|
/retest |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andrewballantyne, christianvogt, jeff-phillips-18 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
Fixes
https://issues.redhat.com/browse/ODC-4966 (portions of it)
Description
Update the pod resources section of overview pages to retrieve pods for display rather than depending upon the passed in item's resources to be kept up to date.
Non-functional change
Browser conformance:
/kind cleanup