Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactored alerting test, and added wait logic to reduce flakiness. #953

Merged
merged 1 commit into from
Nov 21, 2023
+7 −2
Merged

Refactored alerting test, and added wait logic to reduce flakiness. #953

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions cypress/integration/plugins/alerting-dashboards-plugin/monitors_dashboard_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const clusterHealthMonitor = {
severity: '1',
condition: {
script: {
source: 'ctx.results[0].status != "green"',
source: 'ctx.results[0].status != "blue"',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For context, this was changed to blue because the test was failing when run against multinode clusters. The indexes used by the tests were only reliably in a yellow state on single node clusters because they were configured to have replicas. Rather than reconfigure the test indexes, it was less invasive for other tests to adjust the condition used by this test.

Copy link
Collaborator

@Hailong-am Hailong-am Nov 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add comments of this change with the context,so we can understand in future when look back at this code piece

lang: 'painless',
},
},
Expand Down Expand Up @@ -107,8 +107,13 @@ describe('Monitors dashboard page', () => {
});

it('Displays expected number of alerts', () => {
// Wait for table to finish loading
cy.get('tbody > tr').should(($tr) =>
expect($tr).to.have.length.greaterThan(1)
);

// Ensure the 'Monitor name' column is sorted in ascending order by sorting another column first
cy.contains('Last updated by').click({ force: true });
cy.contains('Last notification time').click({ force: true });
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For context, the Last updated by column was removed from this UI in PR opensearch-project/alerting-dashboards-plugin#767

cy.contains('Monitor name').click({ force: true });

testMonitors.forEach((entry) => {
Expand Down
Loading