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

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

Open
jkarstens opened this issue Mar 1, 2023 · 8 comments

Comments

@jkarstens
Copy link

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
Copy link

q0w commented Jul 27, 2023

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
Copy link
Owner

fkirc commented Dec 7, 2023

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
Copy link
Owner

fkirc commented Dec 10, 2023

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

@fkirc fkirc closed this as completed Dec 10, 2023
@q0w
Copy link

q0w commented Dec 10, 2023

@fkirc but is it a bug or expected behaviour?

@paescuj
Copy link
Collaborator

paescuj commented Dec 10, 2023

@fkirc but is it a bug or expected behaviour?

Happy to take a look at it if you can provide some logs 👍

@q0w
Copy link

q0w commented Dec 10, 2023

@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
Copy link
Owner

fkirc commented Dec 11, 2023

Let‘ reopen the issue because my proposed solution seems to be only a workaround

@fkirc fkirc reopened this Dec 11, 2023
@jrson83
Copy link

jrson83 commented Jun 30, 2024

@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

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

No branches or pull requests

5 participants