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

[OSD] Skip cypress tests for data explorer with security enabled #1120

Closed

Conversation

kavilla
Copy link
Member

@kavilla kavilla commented Feb 22, 2024

Description

When security is enabled the url contains the tenancy. For example,

localhost:5601/app/dashboards?security_tenant=private

Which will fail tests that verify URLs if written without security plugin and tenancy enabled. Also with the security plugin enabled then the query string is masked. Which will make it more difficult to verify URL tests without checking the expecting elements within the app. So to mitigate this we will just check the URLs only if security is not enabled but keep the tests running if enabled.

Also includes browserPermissions as one of the tests would pop up and asks but needs permission. Also commented on the cy.visit. Depending on how the tests are executed this could be a re-navigate on origin. But since it just focuses on visiting but fails the tests when it does visit but not validate anything on the location then commented out.

Issues Resolved

opensearch-project/OpenSearch-Dashboards#5633

Check List

  • 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.

cy.visit(clipboardData);
cy.waitForLoader();
// TODO: tries to re-route with a different origin
// cy.visit(clipboardData);
Copy link
Member Author

Choose a reason for hiding this comment

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

@leanneeliatra would you think this is ok?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes this looks good to me @kavilla
Have the tests been ran too, to make sure they still run okay and all is expected with this change?

@SuZhou-Joe
Copy link
Member

SuZhou-Joe commented Feb 26, 2024

As 2.12 has been released, I will remove the backport 2.12 label.

@kavilla
Copy link
Member Author

kavilla commented Feb 27, 2024

As 2.12 has been released, I will remove the backport 2.12 label.

In case there is a patch. Do we want to backport to 2.12 since those tests are inaccurate when installing the security plugin.

cy.getElementByTestId('exportAsSavedObject')
.get('.euiRadio__input')
.should('be.disabled');
if (!Cypress.env('SECURITY_ENABLED')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not a fan of this.

We could just strip the tenant from the query string and then compare. In fact, checking if the URL is identical is not a good idea; we should check if the elements we want are there, in whatever order.

@kavilla
Copy link
Member Author

kavilla commented Apr 2, 2024

Closing. Will track with this: opensearch-project/OpenSearch-Dashboards#6220 (comment)

@kavilla kavilla closed this Apr 2, 2024
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.

6 participants