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.

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

[Cypress Updates] Trace analytics updates #2251

Merged
merged 7 commits into from
Nov 12, 2024

Conversation

ritvibhatt
Copy link
Contributor

@ritvibhatt ritvibhatt commented Nov 8, 2024

Description

Updates trace analytics cypress tests

Screenshot 2024-11-08 at 11 25 23 AM Screenshot 2024-11-08 at 11 37 00 AM Screenshot 2024-11-08 at 11 37 51 AM

Issues Resolved

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented.
    • New functionality has javadoc added
    • New functionality has user manual doc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@ps48
Copy link
Member

ps48 commented Nov 11, 2024

@ritvibhatt Awesome work! 👍🏽 I see the tests pass, the error in CI is a bug in github actions:
Error: Failed to CreateArtifact: Received non-retryable error: Failed request: (409) Conflict: an artifact with this name already exists on the workflow run from here: actions/upload-artifact#478. Will retry to see if this can be solved.

@ritvibhatt ritvibhatt marked this pull request as ready for review November 11, 2024 17:15
cy.contains(' (1)').should('exist');
cy.contains('03/25/2021 10:21:22').should('exist');
});
});

//here
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missed comment remove.

Signed-off-by: Ritvi Bhatt <[email protected]>
Copy link
Collaborator

@TackAdam TackAdam left a comment

Choose a reason for hiding this comment

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

LGTM! Great work, thanks.

Copy link
Collaborator

@RyanL1997 RyanL1997 left a comment

Choose a reason for hiding this comment

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

Hi @ritvibhatt , thanks for taking this on, and I just left some comments.

cy.get('.euiContextMenuItem__text').eq(2).contains('Disable all');
cy.get('.euiContextMenuItem__text').eq(3).contains('Invert inclusion');
cy.get('.euiContextMenuItem__text').eq(4).contains('Invert enabled/disabled');
cy.get('.euiContextMenuItem__text').eq(5).contains('Remove all');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not a blocker, however, I was wondering if we can do something like this:

const menuSelector = '.euiContextMenuItem__text';
const expectedMenuItems = [
  'Enable all',
  'Disable all',
  'Invert inclusion',
  'Invert enabled/disabled',
  'Remove all',
];

cy.verifyContextMenuItems(menuSelector, expectedMenuItems);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like cypress can do something like that by iterating through each menu item to check but I think it might be more clear to have the order of the menu items laid out so holding off on this for now

@TackAdam TackAdam merged commit 58d58a7 into opensearch-project:main Nov 12, 2024
10 of 18 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Nov 12, 2024
* updated cypress testing for traces

Signed-off-by: Ritvi Bhatt <[email protected]>

* update snapshots

Signed-off-by: Ritvi Bhatt <[email protected]>

* wait for page to load before checking rows

Signed-off-by: Ritvi Bhatt <[email protected]>

* remove forgetten comment

Signed-off-by: Ritvi Bhatt <[email protected]>

* centralize exapnding service view

Signed-off-by: Ritvi Bhatt <[email protected]>

---------

Signed-off-by: Ritvi Bhatt <[email protected]>
Co-authored-by: Ritvi Bhatt <[email protected]>
Co-authored-by: Adam Tackett <[email protected]>
(cherry picked from commit 58d58a7)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
mengweieric pushed a commit that referenced this pull request Nov 12, 2024
* updated cypress testing for traces



* update snapshots



* wait for page to load before checking rows



* remove forgetten comment



* centralize exapnding service view



---------




(cherry picked from commit 58d58a7)

Signed-off-by: Ritvi Bhatt <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Ritvi Bhatt <[email protected]>
Co-authored-by: Adam Tackett <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants