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.

Sign up for GitHub

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

Jump to bottom

Created tests for disable rule modal #111

Merged
merged 5 commits into from
Apr 12, 2022
Merged

Created tests for disable rule modal #111

merged 5 commits into from
Apr 12, 2022

Conversation

ikerreyes
Copy link
Collaborator

Add test for the rule disabling modal that appears in the AffectedClustersTable

@ikerreyes
Copy link
Collaborator Author

I need some help to write spy to capture function calls to disableRuleForCluster and useSetAckMutation or alternatively intercept network requests to validate them. If we can make both that would be better, because the spy is probably faster and simpler but not suitable for all scenarios, and the second one can help with other issues (e.g. that code is triggered when you press cancel button)

@ikerreyes ikerreyes self-assigned this Feb 14, 2022
@gkarat
Copy link
Collaborator

gkarat commented Feb 26, 2022

We can help you with these scenarios (after GA, most probably). I think we also can take advantage of cy.wait: https://docs.cypress.io/guides/guides/network-requests#Waiting. You will just wait for the certain requests to come. Once they come, the test should proceede, otherwise fail.

Also, asserting JS functions or capturing React component methods calls can be also done via Jest (that we have integrated in the project). I will try to compare both approaches and complete the PR (or assign Alex to it).

@gkarat gkarat added the help wanted Extra attention is needed label Feb 28, 2022
@gkarat gkarat self-assigned this Mar 16, 2022
@gkarat gkarat mentioned this pull request Mar 23, 2022
Merged
gkarat added 2 commits March 23, 2022 16:19
@gkarat
fix(disable modal): bug with multiple selections
6f8acc6 
This fixes a bug: if user selects multiple clusters, unchecks
"Disable recommendation for selected clusters" flag, then the
recommendation will only be disabled for selected clusters. This is incorrect, and
the recommendation should be disabled for all clusters in this scenario.
@gkarat
Intercept network requests and check request body
149a5a0
@gkarat gkarat added enhancement New feature or request and removed help wanted Extra attention is needed labels Mar 23, 2022
@codecov-commenter
Copy link

codecov-commenter commented Mar 23, 2022 •
edited
Loading

Codecov Report

Merging #111 (ab911e5) into master (688857b) will increase coverage by 4.22%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #111      +/-   ##
==========================================
+ Coverage   76.58%   80.81%   +4.22%     
==========================================
  Files          27       27              
  Lines         897      938      +41     
  Branches      322      343      +21     
==========================================
+ Hits          687      758      +71     
+ Misses        210      180      -30
Impacted Files Coverage Δ
src/Components/Modals/DisableRule.js 94.44% <100.00%> (+59.82%) ⬆️
.../Components/ClustersListTable/ClustersListTable.js 75.86% <0.00%> (-2.97%) ⬇️
src/Components/ClusterHeader/ClusterHeader.js 100.00% <0.00%> (ø)
...nts/AffectedClustersTable/AffectedClustersTable.js 100.00% <0.00%> (ø)
src/Components/Common/Tables.js 85.21% <0.00%> (+1.21%) ⬆️
src/Components/RecsListTable/RecsListTable.js 74.61% <0.00%> (+1.24%) ⬆️
src/Utilities/Api.js 80.00% <0.00%> (+20.00%) ⬆️
src/Services/Acks.js 80.00% <0.00%> (+30.00%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 688857b...ab911e5. Read the comment docs.

@gkarat gkarat added tests and removed enhancement New feature or request labels Mar 29, 2022
@ikerreyes ikerreyes marked this pull request as ready for review April 12, 2022 09:26
@gkarat gkarat merged commit 991bceb into RedHatInsights:master Apr 12, 2022
@gkarat
Copy link
Collaborator

gkarat commented Apr 12, 2022

🎉 This PR is included in version 1.3.6 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gkarat gkarat gkarat approved these changes

Assignees

@ikerreyes ikerreyes

@gkarat gkarat

Labels
released tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ikerreyes @gkarat @codecov-commenter