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

Add Cypress test for Default Saved Map #552

Merged

Conversation

naveentatikonda
Copy link
Member

Description

Add Cypress test to validate adding default saved map for the sample flights dataset in the dashboards-maps plugin.

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.

@naveentatikonda naveentatikonda requested a review from a team as a code owner February 21, 2023 17:52
Signed-off-by: Naveen Tatikonda <navtat@amazon.com>
@kavilla
Copy link
Member

kavilla commented Feb 21, 2023

Can you do me a favor in seperate PR can you add your test to the test results explorer site for easier debugging in the future:

Copy link
Member

@kavilla kavilla left a comment

Choose a reason for hiding this comment

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

My suggestion as well is add a number before the file to ensure the order.

@naveentatikonda
Copy link
Member Author

naveentatikonda commented Feb 22, 2023

My suggestion as well is add a number before the file to ensure the order.

I believe for dashboards-maps tests order doesn't matter because they are all running independently. But, if you want to follow that naming convention then I will create a separate PR renaming the tests in dashboards-maps

Signed-off-by: Naveen Tatikonda <navtat@amazon.com>
@kavilla
Copy link
Member

kavilla commented Feb 22, 2023

My suggestion as well is add a number before the file to ensure the order.

I believe for dashboards-maps tests order doesn't matter because they are all running independently. But, if you want to follow that naming convention then I will create a separate PR renaming the tests in dashboards-maps

With hindsight, I've seen developers get test failures. For scalability when more developers contribute they might assume that a specific test will always come before another test and write tests in such a fashion (which isn't great but not preventable) but then they add a new file and since cypress will run in alpha-numeric order and failures occur. So adding a number ensures tests run in a specific order.

@naveentatikonda
Copy link
Member Author

naveentatikonda commented Feb 22, 2023

My suggestion as well is add a number before the file to ensure the order.

I believe for dashboards-maps tests order doesn't matter because they are all running independently. But, if you want to follow that naming convention then I will create a separate PR renaming the tests in dashboards-maps

With hindsight, I've seen developers get test failures. For scalability when more developers contribute they might assume that a specific test will always come before another test and write tests in such a fashion (which isn't great but not preventable) but then they add a new file and since cypress will run in alpha-numeric order and failures occur. So adding a number ensures tests run in a specific order.

Got it. Will raise a separate PR renaming all dashboards-maps tests by adding numbers after merging this PR.

@kavilla
Copy link
Member

kavilla commented Feb 22, 2023

forgot to mention you need to add your path to your tests and your plugin id: https://github.com/opensearch-project/opensearch-dashboards-functional-test/blob/main/test_finder.sh#L10

@junqiu-lei
Copy link
Member

forgot to mention you need to add your path to your tests and your plugin id: https://github.com/opensearch-project/opensearch-dashboards-functional-test/blob/main/test_finder.sh#L10

Hi @kavilla, the layered maps App is still under custom-import-map-dashboards plugin. I don't think we need change here. https://github.com/opensearch-project/opensearch-dashboards-functional-test/blob/main/test_finder.sh#L20

@kavilla
Copy link
Member

kavilla commented Feb 22, 2023

forgot to mention you need to add your path to your tests and your plugin id: https://github.com/opensearch-project/opensearch-dashboards-functional-test/blob/main/test_finder.sh#L10

Hi @kavilla, the layered maps App is still under custom-import-map-dashboards plugin. I don't think we need change here. https://github.com/opensearch-project/opensearch-dashboards-functional-test/blob/main/test_finder.sh#L20

whoops missed it! thank you sorry about that

@kavilla
Copy link
Member

kavilla commented Feb 22, 2023

is the goal is to get this to 2.x branch?

@naveentatikonda
Copy link
Member Author

is the goal is to get this to 2.x branch?

Yes, actually to 2.6

@CCongWang CCongWang merged commit 61766e7 into opensearch-project:main Apr 19, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 19, 2023
* Add Cypress test for Default Saved Map

Signed-off-by: Naveen Tatikonda <navtat@amazon.com>

* Address Review Comments

Signed-off-by: Naveen Tatikonda <navtat@amazon.com>

---------

Signed-off-by: Naveen Tatikonda <navtat@amazon.com>
(cherry picked from commit 61766e7)
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 19, 2023
* Add Cypress test for Default Saved Map

Signed-off-by: Naveen Tatikonda <navtat@amazon.com>

* Address Review Comments

Signed-off-by: Naveen Tatikonda <navtat@amazon.com>

---------

Signed-off-by: Naveen Tatikonda <navtat@amazon.com>
(cherry picked from commit 61766e7)
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 19, 2023
* Add Cypress test for Default Saved Map

Signed-off-by: Naveen Tatikonda <navtat@amazon.com>

* Address Review Comments

Signed-off-by: Naveen Tatikonda <navtat@amazon.com>

---------

Signed-off-by: Naveen Tatikonda <navtat@amazon.com>
(cherry picked from commit 61766e7)
kavilla pushed a commit that referenced this pull request Apr 19, 2023
* Add Cypress test for Default Saved Map

Signed-off-by: Naveen Tatikonda <navtat@amazon.com>

* Address Review Comments

Signed-off-by: Naveen Tatikonda <navtat@amazon.com>

---------

Signed-off-by: Naveen Tatikonda <navtat@amazon.com>
(cherry picked from commit 61766e7)

Co-authored-by: Naveen Tatikonda <navtat@amazon.com>
CCongWang pushed a commit that referenced this pull request Apr 19, 2023
* Add Cypress test for Default Saved Map

Signed-off-by: Naveen Tatikonda <navtat@amazon.com>

* Address Review Comments

Signed-off-by: Naveen Tatikonda <navtat@amazon.com>

---------

Signed-off-by: Naveen Tatikonda <navtat@amazon.com>
(cherry picked from commit 61766e7)

Co-authored-by: Naveen Tatikonda <navtat@amazon.com>
CCongWang pushed a commit that referenced this pull request Apr 19, 2023
* Add Cypress test for Default Saved Map

Signed-off-by: Naveen Tatikonda <navtat@amazon.com>

* Address Review Comments

Signed-off-by: Naveen Tatikonda <navtat@amazon.com>

---------

Signed-off-by: Naveen Tatikonda <navtat@amazon.com>
(cherry picked from commit 61766e7)

Co-authored-by: Naveen Tatikonda <navtat@amazon.com>
leanneeliatra pushed a commit to leanneeliatra/opensearch-dashboards-functional-test-fork that referenced this pull request Sep 15, 2023
…search-project#632)

* Add Cypress test for Default Saved Map

Signed-off-by: Naveen Tatikonda <navtat@amazon.com>

* Address Review Comments

Signed-off-by: Naveen Tatikonda <navtat@amazon.com>

---------

Signed-off-by: Naveen Tatikonda <navtat@amazon.com>
(cherry picked from commit 61766e7)

Co-authored-by: Naveen Tatikonda <navtat@amazon.com>
Signed-off-by: leanne.laceybyrne@eliatra.com <leanne.laceybyrne@eliatra.com>
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