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

GitHub Actions: Add macOS cases. #1170

Merged
merged 4 commits into from
Mar 1, 2021
Merged

Conversation

junaruga
Copy link
Contributor

@junaruga junaruga commented Feb 24, 2021

  • Migrate macOS cases in Travis to GitHub Actions.

  • Increase the expected time in the test to avoid the following random failures
    on macOS.

    1) Mysql2::Client#query threaded queries should be supported
       Failure/Error: expect(stop - start).to be_within(0.1).of(sleep_time)
         expected 0.632204999999999 to be within 0.1 of 0.5
       # ./spec/mysql2/client_spec.rb:711:in `block (3 levels) in <top (required)>'
    
    3) Mysql2::Client#query should run signal handlers while waiting for a response
       Failure/Error: expect(mark.fetch(:QUERY_END) - mark.fetch(:QUERY_START)).to be_within(0.1).of(query_time)
         expected 1.1042130000000157 to be within 0.1 of 1.0
       # ./spec/mysql2/client_spec.rb:632:in `block (3 levels) in <top (required)>'
    

Notes

  • I renamed .github/workflows/ubuntu.yml to build.yml again, after adding the macOS cases to the file. I considered the name ubuntu_macos.yml. But thinking about we want to keep a same name for a long time, and considering the name (name: Build) is used as a build badge name when we will add it later. The name Build makes sense to me.

  • In Travis macOS case, the brew package mariadb-connector-c was installed. So I always installed the mariadb-connector-c in GitHub Actions macOS case too. However when we do not actually install it on the db: '[email protected]' case of the macOS, rake compile can be compiled, and there are some test failures. In db: '[email protected]' case, rake compile shows the following library missing error when mariadb-connector-c is not installed.

    checking for -lmysqlclient... no
    mysql client is missing. You may need to 'brew install mysql' or 'port install mysql', and try again.
    

    We might be able to install mariadb-connector-c conditionally on macOS case like this.

    brew install "${DB}"
    if [[ -n ${DB-} && x$DB =~ ^xmariadb ]]; then
      brew install mariadb-connector-c
    fi
    


# Install the default used DB if DB is not set.
if [[ -n ${GITHUB_ACTION-} && -z ${DB-} ]]; then
if [[ -n ${GITHUB_ACTIONS-} && -z ${DB-} ]]; then
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# where ALTER USER is not available.
# https://stackoverflow.com/questions/56052177/
CHANGED_PWD_BY_RECREATE=false
CHANGED_PASSWORD_BY_RECREATE=false
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • I renamed CHANGED_PWD to CHANGED_PASSWORD, because PWD makes me imagine pwd (print working directory) command. Sorry it was my typo.

fi

# IF NOT EXISTS is mariadb-10+ only - https://mariadb.com/kb/en/mariadb/comment-syntax/
mysql -u root -e 'CREATE DATABASE /*M!50701 IF NOT EXISTS */ test'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new macOS db: '[email protected]' case showed the error when running mysql -u root -e 'CREATE DATABASE /*M!50701 IF NOT EXISTS */ test', because it was recognized as mysql -u root -e 'CREATE DATABASE test' and there was test database already. As every used database supports IF NOT EXISTS in CIs, I think we can remove the /*M!50701 ... */.

.github/workflows/build.yml Outdated Show resolved Hide resolved
@junaruga
Copy link
Contributor Author

The CIs are okay for both GitHub Actions and Travis CI. I noticed the "Checks" tab showing the result of GitHub Actions. It looks convenient.

@junaruga junaruga mentioned this pull request Feb 24, 2021
@junaruga
Copy link
Contributor Author

I added one more commit for the review. Now MariaDB 10.5 (10.5.9) and MySQL 8.0 (8.0.23) are set on the macOS cases. I removed the mariadb-connector-c brew package of installing, because it was conflicted with the mariadb brew package. And both latest MariaDB and MySQL works without the mariadb-connector-c.

@junaruga
Copy link
Contributor Author

I did split the previous 2 commits to 4 commits, splitting the previous 1st commit about migrating macOS and renaming variables. I think it's better to see the commit about migrating macOS. The CIs looks okay. (Travis log on my forked repository).

GITHUB_ACTIONS showing always true on GitHub Actions environment is
a right environment variable
See https://docs.github.com/en/actions/reference/environment-variables .
It's better to know the meaning.
* Migrate macOS cases in Travis to GitHub Actions.

* Increase the expected time in the test to avoid the following random failures
  on macOS.

  ```
  1) Mysql2::Client#query threaded queries should be supported
     Failure/Error: expect(stop - start).to be_within(0.1).of(sleep_time)
       expected 0.632204999999999 to be within 0.1 of 0.5
     # ./spec/mysql2/client_spec.rb:711:in `block (3 levels) in <top (required)>'
  ```

  ```
  3) Mysql2::Client#query should run signal handlers while waiting for a response
     Failure/Error: expect(mark.fetch(:QUERY_END) - mark.fetch(:QUERY_START)).to be_within(0.1).of(query_time)
       expected 1.1042130000000157 to be within 0.1 of 1.0
     # ./spec/mysql2/client_spec.rb:632:in `block (3 levels) in <top (required)>'
  ```
@sodabrew
Copy link
Collaborator

sodabrew commented Mar 1, 2021

Looks good!

@sodabrew sodabrew merged commit 6e194e9 into brianmario:master Mar 1, 2021
@junaruga junaruga deleted the wip/ci-macos branch March 2, 2021 04:50
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.

2 participants