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

adding cancel button to change policy #919

Conversation

kohinoor98
Copy link
Contributor

@kohinoor98 kohinoor98 commented Nov 7, 2023

Description

Added Cancel button to ChangePolicy.tsx

Issues Resolved

#840

Check List

  • Commits are signed per the DCO using --signoff
  • Test the Cancel button in Change Policy page

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.

@AWSHurneyt
Copy link
Contributor

@kohinoor98 looks like some snapshot unit tests failed. Could you run yarn run test:jest -u? That will update the snapshots, and should allow the unit tests to pass.

@kohinoor98
Copy link
Contributor Author

kohinoor98 commented Nov 8, 2023

Hi @AWSHurneyt

The current test case that is failing is addressed in this PR #918. The other test case that is failing should be handled by the changes made to ChangePolicy.test.tsx.snap.

Please advice on next steps.

Thanks,
KC

@AWSHurneyt
Copy link
Contributor

@kohinoor98 Looks like there's still a unit test failing in the ChangePolicy.test.tsx.
https://github.com/opensearch-project/index-management-dashboards-plugin/actions/runs/6805289930/job/18504495378?pr=919#step:10:860

I see in the commits tab for this PR that changes from the main branch were merged into the dev branch for this PR after the snapshot was updated. Could you try running yarn run test:jest -u again? If that doesn't resolve the failing test, it may need deeper investigation.

Copy link

codecov bot commented Nov 10, 2023

Codecov Report

Merging #919 (96b5c0b) into main (461f563) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #919   +/-   ##
=======================================
  Coverage   63.36%   63.37%           
=======================================
  Files         341      341           
  Lines       11553    11554    +1     
  Branches     2243     2243           
=======================================
+ Hits         7321     7322    +1     
  Misses       3658     3658           
  Partials      574      574           

see 1 file with indirect coverage changes

@kohinoor98
Copy link
Contributor Author

kohinoor98 commented Nov 11, 2023

Hi @AWSHurneyt

I have updated the PR with the latest code from yarn run test:jest -u for the cancel button.

I tested snapshots_spec.js on cypress and it kept failing in one to three of the test cases for some reason or the other over 10 runs.

I am guessing @bowenlan-amzn noticed the same and created #925 for the same.

Thanks,
KC

@AWSHurneyt AWSHurneyt merged commit 1c8f0c6 into opensearch-project:main Nov 11, 2023
9 of 10 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Nov 11, 2023
* adding cancel button to change policy

Signed-off-by: kohinoor98 <[email protected]>

* updated ChangePolicy snapshot to incl Cancel's div

Signed-off-by: kohinoor98 <[email protected]>

* auto fixing the ChangePolicy.test snapshot

Signed-off-by: kohinoor98 <[email protected]>

---------

Signed-off-by: kohinoor98 <[email protected]>
Co-authored-by: bowenlan-amzn <[email protected]>
(cherry picked from commit 1c8f0c6)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
bowenlan-amzn added a commit that referenced this pull request Nov 11, 2023
* adding cancel button to change policy



* updated ChangePolicy snapshot to incl Cancel's div



* auto fixing the ChangePolicy.test snapshot



---------



(cherry picked from commit 1c8f0c6)

Signed-off-by: kohinoor98 <[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: bowenlan-amzn <[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.

3 participants