search

ossc-db / pg_hint_plan

Extension adding support for optimizer hints in PostgreSQL
Other
696 stars 103 forks source link

Add test by GithubActions #104

Closed SeinoYuki closed 1 year ago

SeinoYuki commented 2 years ago

PullRequest "Add Test by GithubActions". Please check GithubActions in my repository. https://github.com/SeinoYuki/pg_hint_plan/actions

This test supports CentOS7,UBI8 and UBI9. In the regression test, locale ja_JP is used, but will be changed to locale C as it is more convenient.

This request is for master, but we would like to support PG10~PG13 branches as well.

MasahikoSawada commented 1 year ago

Can we support pg14 and pg15 branches as well?

michaelpq commented 1 year ago

Can we support pg14 and pg15 branches as well?

I agree that this should go down to all the branches we intend to have releases for, dropping branches where we drop their support. So +1.

MasahikoSawada commented 1 year ago

FYI I've tried ci test using github actions in my forked repository. It installs PG server and run make installcheck depending on the branches. I has ci test for the master branch but disabled it for now.

https://github.com/MasahikoSawada/pg_hint_plan/blob/master/.github/workflows/test.yml

michaelpq commented 1 year ago

I has ci test for the master branch but disabled it for now.

Disabling that on master for now is fine by me. it does not seem worth the complication of having our master branch plug into upstream REL_15_STABLE until we don't have PG15 created. Let's get the basics tackled first.

michaelpq commented 1 year ago 
    branches:
      - PG14
      - PG13
      - PG12
      - PG11
      - PG10

@MasahikoSawada I have a question about defining multiple branches in this script. Is this aimed to be pushed only to the master branch or is this something that will be back-patched down to 10?

We should avoid useless tests, which means that we should either test all the branches on master or have each branch test only its own. https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions. In terms of portability of the tests, would it be more readable to just have one script for its own branch? That would avoid if/else branches depending on the version involved, at least, and we have already one stable branch for each PG version in this project..

MasahikoSawada commented 1 year ago

This (PoC) script is aimed to push all the branches except for master. The test will be triggered depending on the branches where a commit is pushed or a pull request is created. For example, if a pull request is created on PG14 branch, it tests it only with PG14 branch and PostgreSQL 14.

For the readability, yes, it would be better to have one script for its own branch. I thought that having common one script for all branches would make back-patching easy.

michaelpq commented 1 year ago

For the readability, yes, it would be better to have one script for its own branch. I thought that having common one script for all branches would make back-patching easy.

Okay. Once you are confident enough with the change, could you take care of it?

MasahikoSawada commented 1 year ago

Sure.

MasahikoSawada commented 1 year ago

Pushed.