Job-level "if" condition not evaluated correctly if job in "needs" property is skipped

[joequincy]

Describe the bug
If a job needs a prior job which has been skipped, the if condition for the dependent job may behave unexpectedly under some conditions. (unsure if condition is evaluated incorrectly or if it's not evaluated at all)

To Reproduce
Steps to reproduce the behavior:
Given a workflow such as this (where job_b needs to complete or be skipped before subsequent jobs run, but subsequent jobs don't depend upon any outputs from job_b)

on: push

jobs:
  job_a:
    runs-on: ubuntu-latest
    outputs:
      truthy_string: ${{ steps.a.outputs.always }}
      null_value: ${{ steps.b.outputs.never }}
    steps:
      - id: a
        run: echo "::set-output name=always::something"
      - id: b
        run: echo "We opt not to set any output at this time"
  job_b:
    runs-on: ubuntu-latest
    needs: job_a
    if: needs.job_a.outputs.null_value
    steps:
      - run: echo "We've ensured this job will be skipped"
  job_c:
    runs-on: ubuntu-latest
    needs: [job_a, job_b]
    if: needs.job_a.outputs.truthy_string
    steps:
      - run: echo "This won't run, even though the IF condition evaluates true."
  job_d:
    runs-on: ubuntu-latest
    needs: [job_a, job_b]
    if: always() && needs.job_a.outputs.truthy_string
    steps:
      - run: echo "This will run, even though we've only changed the condition from `true` to `true && true`"

Examining the output of this workflow, job_a will always run, job_b will always be skipped, job_c will always be skipped, and job_d will run.
The only difference between job_c and job_d is the addition of always() && to the if condition.

Expected behavior
If a job-level conditional evaluates to true, the job should run after all needs'd jobs have completed or been skipped.
Both job_c and job_d should run. The always() && should not be required for job_c, since it doesn't change the ultimate true/false result of the conditional.

OR documentation should be updated to indicate this is expected behavior. The current docks simply says of job needs (emphasis added):

Identifies any jobs that must complete successfully before this job will run. It can be a string or array of strings. If a job fails, all jobs that need it are skipped unless the jobs use a conditional statement that causes the job to continue.
This is relatively ambiguous, but the plain interpretation is that a conditional statement that evaluates to true should cause the job to continue. As seen with job_c in the sample, that isn't always the case.

Runner Version and Platform

Version of your runner?
Unsure - I'm only running via Github Actions, not using a self-hosted runner.

OS of the machine running the runner? OSX/Windows/Linux/...
Linux: ubuntu-latest

What's not working?

Jobs are being skipped even when the job-level conditional evaluates to true.

Job Log Output

Because the job is skipped entirely, there is no job-level output, even if the appropriate DEBUG secret is set.

Runner and Worker's Diagnostic Logs

As above, there is no additional output, even if the appropriate DEBUG secret is set.

[MetRonnie]

I think this meant to be a feature actually, but it's too subtle in my opinion - it left me scratching my head for an hour over why my workflow step wasn't running. From https://docs.github.com/en/actions/learn-github-actions/expressions#job-status-check-functions:

A default status check of success() is applied unless you include one of these functions.

I would have expected the default value of if: success() to be replaced with if: <your_condition>, but it's actually if: success() && <your_condition> unless your condition includes one of the status functions

[t0rr3sp3dr0]

This problem is still present on version 2.273.1

[obermillerk]

Stumbled upon this while looking for a different issue, but thought I'd throw in my perspective on the matter.

needs implies a dependent relationship, so if one of the jobs doesn't run (job_b in this case) then logically it seems to me the dependent job should not run either by default, lest there be something missing from job_b's non-existent run. The fact that you can override this behavior by adding the always() check seems like a good feature; you have to explicitly say "This job does not directly depend on the previous job(s) success". I would not call the reported behavior a problem.

The only thing I think would be appropriate and appreciated is a note about this functionality in the docs for the workflow job if field (ie here) noting the relationship between if and needs, and potentially also with the job status check functions. There's a lot of areas in the docs I find where connections like this are completely overlooked, which makes it hard to get up to speed.

[holm]

This also had me scratching my head for a very long time. It's completely counter-intuitive you have to use any of the status functions. Maybe this is from before the outcome and conclusion status fields were available?

It would hope this requirement can be removed, because it just doesn't really make any sense. Why does one have to do if: ${{always() && ....}} to make things work?

[obermillerk]

I think the issue is that needs is the only way to sequence jobs within a workflow and this gives rise to two groups of use cases: the ones where people are using it just to sequence their jobs, and the ones using it for it's intended purpose of creating a dependent relationship.

Those in the first category feel it is illogical because they are using it simply to order their jobs, while those using it for the original intended purpose I would guess generally find it makes sense. A job that needs another job shouldn't run if that other job doesn't, it would likely be broken because of the dependent relationship between the two.

Perhaps some other form of sequencing should be introduced to alleviate this confusion, or as I suggested in my last comment just making this much more clear in the docs could probably do the same work.

[andreykaipov]

In a similar case, I had a conditional job Z I wanted to run at the end of all other jobs A, B, C. However, when any of A, B, or C were skipped, my Z job also got skipped. Adding always() did make it run, but even when the previous jobs failed, so I had to also check the results of my previous jobs:

jobs:
  ...
  Z:
    runs-on: ubuntu-latest
    needs: [A, B, C]
    if: |
      always() &&
      (needs.A.result == 'success' || needs.A.result == 'skipped') &&
      (needs.B.result == 'success' || needs.B.result == 'skipped') &&
      (needs.C.result == 'success' || needs.C.result == 'skipped') &&
      my_original_conditional()
    steps:
    - name: blah
      ...

Very verbose and I'm probably not using workflows as intended, but it works! 😅

[gad2103]

i also find this behavior burdensome and confusing. for example:

---
name: Test

on:
  - pull_request
  - push

jobs:
  jobA:
    name: This will always runs (JobA)
    runs-on: ubuntu-latest
    steps:
      - run: echo jobA
    outputs:
      runs: ${{ true }}

  jobB:
    name: This will be always skipped (JobB)
    needs:
      - jobA
    if: ${{ false }} 
    runs-on: ubuntu-latest
    steps:
      - run: env

  jobC:
    name: This will always run (JobC)
    needs: [jobA, jobB]
    if: always()
    runs-on: ubuntu-latest
    steps:
      - run: echo world
    outputs:
      runs: ${{ true }}

  jobD: # THIS DOESN'T RUN BECUASE IT JUST DEPENDS ON JOB C
    name: This should run if jobC runs (JobD)
    needs: [jobC]
    runs-on: ubuntu-latest
    steps:
      - run: env

  jobE:
    name: This should run if jobD runs (jobE)
    runs-on: ubuntu-latest
    needs:
      - jobD
    steps:
      - run: env

my expectation from working with DAG based systems is that a parent node should determine a child node directly. why does a child node have to know about grandparents in order to execute a task that depends ONLY ON THE PARENT running? can't see the logic here..

[PaulRBerg]

Idea 💡

What if there was a new field called follows (or trails or runs-after), which would would behave like this?

1. Job C lists job A and job B under follows.
2. Job A and job B run.
3. If both A and B were either executed successfully or skipped, run C as well.
4. If either A or B failed, do not run C.

This would be effectively equivalent to @andreykaipov's solution, but less verbose. Example:

jobs:
  ...
  C:
    runs-on: ubuntu-latest
    follows: [A, B]
    if: my_original_conditional()
    steps:
    - name: blah
      ...

[gad2103]

@PaulRBerg do you understand the reasoning behind why needs shouldn't just behave the way we're expecting it to? I'm all for adding a new keyword if it makes sense to preserve the functionality of the original one, but here it seems to me like needs should just always reference the node closest to it in the DAG and the DAG builder should be able to resolve chained dependencies.

I'm trying to think of if needs was built this way to prevent cycles but not coming up with anything that this specific setup ameliorates.

I'm happy to be shown the light though...

[mkungla]

I think following example will produce expected behavior of original question.

job_d:
  runs-on: ubuntu-latest
  needs: [job_a, job_b]
  if: |
    always() &&
    !contains(needs.*.result, 'failure') &&
    !contains(needs.*.result, 'cancelled') &&
    needs.job_a.outputs.truthy_string
  steps:
    - run: echo "${{ toJSON(needs) }}"

[obermillerk]

@gad2103 I think you're arguing a different issue here. This issue is about needs causing jobs not to run if any of the needs jobs are skipped. While I agree with your point that you shouldn't need to include grandparents in needs (if that is actually the case, doesn't seem like it should be), I think your complaint belongs in a separate issue.

[suzuki-shunsuke]

Could you please post your feedback on the GitHub Community Support Forum which is actively monitored.

I created a discussion.

community/community#45058

[DrummerB]

In a similar case, I had a conditional job Z I wanted to run at the end of all other jobs A, B, C. However, when any of A, B, or C were skipped, my Z job also got skipped. Adding always() did make it run, but even when the previous jobs failed, so I had to also check the results of my previous jobs:

jobs:
  ...
  Z:
    runs-on: ubuntu-latest
    needs: [A, B, C]
    if: |
      always() &&
      (needs.A.result == 'success' || needs.A.result == 'skipped') &&
      (needs.B.result == 'success' || needs.B.result == 'skipped') &&
      (needs.C.result == 'success' || needs.C.result == 'skipped') &&
      my_original_conditional()
    steps:
    - name: blah
      ...

Very verbose and I'm probably not using workflows as intended, but it works! 😅

I believe the same result can be achieved like this:

jobs:
  ...
  Z:
    runs-on: ubuntu-latest
    needs: [A, B, C]
    if: always() && !cancelled() && !failure() && my_original_conditional()
    steps:
    - name: blah
      ...

Essentially, we would need a condition success() || skipped() but skipped() is the only state check that does not exist for some reason. However, we can get the same result by starting with always() and removing all other conditions.

I am not 100% sure this works exactly the same in more complex setups with multiple levels of dependencies, but during my testing it seemed to work.