fkirc / skip-duplicate-actions

Save time and cost when using GitHub Actions
Other
475 stars 40 forks source link

Skip using paths_filter: pre job logs it will skip target jobs but target jobs still run #326

Open jkarstens opened 1 year ago

jkarstens commented 1 year ago

I am following Example 3 from the README, but I am struggling to get jobs actually skipped. I am in a monorepo and want to do exactly as the documented example suggests: only run frontend checks when frontend files are updated and only run backend checks when backend files are updated. In my GitHub Branches settings I have the three status checks (pre, api, and ui) marked as required.

I put up a test PR that contained a change to a file in neither the frontend nor the backend, so I expected both checks to be skipped and the status checks to pass. In the Actions log, the pre job is logging that it is going to skip both checks, as expected. However, I can see from the Action logs that both steps do in fact continue to run - why is this? How can I skip them? pre job log: image api job log: image ui job log: image

The workflow file:

name: Format, compile, and lint
on:
  pull_request:
    branches:
      - main
  push:
    branches:
      - main

jobs:
  pre:
    runs-on: ubuntu-latest
    outputs:
      should_skip: ${{ steps.skip_check.outputs.should_skip }}
      paths_result: ${{ steps.skip_check.outputs.paths_result }}
    steps:
      - id: skip_check
        uses: fkirc/skip-duplicate-actions@v5
        with:
          paths_filter: |
            api:
              paths_ignore:
                - 'api/api/migrations/**'
              paths:
                - 'api/**/*.py'
            ui:
              paths:
                - 'ui/src/**/*.[jt]sx?'

  api:
    needs: pre
    if: needs.pre.outputs.should_skip != 'true' || !fromJSON(needs.pre.outputs.paths_result).api.should_skip
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v3
      - uses: psf/black@stable
        with:
          options: '--check --color --diff --extend-exclude api/api/migrations --verbose'
          src: './api'
          version: '22.12.0'
      - uses: actions/setup-python@v4
        with:
          python-version: '3.8.13'
      - uses: py-actions/flake8@v2
        with:
          args: '--extend-ignore=E203'
          exclude: 'api/api/migrations'
          flake8-version: '6.0.0'
          max-line-length: '88'

  ui:
    needs: pre
    if: needs.pre.outputs.should_skip != 'true' || !fromJSON(needs.pre.outputs.paths_result).ui.should_skip
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v3
      - uses: actions/setup-node@v3
        with:
          node-version: 18
      - name: Compile, lint, and format
        run: |
          cd ui
          yarn
          yarn generate-env-file
          yarn tsc
          npx eslint .
          yarn prettier .

I tried removing the global should_skip check from the api and ui jobs, but then no checks at all ran (not even the pre check) which put me back to the original problem.

q0w commented 1 year ago

Any news?

I get global should_skip = true and !fromJSON(needs.pre.outputs.paths_result).api.should_skip = true for some reason. So api job is never skipped

fkirc commented 12 months ago

I apologize for the late answer. For future readers of this issue:

If you run into problems with the paths_filter option, then I would recommend to copy the example 1 multiple times according to your needs: https://github.com/fkirc/skip-duplicate-actions#example-1-skip-entire-jobs

With the example 1, there is no need for any JSON-parsing, instead you can check the global should_skip output. Normally, the example 1 can be copied and adapted for every sub-job where a skip-behavior is needed.

fkirc commented 11 months ago

I am closing this issue because of PR https://github.com/fkirc/skip-duplicate-actions/pull/336/files , please re-open if needed

q0w commented 11 months ago

@fkirc but is it a bug or expected behaviour?

paescuj commented 11 months ago

@fkirc but is it a bug or expected behaviour?

Happy to take a look at it if you can provide some logs πŸ‘

q0w commented 11 months ago

@fkirc but is it a bug or expected behaviour?

Happy to take a look at it if you can provide some logs πŸ‘

I had the exact same issue as above. Now I'm using 2 steps without paths_filter as a workaround.

fkirc commented 11 months ago

Letβ€˜ reopen the issue because my proposed solution seems to be only a workaround

jrson83 commented 5 months ago

@fkirc after a lot of testing, I can confirm that the paths_filter option is working great without a workaround.

You should consider the following very relevant statement from the docs when developing a workflow with conditions:

if a merge yields a result that is different from the source branch, then the resulting workflow run will not be skipped.

Since I was not aware of this, I got different results than expected when testing the workflow, e.g. if a job failed in an action-run, the job was executed again in the following run. So I was wondering why it did not work as expected. @jkarstens your issue sounds very similar to me.


What I noticed, that there are two issues with the docs:

Workflow syntax

Example 3 does not use expression syntax:

if: needs.pre_job.outputs.should_skip != 'true' && !fromJSON(needs.pre_job.outputs.paths_result).frontend.should_skip

I'm not 100% sure if this is an issue but following the GitHub docs on workflow-syntax-for-github-actions, I was able to get the skip-check working:

You must always use the ${{ }} expression syntax or escape with '', "", or () when the expression starts with !, since ! is reserved notation in YAML format. For example:

if: ${{ ! startsWith(github.ref, 'refs/tags/') }}

Updated code:

if: ${{ needs.pre_job.outputs.should_skip != 'true' && !fromJSON(needs.pre_job.outputs.paths_result).frontend.should_skip }}

How to Use Skip Check With Required Matrix Jobs?

When trying to combine the Example code 3 paths_filter with the example code from Required Matrix Jobs, it does not work. The query (with expression syntax) if: ${{ needs.frontend.result != 'success' }}, will always match and thus always execute the job.step with exit 1, since the expected result of a skipped job is 'skipped'.

The final and only required check should only fail if the result of a preceding job is 'failure'. Not for 'skipped' or 'success'.

What helped me getting my final required Build Result work was the answers related in the discussion:

I hope this helps someone, since I spent much time in investigating. You can find my full build-workflow here.

# This job is the final job that runs after all other jobs and is used for branch protection status checks.
# See: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/collaborating-on-repositories-with-code-quality-features/about-status-checks
# https://github.com/orgs/community/discussions/26822#discussioncomment-5122101
build-result:
  name: Build Result
  needs: [pre_job, core, react, vue, laravel]
  if: ${{ always() }}
  runs-on: ubuntu-latest
  permissions:
    actions: read
  steps:
    - name: Debug build result
      run: |
        echo "pre_job_result: ${{ needs.pre_job.outputs.should_skip }}"
        echo "core_job_result: ${{ needs.core.result }}"
        echo "react_job_result: ${{ needs.react.result }}"
        echo "vue_job_result: ${{ needs.vue.result }}"
        echo "laravel_job_result: ${{ needs.laravel.result }}"

    - name: Check status of all precursor jobs
      if: ${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') }}
      run: exit 1