Skip to content

HDDS-7044. Ignore pr_title_check for selective checks#3620

Merged
adoroszlai merged 1 commit intoapache:masterfrom
adoroszlai:HDDS-7044
Aug 3, 2022
Merged

HDDS-7044. Ignore pr_title_check for selective checks#3620
adoroszlai merged 1 commit intoapache:masterfrom
adoroszlai:HDDS-7044

Conversation

@adoroszlai
Copy link
Contributor

@adoroszlai adoroszlai commented Jul 24, 2022

What changes were proposed in this pull request?

Avoid triggering all tests:

  • for workflows except post-commit.yml: they are not exercised by any jobs in the post-commit.yml (build-branch) workflow, so running those jobs is useless
  • for pr_title_check.*: these are covered by specific checks (rat for license, and bats for bash unit tests)

https://issues.apache.org/jira/browse/HDDS-7044

How was this patch tested?

Added test case in selective_ci_checks.bats. Also checked output for the commit (96009ab) for HDDS-7043 locally:

$ bash dev-support/ci/selective_ci_checks.sh 96009ab20
...

All 2 changed files are known to be handled by specific checks.

needs-basic-checks=true
basic-checks=["rat","bats"]
needs-build=false
needs-compile=false
needs-compose-tests=false
needs-dependency-check=false
needs-integration-tests=false
needs-kubernetes-tests=false

and for a commit (1493541) that only changed the misc. workflows:

$ bash dev-support/ci/selective_ci_checks.sh 1493541d7
...

All 2 changed files are known to be handled by specific checks.

needs-basic-checks=true
basic-checks=["rat"]
needs-build=false
needs-compile=false
needs-compose-tests=false
needs-dependency-check=false
needs-integration-tests=false
needs-kubernetes-tests=false

Regular CI:
https://github.com/adoroszlai/hadoop-ozone/runs/7486998639
https://github.com/adoroszlai/hadoop-ozone/runs/7487000326

@adoroszlai adoroszlai self-assigned this Jul 24, 2022
@adoroszlai adoroszlai requested a review from smengcl July 26, 2022 14:57
@kerneltime
Copy link
Contributor

@adoroszlai I will need to sync up with you to understand this change.

Copy link
Contributor

@smengcl smengcl left a comment

Choose a reason for hiding this comment

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

Thanks @adoroszlai for the improvement.

IIUC the idea is to ignore the title checker (which is added in 4f0bd4a not so long ago) for changes that doesn't really involve a PR title change.

@adoroszlai
Copy link
Contributor Author

@kerneltime @smengcl Thanks for taking a look. Let me try to give some background for the change.

  1. We have different Github Actions workflows under .github/workflows:
    • regular build/test CI is defined in post-commit.yml, triggered by pushes, PRs and on schedule
    • PR titles are checked by pull-request.yml
    • special PR comments (example) are handled by comments.yaml
    • pending PRs are closed by close-pending.yaml on schedule
  2. Regular CI (post-commit.yml) has some logic to decide whether to execute or skip specific checks (e.g. checkstyle, unit, acceptance, etc.) depending on which files were changed. This is implemented in selective_ci_checks.sh, which is run as the first job. It defines include and exclude regex patterns for each kind of check. These patterns are matched against the list of changed files. The script outputs true/false for each check, which is then picked up by the rest of the post-commit.yml workflow to skip/run other jobs.
  3. There is also a "must run all checks" case, triggered by either of two conditions:
    • if the change affects CI files, e.g. the workflow definition itself
    • if some changed files are not handled by any of the check-specific patterns
  4. We also run all checks for pushes and schedule, in other words, selective checks only applies to pull requests. (Example: compare workflow runs for the same change when pushed to developer's fork, pull request is created, and finally merged to master.)

Consider the case that I would like to change one of the other (non-post-commit.yml) workflow files, e.g. pull-request.yml. When I open a PR, this kind of change currently triggers all checks in the post-commit.yml CI workflow. Yet, most of the checks in post-commit.yml have nothing to do with pull-request.yml. None of the unit, integration or acceptance tests are affected. Checkstyle and findbugs (Java-specific checks) are not applicable.

So this PR tweaks some of the include/exclude patterns to be more specific.

Copy link
Contributor

@smengcl smengcl left a comment

Choose a reason for hiding this comment

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

Thanks @adoroszlai for the detailed explanation. The change looks good to me.

@adoroszlai adoroszlai merged commit 2396611 into apache:master Aug 3, 2022
@adoroszlai
Copy link
Contributor Author

Thanks @smengcl for the review.

@adoroszlai adoroszlai deleted the HDDS-7044 branch August 3, 2022 09:46
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.

3 participants

Comments