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 unix socket tests #686

Closed
wants to merge 1 commit into from

Conversation

Nothing4You
Copy link
Collaborator

@Nothing4You Nothing4You commented Jan 22, 2022

What do these changes do?

Run the entire test suite against unix sockets as well as over TCP.
This effectively runs every test twice against every DB version, once over unix socket, once over TCP.

This also implements passing arbitrary connection info to the test suite, allowing to run tests against multiple databases in the same test run.
It can later be expanded to run more databases in the same test run, which should allow us to launch containers for all databases in the same CI job (assuming CI VM resource limits permit that).
For that to be implemented however, the current workaround of passing DB versions via env var needs to be replaced.

Due to doing even more changes to the service containers with this it may be worth to just not use service containers at all in the future, launching custom containers by ourselves at an early step.
That may also benefit testing on other OS, as service containers are currently only supported on ubuntu.

Are there changes in behavior for the user?

No.

Related issue number

Fixes #664

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • Add a new news fragment to CHANGES.txt

@Nothing4You Nothing4You added this to the 1.0.0 milestone Jan 22, 2022
@codecov
Copy link

codecov bot commented Jan 22, 2022

Codecov Report

Merging #686 (1dbd5e9) into master (8fe7e53) will decrease coverage by 1.91%.
The diff coverage is 62.50%.

❗ Current head 1dbd5e9 differs from pull request most recent head 549dd46. Consider uploading reports for the commit 549dd46 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #686      +/-   ##
==========================================
- Coverage   85.68%   83.76%   -1.92%     
==========================================
  Files          12       12              
  Lines        2088     2088              
  Branches      336      336              
==========================================
- Hits         1789     1749      -40     
- Misses        229      271      +42     
+ Partials       70       68       -2     
Flag Coverage Δ
ubuntu-latest_3.7_mariadb-10.2 82.50% <62.50%> (+0.58%) ⬆️
ubuntu-latest_3.7_mariadb-10.3 82.50% <62.50%> (+0.58%) ⬆️
ubuntu-latest_3.7_mariadb-10.4 82.50% <62.50%> (+0.58%) ⬆️
ubuntu-latest_3.7_mariadb-10.5 82.50% <62.50%> (+0.58%) ⬆️
ubuntu-latest_3.7_mariadb-10.6 82.50% <62.50%> (+0.58%) ⬆️
ubuntu-latest_3.7_mariadb-10.7 82.50% <62.50%> (+0.58%) ⬆️
ubuntu-latest_3.7_mysql-5.7 83.38% <62.50%> (+0.58%) ⬆️
ubuntu-latest_3.7_mysql-8.0 ?
ubuntu-latest_3.8_mariadb-10.2 82.90% <62.50%> (+0.57%) ⬆️
ubuntu-latest_3.8_mariadb-10.3 82.90% <62.50%> (+0.57%) ⬆️
ubuntu-latest_3.8_mariadb-10.4 82.90% <62.50%> (+0.57%) ⬆️
ubuntu-latest_3.8_mariadb-10.5 82.90% <62.50%> (+0.57%) ⬆️
ubuntu-latest_3.8_mariadb-10.6 82.90% <62.50%> (+0.57%) ⬆️
ubuntu-latest_3.8_mariadb-10.7 82.90% <62.50%> (+0.57%) ⬆️
ubuntu-latest_3.8_mysql-5.7 83.76% <62.50%> (+0.57%) ⬆️
ubuntu-latest_3.8_mysql-8.0 ?
ubuntu-latest_3.9_mariadb-10.2 82.90% <62.50%> (+0.57%) ⬆️
ubuntu-latest_3.9_mariadb-10.3 82.90% <62.50%> (+0.57%) ⬆️
ubuntu-latest_3.9_mariadb-10.4 82.90% <62.50%> (?)
ubuntu-latest_3.9_mariadb-10.5 82.90% <62.50%> (+0.57%) ⬆️
ubuntu-latest_3.9_mariadb-10.6 82.90% <62.50%> (+0.57%) ⬆️
ubuntu-latest_3.9_mariadb-10.7 82.90% <62.50%> (+0.57%) ⬆️
ubuntu-latest_3.9_mysql-5.7 83.76% <62.50%> (+0.57%) ⬆️
ubuntu-latest_3.9_mysql-8.0 ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
aiomysql/connection.py 78.31% <62.50%> (-4.39%) ⬇️
aiomysql/pool.py 95.18% <0.00%> (-1.81%) ⬇️

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 8fe7e53...549dd46. Read the comment docs.

configure custom socket path for mysql container, working around implicitly created volume folders being owned by root
we should probably just not use service containers for this to avoid having to do this patching
@Nothing4You Nothing4You marked this pull request as ready for review January 28, 2022 19:24
@Nothing4You
Copy link
Collaborator Author

somehow codecov broke...
continuing in a new pr

@Nothing4You
Copy link
Collaborator Author

replaced by #696

@Nothing4You Nothing4You removed this from the 1.0 milestone Jan 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add tests for unix socket connections
1 participant