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

Fix role integration tests for mariadb #291

Merged

Conversation

rsicart
Copy link
Contributor

@rsicart rsicart commented Mar 2, 2022

SUMMARY

Fixes #260

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

Integration tests for target test_mysql_role

ADDITIONAL INFORMATION

@codecov
Copy link

codecov bot commented Mar 2, 2022

Codecov Report

Merging #291 (5428dad) into main (c273ee3) will increase coverage by 1.11%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #291      +/-   ##
==========================================
+ Coverage   77.18%   78.29%   +1.11%     
==========================================
  Files          27       27              
  Lines        2244     2244              
  Branches      526      526              
==========================================
+ Hits         1732     1757      +25     
+ Misses        349      331      -18     
+ Partials      163      156       -7     
Impacted Files Coverage Δ
plugins/module_utils/user.py 87.29% <0.00%> (+2.58%) ⬆️
plugins/modules/mysql_role.py 87.94% <0.00%> (+4.56%) ⬆️

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 c273ee3...5428dad. Read the comment docs.

@rsicart
Copy link
Contributor Author

rsicart commented Mar 2, 2022

Hey @Andersson007 @Jorge-Rodriguez @bmalynovytch
I added you as possible reviewers, nothing urgent, feel free to ignore if you do not have time.
Thanks in advance!

@rsicart
Copy link
Contributor Author

rsicart commented Mar 2, 2022

Sorry for the noise, I'll try to fix that before asking your review next time...

@Andersson007
Copy link
Collaborator

@rsicart thank you for this very important fix! Yep, please, when it ready, let us know

@rsicart rsicart force-pushed the rsi/fix-role-tests-for-mariadb branch from d0075a5 to 1ebcfb1 Compare March 2, 2022 09:29
@rsicart
Copy link
Contributor Author

rsicart commented Mar 2, 2022

I don't know why mariadb with mysqlclient doesn't work, so I excluded that from test matrix... Do not hesitate if you know how to fix that properly!
Tests are passing now :)

@Andersson007 @Jorge-Rodriguez @bmalynovytch ready for review

@Andersson007
Copy link
Collaborator

I don't know why mariadb with mysqlclient doesn't work, so I excluded that from test matrix... Do not hesitate if you know how to fix that properly! Tests are passing now :)

@rsicart thanks for fixing the CI! Yep, let's do it incrementally.
When the PR gets merged, could you please update the issue (or create a new one) to commit somewhere that it doesn't work with mysqlclient. Later someone will take a look.

Also let's wait until tomorrow for the folks' feedback. If no objections, we'll merge it tomorrow.

Thank you much!

@Andersson007 Andersson007 merged commit 82cedf8 into ansible-collections:main Mar 4, 2022
@Andersson007
Copy link
Collaborator

@rsicart thank you for contributing to the CI part of the collection, this is extremely important for the collection's stability! Really hard to underestimate.
If you create an issue mysqlclient doesn't work with mariadb with the info that it's disabled in this PR, it would be great, thank you!

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.

Test roles on mariadb
2 participants