Bug 1906896: show empty message for no alerts #7523

debsmita1 · 2020-12-11T10:30:49Z

Fixes:
https://issues.redhat.com/browse/ODC-5116

Analysis / Root cause:

Solution Description:

added a loader
it shows No Alerts found if there are no alerts

Unit test coverage report:
Added new test

Screen shots / Gifs for design review:

Browser conformance:

Chrome
Firefox
Safari
Edge

sahil143

@debsmita1 When visiting the Alerts tab page starts to hang and becomes unresponsive. according to the warning in the console, useEffect hook is being called repeatedly.

sahil143 · 2020-12-11T13:58:21Z

@debsmita1 Unit tests are failing

karthikjeeyar · 2020-12-11T14:03:10Z

frontend/packages/dev-console/src/components/monitoring/alerts/MonitoringAlerts.tsx

+    return <LoadingBox />;
+  }
+  if (thanosAlertsAndRules?.alerts?.length === 0) {
+    return <EmptyBox label="Alerts" />;


localize the string.

openshift-ci-robot · 2020-12-11T17:31:48Z

@debsmita1: This pull request references Bugzilla bug 1906896, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.7.0) matches configured target release for branch (4.7.0)
bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1906896: show empty message for no alerts

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

debsmita1 · 2020-12-11T17:46:32Z

@debsmita1 When visiting the Alerts tab page starts to hang and becomes unresponsive. according to the warning in the console, useEffect hook is being called repeatedly.

@sahil143 This issue is being fixed in PR #7480

debsmita1 · 2020-12-14T11:27:27Z

@debsmita1 Unit tests are failing

fixed

openshift-ci-robot · 2020-12-14T11:28:07Z

@debsmita1: This pull request references Bugzilla bug 1906896, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.7.0) matches configured target release for branch (4.7.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1906896: show empty message for no alerts

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

debsmita1 · 2020-12-14T12:36:39Z

/test frontend

debsmita1 · 2020-12-14T17:25:52Z

/retest

jerolimov · 2020-12-15T00:48:40Z

...nd/packages/dev-console/src/components/monitoring/alerts/__tests__/MonitoringAlerts.spec.tsx

  it('should render monitoring alerts', () => {
    const wrapper = shallow(<MonitoringAlerts {...monitoringAlertsProps} />);
    expect(wrapper.find(FilterToolbar).exists()).toBe(true);
    expect(wrapper.find(Table).exists()).toBe(true);
    expect(wrapper.find(TableHeader).exists()).toBe(true);
    expect(wrapper.find(TableBody).exists()).toBe(true);
  });
+  spyPrometheusRulesPoll.mockReturnValueOnce([{}, null, false]);
+  it('should show empty state message', () => {
+    const wrapper = shallow(<MonitoringAlerts {...monitoringAlertsProps} />);
+    expect(wrapper.find(EmptyBox).exists()).toBe(true);
+  });


@debsmita1 Really great to see more tests coming. In jest all testcases and code which is required to run a test case should be part of a it or beforeEach, afterEach, etc. function. Only test setup calls which are similar for all tests could be done before all tests.

In your case you modify the spyPrometheusRulesPoll mock between two test functions. This works only fine when you run your test from top to bottom 'once'. The first mock call saved one result, the second one the second (empty) version.

The it calls just register "test" calls (your callback function) and it will be run later. When you add console.log statements you can see that the tests are runned in this order:

console.log('1') it('test 1' => { console.log('3') }) console.log('2') it('test 2' => { console.log('4') })

And this is an issue if you don't run all tests at the same time or in a different order. You can test this with this command:

yarn test packages/dev-console/src/components/monitoring/alerts/__tests__/MonitoringAlerts.spec.tsx -t empty

This runs both mock "Once" calls and then runs the second it callback. But the mock returns the first value here.

So the general idea should be that each test works completely independent. You can reach this by just moving the setup of spyPrometheusRulesPoll also into the it function.

jerolimov · 2020-12-15T00:49:03Z

/retest

jerolimov

Empty message was shown and I really like the new tests here, but found a small nit in the tests, also if they are green.
After this small change I would like to add a lgtm here. :)

openshift-merge-robot · 2020-12-16T20:50:48Z

@debsmita1: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/frontend	`bfb963f`	link	`/test frontend`
ci/prow/e2e-gcp-console	`bfb963f`	link	`/test e2e-gcp-console`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

vikram-raj · 2020-12-18T17:05:46Z

frontend/packages/dev-console/src/components/monitoring/alerts/MonitoringAlerts.tsx

+    return <LoadingBox />;
+  }
+  if (_.isEmpty(response?.data)) {
+    return <EmptyBox label={t('devconsle~Alerts')} />;


Suggested change

return <EmptyBox label={t('devconsle~Alerts')} />;

return <EmptyBox label={t('devconsole~Alerts')} />;

vikram-raj · 2020-12-18T18:26:40Z

frontend/packages/dev-console/src/components/monitoring/alerts/MonitoringAlerts.tsx

+  if (loading && !loadError) {
+    return <LoadingBox />;
+  }
+  if (_.isEmpty(response?.data)) {


Suggested change

if (_.isEmpty(response?.data)) {

if (_.isEmpty(response?.data?.groups)) {

openshift-bot · 2020-12-22T18:42:53Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-12-22T23:29:09Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-12-23T01:26:21Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-12-23T03:23:08Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-12-23T05:06:48Z

/retest

Please review the full test history for this PR and help us cut down flakes.

debsmita1 · 2020-12-23T06:48:25Z

/test kubevirt-plugin

openshift-bot · 2020-12-23T09:39:48Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-12-23T11:23:49Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-12-23T13:20:49Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-12-23T15:17:33Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-12-23T16:35:18Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-12-23T16:48:16Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-12-23T17:01:17Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-12-23T18:57:43Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-12-23T19:40:57Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-12-23T20:45:53Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-12-23T21:37:57Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-12-23T22:29:54Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-12-23T22:55:51Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-12-23T23:47:51Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-12-24T00:01:13Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-12-24T00:14:07Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-12-24T01:05:49Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci · 2020-12-24T02:44:03Z

@debsmita1: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/kubevirt-plugin	`6f926d3`	link	`/test kubevirt-plugin`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

openshift-bot · 2020-12-24T03:54:51Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci-robot · 2020-12-24T04:09:24Z

@debsmita1: All pull requests linked via external trackers have merged:

openshift/console#7523

Bugzilla bug 1906896 has been moved to the MODIFIED state.

In response to this:

Bug 1906896: show empty message for no alerts

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

debsmita1 · 2021-01-13T18:16:46Z

/kind bug

openshift-ci-robot requested review from abhinandan13jan and andrewballantyne December 11, 2020 10:31

openshift-ci-robot added the component/dev-console Related to dev-console label Dec 11, 2020

sahil143 suggested changes Dec 11, 2020

View reviewed changes

karthikjeeyar reviewed Dec 11, 2020

View reviewed changes

debsmita1 changed the title ~~show empty message for no alerts~~ Bug 1906896: show empty message for no alerts Dec 11, 2020

openshift-ci-robot added the bugzilla/severity-low Referenced Bugzilla bug's severity is low for the branch this PR is targeting. label Dec 11, 2020

openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Dec 11, 2020

debsmita1 force-pushed the alert-empty-state branch 2 times, most recently from db7490d to 0920284 Compare December 14, 2020 11:26

debsmita1 requested review from sahil143 and karthikjeeyar December 14, 2020 11:28

jerolimov reviewed Dec 15, 2020

View reviewed changes

jerolimov suggested changes Dec 15, 2020

View reviewed changes

debsmita1 force-pushed the alert-empty-state branch 2 times, most recently from ef7344e to bfb963f Compare December 16, 2020 18:49

debsmita1 requested a review from jerolimov December 16, 2020 18:49

vikram-raj reviewed Dec 18, 2020

View reviewed changes

vikram-raj suggested changes Dec 18, 2020

View reviewed changes

debsmita1 force-pushed the alert-empty-state branch from bfb963f to 683afe6 Compare December 21, 2020 06:32

show empty message for no alerts

6f926d3

openshift-merge-robot merged commit 94bcdd6 into openshift:master Dec 24, 2020

spadgett added this to the v4.7 milestone Jan 7, 2021

openshift-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Jan 13, 2021

	return <EmptyBox label={t('devconsle~Alerts')} />;
	return <EmptyBox label={t('devconsole~Alerts')} />;

	if (_.isEmpty(response?.data)) {
	if (_.isEmpty(response?.data?.groups)) {

Bug 1906896: show empty message for no alerts #7523

Bug 1906896: show empty message for no alerts #7523

Conversation

debsmita1 commented Dec 11, 2020 • edited

sahil143 left a comment • edited

Choose a reason for hiding this comment

sahil143 commented Dec 11, 2020

karthikjeeyar Dec 11, 2020

Choose a reason for hiding this comment

openshift-ci-robot commented Dec 11, 2020

debsmita1 commented Dec 11, 2020

debsmita1 commented Dec 14, 2020

openshift-ci-robot commented Dec 14, 2020

debsmita1 commented Dec 14, 2020

debsmita1 commented Dec 14, 2020

jerolimov Dec 15, 2020

Choose a reason for hiding this comment

jerolimov commented Dec 15, 2020

jerolimov left a comment

Choose a reason for hiding this comment

openshift-merge-robot commented Dec 16, 2020

vikram-raj Dec 18, 2020

Choose a reason for hiding this comment

vikram-raj Dec 18, 2020

Choose a reason for hiding this comment

openshift-bot commented Dec 22, 2020

openshift-bot commented Dec 22, 2020

openshift-bot commented Dec 23, 2020

openshift-bot commented Dec 23, 2020

openshift-bot commented Dec 23, 2020

debsmita1 commented Dec 23, 2020

openshift-bot commented Dec 23, 2020

openshift-bot commented Dec 23, 2020

openshift-bot commented Dec 23, 2020

openshift-bot commented Dec 23, 2020

openshift-bot commented Dec 23, 2020

openshift-bot commented Dec 23, 2020

openshift-bot commented Dec 23, 2020

openshift-bot commented Dec 23, 2020

openshift-bot commented Dec 23, 2020

openshift-bot commented Dec 23, 2020

openshift-bot commented Dec 23, 2020

openshift-bot commented Dec 23, 2020

openshift-bot commented Dec 23, 2020

openshift-bot commented Dec 23, 2020

openshift-bot commented Dec 24, 2020

openshift-bot commented Dec 24, 2020

openshift-bot commented Dec 24, 2020

openshift-ci bot commented Dec 24, 2020

openshift-bot commented Dec 24, 2020

openshift-ci-robot commented Dec 24, 2020

debsmita1 commented Jan 13, 2021

debsmita1 commented Dec 11, 2020 •

edited

sahil143 left a comment •

edited