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

ci: add test for multiple os #68

Merged
merged 2 commits into from
May 8, 2022
Merged

Conversation

greenhandatsjtu
Copy link
Contributor

Test project on multiple operating systems via matrix.

ref: casbin/casbin-rs#234
credit to: https://github.com/diesel-rs/diesel/blob/0c6219f5bd9641eea09c6fa7dca4995aefda30a0/.github/workflows/ci.yml

Note: replace 127.0.0.1 with localhost in DATABASE_URL as a workaround for this this issue: launchbadge/sqlx#846
otherwise, build with feature runtime-xxx-rustls would fail with errors like:

error: error occurred while attempting to establish a TLS connection: InvalidDNSNameError
Error:   --> src/actions.rs:29:5
   |
29 | /     sqlx::query!(
30 | |         "CREATE TABLE IF NOT EXISTS casbin_rule (
31 | |                     id SERIAL PRIMARY KEY,
32 | |                     ptype VARCHAR NOT NULL,
...  |
41 | |         "
42 | |     )
   | |_____^
   |
   = note: this error originates in the macro `$crate::sqlx_macros::expand_query` (in Nightly builds, run with -Z macro-backtrace for more info)

@casbin-bot
Copy link

@smrpn @hackerchai @PsiACE @GopherJ please review

@codecov-commenter
Copy link

codecov-commenter commented May 6, 2022

Codecov Report

Merging #68 (0c86424) into master (cc2a690) will decrease coverage by 10.22%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           master      #68       +/-   ##
===========================================
- Coverage   76.03%   65.80%   -10.23%     
===========================================
  Files           3        3               
  Lines         363      427       +64     
===========================================
+ Hits          276      281        +5     
- Misses         87      146       +59     
Impacted Files Coverage Δ
src/adapter.rs 88.38% <100.00%> (+0.08%) ⬆️
src/actions.rs 47.53% <0.00%> (-18.34%) ⬇️
src/error.rs 0.00% <0.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 cc2a690...0c86424. Read the comment docs.

@hsluoyz
Copy link
Member

hsluoyz commented May 6, 2022

@PsiACE @hackerchai

@hackerchai
Copy link
Member

@greenhandatsjtu Can we use docker to replace these platform-specific configuration. You can refer to SQLx's solution. And here is the SQLx Github Actions Conf, we don't need multiple versions of database backend but MySQL, PosgreSQL , SQLite is needed.

@greenhandatsjtu
Copy link
Contributor Author

greenhandatsjtu commented May 6, 2022

Can we use docker to replace these platform-specific configuration

Hi, @hackerchai , currently, docker container is only available on Linux (https://docs.github.com/en/actions/using-containerized-services/about-service-containers#about-service-containers):

If your workflows use Docker container actions, job containers, or service containers, then you must use a Linux runner:

As a result, to support tests for windows and macOS, I have to install databases instead of using containers.
SQLx only runs tests on Ubuntu, so using containers is fine. You can refer to Diesel's CI workflow conf, they run tests on multiple os, so they don't use containers too.

@hackerchai
Copy link
Member

Can we use docker to replace these platform-specific configuration

Hi, @hackerchai , currently, docker container is only available on Linux (https://docs.github.com/en/actions/using-containerized-services/about-service-containers#about-service-containers):

If your workflows use Docker container actions, job containers, or service containers, then you must use a Linux runner:

As a result, to support tests for windows and macOS, I have to install databases instead of using containers. SQLx only runs tests on Ubuntu, so using containers is fine. You can refer to Diesel's CI workflow conf, they run tests on multiple os, so they don't use containers too.

@greenhandatsjtu Thanks for the explanation, you have done good job, thank you for the PR.

@hackerchai hackerchai merged commit 57a8b97 into casbin-rs:master May 8, 2022
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.

5 participants