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

Enable cypress test for windows and mac platform #91

Merged
merged 1 commit into from
Nov 23, 2022

Conversation

heemin32
Copy link
Collaborator

Signed-off-by: Heemin Kim [email protected]

Description

Enable cypress test for windows and mac platform

Issues Resolved

#55

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.

@heemin32 heemin32 requested a review from a team November 22, 2022 19:58
@heemin32 heemin32 force-pushed the cypress branch 2 times, most recently from ca860a6 to 13a78fb Compare November 22, 2022 20:04
Copy link
Collaborator

@navneet1v navneet1v left a comment

Choose a reason for hiding this comment

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

I can see the failing GH actions. Please fix that.

@codecov-commenter
Copy link

codecov-commenter commented Nov 22, 2022

Codecov Report

Merging #91 (4be60b5) into 2.x (7e33614) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##              2.x      #91   +/-   ##
=======================================
  Coverage   85.03%   85.03%           
=======================================
  Files           5        5           
  Lines         147      147           
  Branches       24       24           
=======================================
  Hits          125      125           
  Misses         16       16           
  Partials        6        6           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@heemin32 heemin32 force-pushed the cypress branch 18 times, most recently from cf50bf4 to 1033c22 Compare November 23, 2022 20:47
@heemin32
Copy link
Collaborator Author

I had to increase timeout values and reduce sample_geojson.json file so that cypress tests in windows to pass.
Below screenshot shows timeout when the cypress test adds sample_geojson.json file.
Screen Shot 2022-11-23 at 1 38 42 PM

Copy link
Member

@junqiu-lei junqiu-lei left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! I think main branch also need the change?

@heemin32
Copy link
Collaborator Author

LGTM, thanks! I think main branch also need the change?

Yes. Will forward port it once it is approved.

- name: Run Cypress tests
uses: cypress-io/github-action@v2
with:
working-directory: OpenSearch-Dashboards/plugins/custom_import_map
command: yarn run cypress run
command: yarn run cypress run --browser chrome
Copy link
Collaborator

Choose a reason for hiding this comment

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

why we are sticking to chrome here? Should we test on multiple browsers? Does OSD has some recommendation?

Copy link
Collaborator Author

@heemin32 heemin32 Nov 23, 2022

Choose a reason for hiding this comment

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

OSD uses chrome as default. https://github.com/opensearch-project/opensearch-dashboards-functional-test/blob/main/.github/workflows/cypress-workflow-bundle-snapshot-based-windows.yml#L31

Haven't checked every plugins but I know some plugins also have chrome as a default browser.

@navneet1v
Copy link
Collaborator

Please forward port this change to the main branch.

@heemin32 heemin32 merged commit ef85fb7 into opensearch-project:2.x Nov 23, 2022
@heemin32 heemin32 deleted the cypress branch November 23, 2022 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants